feat(core): productize first-mention events + TLL EO read-path

[moralespanitz]

Summary

Productizes two memory primitives — first-mention events and Temporal
Linkage List (TLL) read-path retrieval — that were previously only
available in the eval harness.

This branch is a clean port off main (vs the existing draft PR #17
which targets experiment/phase2-combined-stack and shares no history
with main).

What's added

• GET /v1/memories/event-chains?user_id=X&entity_ids=Y,Z — returns
  ordered event chains for the given entities (TLL read-path).
• POST /v1/memories/first-mentions/extract — extracts the
  chronological topic-introduction list from a conversation.
• Schema migration: first_mention_events table + 2 indexes.
• TLL infrastructure: temporal_linkage_list table + repository +
  read-path service.

Behavior changes for callers

• Two new HTTP endpoints (additive; no breaking changes).
• Schema migration runs on next npm run migrate.
• TLL chain expansion is a fail-open augmentation in memory-search:
  when an ordering/temporal query is detected and TLL is wired,
  candidates from the entity event chain are appended; errors don't
  block primary retrieval.

Test coverage

• 26 unit cases in tll-retrieval.test.ts (shouldUseTLL regex,
  entitiesForMemories SQL shape, expandViaTLL call ordering /
  10-id slice / userId pass-through).
• 13 integration cases in repository-tll.test.ts (append
  idempotency + predecessor wiring, chain and chainsFor ordering,
  chainEventsForEntities enriched-join + soft-delete filtering).
• 9 unit cases in first-mention-service.test.ts (happy path, salvage
  of truncated JSON, garbage-text fallback, non-array JSON, chatFn
  throw, missing memoryId mapping drop, schema validation drop,
  anchor_date parsing, ascending sort).

Test plan

• npm run migrate:test
• npm test (full vitest suite)
• Manual smoke: ingest a conversation, hit GET /v1/memories/event-chains and POST /v1/memories/first-mentions/extract via curl.

Verification done locally

• npx tsc --noEmit clean.
• npm test — 1253/1253 vitest tests pass against .env.test Postgres
  on this branch.
• npx fallow audit clean (no fatal issues; one duplication warn in
  the new test file's small repeated setup blocks, audit gate exits 0).

[ethanj]

Code review

Read all 18 changed files end-to-end. Tests look thoughtful (26 + 13 + 9 cases) and the architectural shape (repository → service → route, threading both new deps through MemoryServiceDeps as nullable fields) matches the rest of the codebase. The two endpoints are additive and won't break existing callers. Three findings below are blocking; the rest are smaller.

Critical

1. TllRepository.append() has a TOCTOU race that produces duplicate position_in_chain entries
src/db/repository-tll.ts:41–80

append() first does a SELECT MAX(position_in_chain) per entity, then issues a separate INSERT at tip+1. This runs without a transaction or advisory lock, and the unique constraint is only on (user_id, entity_id, memory_id). Two concurrent appends to the same (user_id, entity_id) (the normal case: two ingest requests sharing one entity, e.g. "Postgres") will both read the same tip and both insert at the same position, both with the same predecessor_memory_id. chain() and chainsFor() order by position_in_chain ASC, so tie-break is undefined and chain traversal becomes ambiguous.

The fire-and-forget call site in src/services/memory-storage.ts:226-231 makes this worse — there's no upstream serialization, and failures are only printed to console.error. With batch ingest or a busy user this is not theoretical.

Fix options:

• Replace the read+insert with a single atomic INSERT ... SELECT (SELECT MAX(...)+1 ... FOR UPDATE) so the whole assignment happens inside one statement, or
• Wrap append in BEGIN/COMMIT with SELECT ... FOR UPDATE on the existing chain rows for that (user_id, entity_id).
• Either way, add a UNIQUE (user_id, entity_id, position_in_chain) index so a duplicate-position bug fails loudly at the DB layer instead of corrupting the chain silently.
• Add a concurrent-write integration test (Promise.all([append, append]) with a real pool, then assert chain integrity).

2. Hardcoded similarity: 0.5 on TLL-augmented rows passes through the relevance filter and either silently drops them or pollutes ranking
src/services/memory-search.ts:160-180

Trace: executeSearchStep → maybeExpandViaTLL → hydrateChainMemories returns rows with similarity: 0.5. The combined set is then passed to postProcessResults (line 368), which calls applySearchRelevanceFilter → applyRelevanceFilter. Inside relevance-policy.ts:100-102, relevance = memory.relevance ?? memory.semantic_similarity ?? memory.similarity — so for hydrated rows relevance = 0.5.

Net effect:

• If similarityThreshold > 0.5 (a common 0.6–0.7 production value), every TLL-augmented memory gets filtered out and the entire feature does nothing.
• If similarityThreshold <= 0.5, augmentations pass with a meaningless score that pollutes downstream ranking and audit decisions.

In either case the score is noise, not a signal. TLL membership is a different retrieval signal and shouldn't share the similarity gate. Suggest one of:

• Mark hydrated rows with a dedicated field (retrieval_signal: 'tll-chain') and short-circuit the relevance filter for them.
• Append TLL rows after applySearchRelevanceFilter rather than before, so they're not gated against a similarity score they don't have.

3. observation_date semantic gap — new Date() at TLL-append time, not the memory's observed_at
src/services/memory-storage.ts:225

memories.observed_at already exists in schema.sql:66 and is documented as "when the conversation actually happened (vs created_at = DB insertion time)." The TLL append uses const observationDate = new Date() instead — wall-clock at HTTP-request time. chainsFor() orders by observation_date ASC, so any backfill, batch import, or client passing an explicit observed_at produces a chain in ingest-arrival order rather than conversation chronology. Every downstream temporal-reasoning query will be wrong for that data.

Fix: pass the memory's observed_at (already available on the stored record at this call site) into append() instead of new Date().

Important

4. predecessor_memory_id ON DELETE SET NULL corrupts chain backward-traversal on hard deletes
src/db/schema.sql:337

Hard-delete paths exist (resetBySource in repository-write.ts, full reset). When a node referenced as a predecessor_memory_id is hard-deleted, the FK silently nulls its successor's predecessor pointer — the successor now looks like a chain root and backward traversal stops. Suggest ON DELETE CASCADE for predecessor_memory_id (matches the policy already in place for memory_id); orphaned nodes get removed rather than leaving a half-broken chain.

5. No HTTP-level tests for either new endpoint
src/routes/memories.ts:457-520

Schema validation (missing user_id, invalid UUID in entity_ids, conversation_text exceeding MAX_CONVERSATION_LENGTH, empty memory_ids_by_turn_id), error shapes, and successful-response shapes are all untested at the route layer. Other endpoints in this file have route-level integration tests; these two ship without any. Recommend at least: missing user_id → 400, invalid UUID → 400, oversized body → 400, plus a happy-path response-shape assertion per endpoint.

6. EventChainsQuerySchema has no upper bound on entity_ids count
src/schemas/memories.ts:543-562

A request with thousands of UUIDs generates a single ANY($2::uuid[]) joined against memories with no pagination. An authenticated user has a straightforward amplification vector. Add .refine(q => q.entityIds.length <= MAX_ENTITY_IDS_PER_REQUEST, ...) with a named constant (e.g. 50–100).

7. positionInConversation = m.turn_id conflates two separate concepts
src/services/first-mention-service.ts:198

positionInConversation becomes a duplicate of turnId rather than an independent ordinal. If extractAndStore is called twice on the same conversation and the LLM returns slightly different turn_id values for the same logical topic, the (user_id, memory_id) UNIQUE swallows the second write and you get whichever was first. Suggest computing positionInConversation as the 0-based index in the post-sorted output, and adding a re-extraction idempotency test.

Minor

8. Silent error catching at three sites

• memory-storage.ts:228-231 — fire-and-forget .catch(err => console.error(...)) swallows TLL-append failures and the ingest caller gets a 200 even when chain state is now inconsistent.
• memory-search.ts:143-147 — maybeExpandViaTLL swallows all errors as fail-open augmentation.
• first-mention-service.ts invokeLlm — catches and returns null silently to stderr.

The fail-open behavior may be deliberate, but per CLAUDE.md "no silent error catching" / "no fallback modes," at minimum: route through the structured logger rather than console.error, surface a tllExpansionFailed flag in the trace, and document the deliberate fail-open in tech-debt.md if it stays.

9. shouldUseTLL regex false-positives
src/services/tll-retrieval.ts:22-27

Bare \b(first|last|before|after|then|later|track)\b fires on common factual lookups: "what is my first name", "the model used before GPT-4", "track my spending". Direct harm is modest because expansion is fail-open and additive — but see finding #2, where over-triggering compounds the similarity-gate problem. Tightening the regex (e.g. require two ordering signals) would reduce both.

10. slice(0, 10) magic number duplicated in two files
src/services/tll-retrieval.ts:59 and src/services/memory-search.ts:135. Pull into a named constant (e.g. TLL_ENTITY_LOOKUP_SEED_LIMIT).

11. Hardcoded inputTokens: 0, outputTokens: 0 in the runtime-container.ts chat adapter
src/app/runtime-container.ts:228-229. Cost telemetry from FirstMentionService will be wrong if anything starts reading these fields. Either thread real token counts through llm.chat, or remove the fields from the ChatResult shape.

---

Findings #1–#3 are blocking; #4–#7 are important to address before merge or in a fast-follow. Strong shape overall — the architectural primitives (TLL, first-mention) are well-isolated, repositories are thin, and the test files demonstrate the intended invariants clearly. Just need the concurrency, scoring, and timestamp issues nailed down before the read-path goes live.

[moralespanitz]

Thanks for the detailed review. Addressed in commits dc7b9ff, 91676f4, 4020135, d6fa8ed, 63efbea (initial round) and 58c3095, 623ac96, 9eacd79, 36701b0, 35dc36a, c46cddf (fast-follow round).

Blockers (fixed)

• Improve generic temporal retrieval evidence #1 TOCTOU race in TllRepository.append() (dc7b9ff) — replaced read+insert with atomic INSERT...SELECT under pg_advisory_xact_lock keyed on (user_id, entity_id); added UNIQUE (user_id, entity_id, position_in_chain) index as defense-in-depth; added a concurrent-write integration test that fires Promise.all([append, append, append]) to the same chain and asserts positions 0/1/2 with correctly wired predecessors.
• feat: enforce retrieval relevance thresholds #2 hardcoded similarity: 0.5 on TLL-augmented rows (91676f4) — moved TLL augmentation AFTER applySearchRelevanceFilter. Hydrated rows now carry similarity: null and retrieval_signal: 'tll-chain' and ride around the similarity gate entirely.
• feat(extraction): preserve first-person negations in extraction prompt #3 observation_date semantic gap (4020135) — TLL append now threads the caller's logicalTimestamp through from storeCanonicalFact, falling back to the just-stored memory's observed_at column when absent. No more new Date() reordering chains by ingest-arrival time.

Important (fixed)

• fix(extraction): bump EXTRACTION_MAX_TOKENS 4096 → 8192 #4 predecessor FK on delete (d6fa8ed) — changed to ON DELETE CASCADE, matching the memory_id policy on the same table. Idempotent migration via DO $$ … END$$ since CREATE TABLE IF NOT EXISTS won't update column-level FKs on existing tables.
• feat(search): EXP-12 — log-spaced recency bins for TR and MR #6 entity_ids upper bound (63efbea) — capped at 100 in EventChainsQuerySchema via a named MAX_ENTITY_IDS_PER_REQUEST constant.

Fast-follow round (now also fixed)

• feat(search): EXP-05 — instruction-type tagging and search boost #5 HTTP-level route tests (c46cddf) — new src/routes/__tests__/event-chains-and-first-mentions.test.ts covers the schema-validation 400 paths for both endpoints (missing user_id, invalid UUID in entity_ids, > 100 cap, empty tokens, missing/oversized conversation_text, missing memory_ids_by_turn_id, missing source_site) plus a happy-path shape assertion per endpoint that parses the response with EventChainsResponseSchema / FirstMentionsExtractResponseSchema. 12/12 new tests pass.
• feat(extraction): EXP-06 — generic event anchors for As-of facts #7 positionInConversation = m.turn_id conflation (35dc36a) — mapToEvents now sorts candidates by turnId ASC and assigns positionInConversation as the 0-based post-sort index. Re-runs of extractAndStore on the same conversation now produce identical (position, topic) tuples even when the LLM's turn_id assignment drifts between runs. Added a service-level idempotency test plus a new DB-integration test in repository-first-mentions.test.ts that exercises ON CONFLICT DO NOTHING behaviour with drifted turn_ids and asserts only 2 rows survive.
• feat(search): EXP-14 — retrieval-side abstention gate #8 silent error catching at 3 sites (36701b0) — TLL append (memory-storage.ts), TLL expansion (memory-search.ts), and first-mention LLM call (first-mention-service.ts) now log via structured prefixes ([tll-append-failed], [tll-expansion-failed], [first-mention-llm-failed]). The TLL-expansion path additionally surfaces a tll_expansion_failed event on the active retrieval TraceCollector so the failure is observable in trace artifacts. Comments at each site document the deliberate fail-open (TLL augmentation and first-mention extraction are best-effort augmentations, not primary mutations — fail-open here is the correct policy under CLAUDE.md's "no fallback modes" rule, which targets AUDN mutations).
• feat(extraction): EXP-05 H-310 — extend INSTRUCTION_MARKERS for BEAM phrasings #9 shouldUseTLL regex false-positives (9eacd79) — replaced the single-alternation regex with a two-tier check: an ORDERING_TERMS_RE set of single-token signals (first/last/before/after/then/later/earlier/previous/next/prior) where TWO must co-occur to fire, plus a curated SEQUENCE_PATTERNS list of structural phrases (in (chronological/reverse/the) order, when did, since when, over time, evolution of, history|timeline of, originally/initially, progression of, how X evolved/shifted/changed, brought up) where one phrase suffices. Removed track, sequence, and bare order from the gate. Updated test asserts false for the three reviewer-cited false-positives ("what is my first name", "the model used before GPT-4", "track my spending") and true for canonical EO/MSR/TR shapes. 41/41 unit tests pass.
• feat(memory-ingest): EXP-08 — scheduled consolidation every N turns #10 slice(0, 10) duplicated magic number (623ac96) — defined and exported TLL_ENTITY_LOOKUP_SEED_LIMIT = 10 in tll-retrieval.ts; removed the duplicate TLL_SEED_CANDIDATE_COUNT declaration from memory-search.ts and imported the canonical constant; updated the unit test to reference the exported name instead of the literal 10.
• feat(search): Phase 4B — broader IF retrieval preference (two-stage) #11 hardcoded inputTokens: 0 in chat adapter (58c3095) — LLMProvider.chat() returns Promise<string> (no usage), and nothing in the FirstMentionService path actually consumed the fields. Took the safer of the two reviewer-suggested options: dropped inputTokens / outputTokens from the local ChatResult shape entirely. Per-call cost telemetry continues to flow from LLMProvider.chat -> writeCostEvent unchanged. JSDoc on the trimmed type and on the runtime-container adapter both document the deliberate decision.

Pre-commit status (this push)

• npx tsc --noEmit: clean
• npm test: 1277/1277 tests pass across 127 test files
• npx fallow audit: clean (audit gate excluded 1 inherited finding; new findings: 0)

PR is now ready for re-review.

[ethanj]

Code review (v2 — after dc7b9ff..c46cddf)

Re-read the 12 follow-up commits, each labeled to the v1 finding it addresses. 10 of the 11 prior findings are properly resolved. One fix (#2 — move TLL augmentation past the similarity gate) is the right design but introduces a new blocking regression on the response-formatting path. Details below.

✅ Verified resolved

• Improve generic temporal retrieval evidence #1 — TOCTOU race in append() (dc7b9ff). Per-entity transaction guarded by pg_advisory_xact_lock on (namespace, hashtext("$user:$entity")), with predecessor and position_in_chain computed inline by INSERT ... SELECT. UNIQUE (user_id, entity_id, position_in_chain) index added as defense-in-depth so any future bypass fails loudly. New integration test at repository-tll.test.ts fires three concurrent appends via Promise.all and asserts [0,1,2] positions with correctly-wired predecessors. The previous batch-insert path is gone, so the per-entity loop now does N round-trips for N entities — acceptable correctness/perf trade-off.
• feat(extraction): preserve first-person negations in extraction prompt #3 — observation_date (4020135). logicalTimestamp threaded from storeCanonicalFact through to a new maybeAppendTll helper. Falls back to memory.observed_at (looked up via the store), and only resorts to new Date() if that lookup itself fails (with a structured log). Conversation-chronology ordering is preserved for backfilled / out-of-order ingests.
• fix(extraction): bump EXTRACTION_MAX_TOKENS 4096 → 8192 #4 — predecessor FK (d6fa8ed). ON DELETE CASCADE on predecessor_memory_id, plus an idempotent DO $$ ... DROP CONSTRAINT / ADD CONSTRAINT $$ block so existing DBs migrate (the column-level CREATE TABLE IF NOT EXISTS would not have updated the constraint).
• feat(search): EXP-05 — instruction-type tagging and search boost #5 — HTTP tests (c46cddf). New routes/__tests__/event-chains-and-first-mentions.test.ts covers 12+ cases across both endpoints: missing user_id, invalid UUID, exceeding the 100-entry cap, empty tokens, oversized conversation_text, missing fields, and happy-path response-shape assertions for each. Real Express app + real DB.
• feat(search): EXP-12 — log-spaced recency bins for TR and MR #6 — entity_ids cap (63efbea). .refine(...) cap at 100 with a named constant and a covering test.
• feat(extraction): EXP-06 — generic event anchors for As-of facts #7 — positionInConversation idempotency (35dc36a). Now the 0-based index in the post-sorted output rather than turn_id. Two new tests verify stable positions across re-extraction even when LLM turn_id drifts; integration test confirms (user_id, memory_id) UNIQUE semantics.
• feat(search): EXP-14 — retrieval-side abstention gate #8 — silent-error logging (36701b0). Three fail-open sites now emit structured prefixes ([tll-expansion-failed], etc.) and the trace records a tllExpansionFailed flag callers can observe. Behaviour stays fail-open by design (which the commit message correctly grounds in CLAUDE.md "no fallback modes" applying to mutations, not augmentation).
• feat(extraction): EXP-05 H-310 — extend INSTRUCTION_MARKERS for BEAM phrasings #9 — shouldUseTLL precision (9eacd79). Replaced the loose alternation regex with a two-tier check: single-token signals require co-occurrence (e.g. "before AND after"), structural phrases match alone (in (chronological/reverse) order, over time, evolution of, etc.). Removed the largest false-positive sources (track, sequence, bare order). The previously-flagged shapes (what is my first name, the model used before GPT-4, track my spending) are now in the negative-test list.
• feat(memory-ingest): EXP-08 — scheduled consolidation every N turns #10 — magic 10 (623ac96). Single exported TLL_ENTITY_LOOKUP_SEED_LIMIT reused from both expandViaTLL and memory-search.ts.
• feat(search): Phase 4B — broader IF retrieval preference (two-stage) #11 — token-count placeholder (58c3095). ChatResult shrunk to { text: string }. Misleading zeros gone; LLMProvider.chat continues to write its own cost telemetry independently.

❌ New regression introduced by fix #2

Blocking. hydrateChainMemories returns objects that aren't valid SearchResults and will crash the response-rendering path. 91676f4 src/services/memory-search.ts:198-210

The intent is right — chain-augmented rows shouldn't pass through the similarity gate. But the new shape sets similarity: null and omits score, source_site, summary, and several other required SearchResult/MemoryRow fields. The as unknown as SearchResult cast at line 210 hides this from the typechecker.

When the augmented rows reach retrieval-format.ts via appendTllAugmentation → assembleResponse → packageSearchOutput → buildInjection → formatFullLine/formatStagedLine → buildCommonAttrs, the formatter crashes on missing/null fields:

// src/services/retrieval-format.ts:275-286
function buildCommonAttrs(memory: SearchResult, index: number): string {
  return [
    `index="${index + 1}"`,
    `source="${escapeXml(memory.source_site)}"`,         // undefined → "undefined" in output
    `memory_id="${memory.id}"`,
    `created_at="${memory.created_at.toISOString()}"`,
    `importance="${memory.importance.toFixed(1)}"`,
    `similarity="${memory.similarity.toFixed(2)}"`,      // 💥 TypeError: null
    `score="${memory.score.toFixed(2)}"`,                // 💥 TypeError: undefined
    ...
  ].join(' ');
}

SearchResult requires similarity: number (not nullable) and score: number, plus the full MemoryRow surface (source_site, summary, embedding, memory_type, metadata, keywords, overview, trust_score, observed_at, last_accessed_at, access_count, expired_at, deleted_at, network, opinion_confidence, observation_subject, source_url, episode_id, status).

Why CI is green:

1. The as unknown as SearchResult cast silences tsc --noEmit.
2. Unit tests for maybeExpandViaTLL (tll-retrieval.test.ts) mock at the expandViaTLL boundary, never running real hydration.
3. The new HTTP tests in event-chains-and-first-mentions.test.ts hit the read endpoints (which use chainEventsForEntities directly), not the search retrieval path that triggers appendTllAugmentation.
4. There is no end-to-end test where a shouldUseTLL-positive query goes through performSearch with a real seeded chain.

This will fire the first time a production user issues an ordering query against a workspace that has TLL chains populated — i.e., the moment the feature does what it's supposed to.

Suggested fix. Either of these lines up cleanly:

• (a) Hydrate the full MemoryRow shape, with similarity: 0 and score: 0 (or copy the top-candidate's similarity as an inherited signal). The query becomes SELECT * (or an explicit field list matching MemoryRow) and the synthesized object covers every required field. Strictest fix — the type cast can come out.

• (b) Special-case retrieval_signal === 'tll-chain' in buildCommonAttrs — emit the chain-membership flag instead of similarity/score, and tolerate missing fields. Keeps the projection narrow but spreads TLL awareness into the formatter.

• (c) is fragile but minimal: set similarity: 0, score: 0, leave the missing string fields as empty strings or 'tll-chain'. Stops the crash but pollutes the rendered output with placeholder values.

Add an integration test that drives a shouldUseTLL-positive query through performSearch against a populated chain and asserts the response renders without throwing. The unit tests as written cannot catch this class of bug.

Other notes

• The advisory-lock namespace 0x544c4c00 ("TLL\0") is a nice touch and avoids collisions with unrelated callers; worth a comment pointing at any other namespaces in use elsewhere if/when they're added.
• Per-entity transactions in appendOne (one DB round-trip per entity) is a bigger correctness win than a perf cost in the typical case (1–3 entities per memory). If batch-ingest perf becomes an issue later, consider a sorted-by-entity-id batched path that still serializes via the advisory lock per chain.
• The structured-logging change in feat(search): EXP-14 — retrieval-side abstention gate #8 is good but I'd lean toward routing through the project's structured logger (when one exists) rather than console.error — but the prefix scheme ([tll-expansion-failed]) is grep-friendly and a strict improvement over the prior state.

Net: ship-ready once #2's hydration shape is fixed and an integration test covers the augmentation path.

[ethanj]

additional review from codex:

Findings

• High: TLL’s public event-chain read path is still append-order, not observation-time order. appendOne() selects predecessor and position from the current chain tip by position_in_chain
  atomicmemory-core/src/db/repository-tll.ts:81, and chainEventsForEntities() returns endpoint events ordered by that same append position atomicmemory-core/src/db/repository-tll.ts:165. That
  contradicts the stated backfill/chronology contract in atomicmemory-core/src/services/memory-storage.ts:228. Out-of-order/backfilled appends for the same entity will return and predecessor-link by
  ingest order, so EO/TR claims are not proven.
• Medium: POST /memories/first-mentions/extract validates memory_ids_by_turn_id as arbitrary string values atomicmemory-core/src/schemas/memories.ts:584, then passes them through to an insert into a
  UUID column atomicmemory-core/src/db/repository-first-mentions.ts:58. If the extractor emits a matching turn for an invalid memory ID, this becomes a DB error and the route returns 500 via
  atomicmemory-core/src/routes/route-errors.ts:14, not a 400. The route tests cover missing fields, but not invalid mapping values atomicmemory-core/src/routes/tests/event-chains-and-first-
  mentions.test.ts:215.

[moralespanitz]

it's ok @ethanj