Files
interactive-story/planning/reports/M6-IMPLEMENTATION-REPORT.md
JesseMarkowitzandClaude Opus 5 a6e9c7a32b M6: branch-safe context, summaries and long-term story memory
Aligns the inherited AI-DnD memory and context foundation with the history,
authority and state model M3-M5 established. Long stories now reach the narrator
through a bounded, lineage-safe, inspectable context rather than a growing
transcript.

This commit includes the corrective work that followed the independent review in
planning/reports/M6-IMPLEMENTATION-REPORT.md. The first implementation reported
E03 as passing and it was not; the report records that history rather than
hiding it.

What was already correct, and was kept rather than rebuilt

  Memory lineage. Memories already carried (branch_id, depth) and retrieval
  already filtered through the capped-path clause; the ten-step negative control
  was measured passing against b7005e6 before any change here. M6 adds the
  regression tests that pin it, plus provenance and authority on the result.

Summary lineage — both halves

  A summary is a row carrying the coordinate of the last node it covers, and
  eligibility is the same head-capped lineage clause memories use. That alone
  was not enough: generation was seeded from adventures.story_summary, a
  campaign-global column with no lineage, so after a divergence the summariser
  was handed the abandoned line's prose and asked to update it. The row it
  produced was correctly anchored and therefore looked safe while its sentences
  described a story the reader had left.

  Generation is now seeded from summaries.current — the same question the
  context builder asks — so the input and the output are scoped by one rule.
  adventures.story_summary remains a reader-facing mirror for the Plot panel and
  the export bundle, kept in step when a summary is written and when the head
  moves, and nothing authoritative reads it.

Retrieval redundancy

  With a real embedding model, four near-identical memories crowded out the one
  distinctive clue, which survived only because the default memory_top_k is 5.
  Retrieval now drops a candidate that repeats one already chosen, never across
  authority classes, at a threshold measured against the configured embedding
  model. The clue is retrieved at top_k 5, 4 and 3. Ranking itself is unchanged;
  the further factors CONTEXT-AND-MEMORY §20 contemplates remain unimplemented
  and are recorded as such.

Memory authority, budgeting, observability

  Memory.authority is accepted_story or heuristic, classified by the application
  and marked in the prompt; retrieval never writes state. The reply is reserved
  out of the context budget, and an impossible configuration fails clearly
  instead of overflowing. Each derived pass records ok/idle/failed per campaign,
  served by GET /adventures/{id}/derived and shown in Insights, so the M2
  failure — a dead memory bank with a green suite — is visible if it recurs.
  Provider-wiring tests mock no factory.

Also: two pre-existing test-suite leaks fixed; two fixtures that stored one
vector in every memory now use distinct ones, so lineage assertions stay
readable alongside redundancy suppression.

Planning: CONTEXT-AND-MEMORY, TECHNICAL-DESIGN, DATA-MODEL, V1-ACCEPTANCE-TESTS,
BUILD-MILESTONES, VERSION and planning/README updated to describe what exists,
including that a valid E03 test must regenerate a summary after diverging. The
M5 report was rotated to planning/archive/milestone-reports/. No new ADR — every
choice implements a decision the package had already settled.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWU4gTfLYY6Qq9U7aa9Qw2
2026-09-06 03:00:33 -04:00

51 KiB
Raw Permalink Blame History

M6 Implementation Review — Branch-Safe Context, Summaries, and Long-Term Story Memory

Review date: 2026-09-05 Reviewer: independent review pass (the M6 build summary was treated as claims to verify) Tree reviewed: the staged working tree on m6-context-memory. There are no M6 commits.


A. Executive result

PASS WITH CORRECTIVE WORK REQUIRED

M6 delivers most of what it promises, and the parts that work are demonstrated rather than asserted: memory lineage safety, background-failure observability, context budgeting, provenance, authority, migration and restart all hold up under independent testing with real local models.

One blocking defect: abandoned summary content still reaches the active prompt (E03). The row-level eligibility M6 built is correct, but the summariser seeds each new summary from a campaign-global column that has no lineage, so a summary generated after a divergence inherits the abandoned line's prose. This is one of the failure modes the milestone explicitly names.


B. Scope and review method

Evidence was gathered in the order the brief prescribes — observed end-to-end behaviour first, source inspection last. Specifically:

  • Real local models for the summariser, the memory extractor and the embedder, through the production provider path, for the E-series, F02 and the summary-generation checks.
  • Deterministic reproductions where a mechanism needed isolating.
  • A genuine OS process boundary for the restart check (two processes, two PIDs, one database file).
  • A real socket failure (endpoint pointed at a dead loopback port) for the observability gate, rather than patched application functions.
  • A real browser (Firefox 154.0.1 over WebDriver) against the built frontend.
  • A clean export of the M5 base commit to establish the baseline by measurement rather than from the previous report's claim.

Every negative result below is preceded by a positive control.

The M6 non-scope was preserved: M7 imported knowledge is absent and is not scored as an M6 defect; no remote embedding, vector database or cloud path was introduced; Story Cards were not promoted into a knowledge subsystem.


C. Repository and provenance state

branch                       m6-context-memory
HEAD                         b7005e6fddec337f940749455739a60f0c6cb54f
HEAD signature               Good signature (RSA 7D8AE19DB5C68569), status G
M6 base commit               b7005e6  — the accepted M5 closeout
M6 commits                   NONE
staged                       29 files changed, 2973 insertions(+), 48 deletions(-)
unstaged                     0
untracked                    0
working tree == index        yes
upstream ancestry            d72f7c1b is an ancestor of HEAD
LICENSE                      unchanged (not in the change set)

All M6 work is uncommitted. HEAD is still the M5 closeout, so there is no M6 commit to sign or verify. Every test result in this report was produced from the staged working tree, which is byte-identical to the index.

A commit message is prepared at .git/M6_MSG (untracked, outside the work tree).


D. Change inventory

Added (7)

backend/app/summaries.py                     lineage-anchored summary store
backend/app/derived.py                       per-campaign derived-work status
backend/tests/test_context_memory.py         F01-F08, E02/E03, authority   (20 tests)
backend/tests/test_provider_wiring.py        real provider construction    (10 tests)
backend/tests/test_context_performance.py    query growth, context size     (5 tests)
backend/tests/test_context_realistic.py      real-model context             (3 tests)
planning/reports/M6-IMPLEMENTATION-REPORT.md this document

Modified (21) — context/builder.py, context/__init__.py, memorybank.py, migrations.py, models.py, routers/adventures/{crud,insights,turns}.py, six existing test files, InsightsPanel.jsx, play.css, and five planning documents.

Renamed (1) — planning/reports/M5-IMPLEMENTATION-REPORT.md → planning/archive/milestone-reports/. Verified a pure rename: git diff -M reports 0 insertions and 0 deletions, so the archived M5 report is unmodified.

Schema — two new tables (summaries, derived_status), one new column (memories.authority), three migrations (89 column, 90 index, 91 data pass). No new dependency. No requirements.txt or package.json change.


E. Implemented architecture

turn accepted ─┬─ authoritative state (M5, unchanged)
               └─ schedule_post_turn ── fire-and-forget task
                     ├─ _guarded(memory)     ─ summarise blocks → memories
                     ├─ _guarded(summary)    ─ roll up → summaries row
                     └─ _guarded(embedding)  ─ embed pending memories
                          each records ok/failed to derived_status

build_context
   protected = system + canon + state + summary + memories + input
   reserve   = max_output_tokens + 64
   history   = newest-first until (budget − protected − reserve) is spent
   raises ContextOverflow when protected + reserve >= budget

eligibility, for both memories and summaries:
   lineage.path_of(db, adventure).clause(Model)      # head-capped

The single most important architectural fact is that summaries were moved onto the same eligibility chokepoint memories already used, rather than getting a mechanism of their own. That is the right shape, and it is why Undo, Redo, Save Point restore and divergence need no per-feature rules. The defect in §H is not in that mechanism; it is in what the summariser is fed.


F. F01–F08 acceptance results

ID Result Evidence
F01 Recent turns remain coherent PASS Assembled prompt contains the preceding narration and the reader's input in order, after ordinary play and after Undo/Redo. Read from the context report, not the prose.
F02 Old important event retrieval PASS (fragile) At the production default (memory_top_k=5), all five conditions hold — see §F02 below. At memory_top_k=4 the same fixture fails.
F03 Prompt remains bounded PASS 51 → 231 actions: input tokens identical at 5,342, included capped at 38–40. Measured through the builder, not by reading a constant.
F04 Output token reserve PASS Reserve = max_output_tokens + 64, subtracted before history selection. input + reserve <= budget at every size measured. An impossible budget raises ContextOverflow naming both figures.
F05 Prompt inspector PASS for the components M6 owns All M6-owned components inspectable in API and browser (§K). M7 imported knowledge absent and not simulated.
F06 Retrieval provenance PASS for story memory Every retrieved memory carries branch_id, depth, source_start/end; the coordinate resolves to a real action row (verified: action id 6).
F07 Heuristic memory is not canon PASS Classified by the application, marked [inferred] in the prompt, exposed in the API, and produced no authoritative fact.
F08 Memory failure is non-fatal PASS Real socket failure; story, state, head intact; failure recorded, served, cleared on recovery.

F02 in detail — the fragility

Real embedder (nomic-embed-text) and real memory extractor (qwen2.5:3b-instruct). One distinctive clue planted early, 26 long filler turns played over it, then a query naming the clue's subject.

story                    57 actions, 14 included in the prompt
1. absent from verbatim history                    yes
2. present in the memories section                 yes
3. recovered in the prompt                         yes
4. not because the transcript was sent             yes  (14 of 57)
5. provenance resolves to a real action            yes  (b1 d5, src 0-5, action 6)
   authority                                       accepted_story

But the recovering memory ranked last of five:

sim 0.8157  The party walks the muddy road north, talking of weather…
sim 0.8132  The party walks the muddy road north, talking of weather…
sim 0.8114  The party walks the muddy road north, talking of weather…
sim 0.7984  The party walks the muddy road north, talking of weather…
sim 0.61    Aldric found a brass key carved with a heron under the third flagstone

Four near-identical filler memories outrank the one memory that matters. At memory_top_k=4 the clue is evicted and F02 fails outright; the production default of 5 is what saves it. Ranking is pure cosine similarity plus a pinned flag — none of the other factors CONTEXT-AND-MEMORY.md §20 lists (importance, entity overlap, recency, thread overlap) is implemented, and §22's duplicate suppression is not implemented at all. See finding M6-F2.


G. Memory lineage safety (E02)

PASS, end to end, with positive controls. Real embeddings; sentinel ABANDONED-MARA-SPY-7421.

positive control  5 memories retrieved on path A                      yes
                  sentinel inside a retrieved memory                  yes
                  sentinel in the active prompt                       yes
negative          undo below the sentinel, diverge, play 9 turns
                  sentinel in the active prompt on path B             NO
retention         rows still on disk                     7 memories (was 5)
re-embedding      memories awaiting embedding after return to lineage  0

Ineligibility is not deletion: the abandoned memories remain stored, and the derived endpoint lists retained-but-ineligible summaries explicitly. The deterministic Undo → Redo control (no divergence) shows eligibility returning without re-embedding.

A note on one control: after a divergence, Redo cannot return to the superseded line — that is correct M3 divergence semantics, not a memory defect, and the return-to-lineage control is therefore only meaningful for Undo/Redo without divergence, where it passes.


H. Summary lineage safety (E03) — FAILS

Row-level eligibility is correct. A summary carries (branch_id, depth) and source_start/source_end; summaries.current selects the newest row on the head-capped lineage; abandoned rows are retained and reported as ineligible. Verified directly: after divergence the path-A summary row was ineligible and still on disk (1 eligible, 2 retained-but-ineligible).

Content propagation is not. _update_story_summary seeds the model with:

current = adventure.story_summary.strip()     # memorybank.py, in _update_story_summary

adventures.story_summary is a campaign-global mirror that summaries.record rewrites on every write, regardless of which line produced it. After a divergence the summariser is therefore handed the abandoned line's prose and asked to update it, and the resulting row — correctly anchored to the new branch — carries the abandoned content forward.

Real-model evidence. Sentinel ABANDONED-CHAPEL-OATH-9930, established only on path A:

path A   summary row id 2, branch 1, depth 30, source 15–30      sentinel present
undo to head (1,0); diverge; play 9 turns; run the derived pass
path B   summary row id 3, branch 2, depth 16, source 1–16
         >>> sentinel IS in the active prompt on path B

Deterministic reproduction, with the memory extractor emitting the sentinel only for blocks that actually contain it:

path B summary section:
  "- MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[ABANDONED-CHAPEL-OATH-9930]
   - MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[ABANDONED-CHAPEL-OATH-9930]
   - MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[dry road]"

sentinel in the ACTIVE PROMPT on path B                    True
summariser seed contained the sentinel                     True
path-A actions on path B's lineage                         0

Zero path-A story is on path B's lineage, and the path-B events contributed only MEM[dry road] — so the sentinel can only have arrived through the seed.

The brief's question "whether invalid later coverage is rebuilt rather than silently reused" is answered: the row is rebuilt, the content is reused.

Why the existing tests missed it: test_e03_an_abandoned_summary_is_retained_but_never_used and browser check 10b both verify that the old row is not used, and neither regenerates a summary after the divergence. The blind spot is shared by the unit test and the browser check.


I. Memory authority and provenance

PASS. Two classes, both load-bearing rather than cosmetic.

schema        memories.authority  "accepted_story" | "heuristic"
creation      memorybank.classify_authority — the application decides, from
              hedging in the memory's own text; a model label is not trusted
retrieval     returned in the same query as the text (no extra round trip)
prompt        "Lines marked [inferred] are interpretation, not established
               fact — do not treat them as settled truth"
              - Mara handed Captain Vale the sealed letter.
              - [inferred] Mara seemed nervous around Captain Vale.
inspector     authority + source coordinate per memory, in API and browser

Verified that a heuristic memory produced no authoritative fact: after retrieval the state document's facts list was empty and no Vale-related assertion existed. Retrieval has no write path to state; the M5 typed-event pipeline remains the only route (ADR 013).

Provenance is real, not a similarity score. The returned coordinate resolves to an actual action row, checked by query.

Authority conflict (§13). With a memory asserting a fact the reader had since withdrawn by manual correction:

memory still retrieved saying she knows it            yes
state section says "No longer true"                   yes
withdrawn fact absent from the facts that stand       yes

The authoritative correction is stated in the same prompt and outranks the memory. Correct, though the contradicting memory is still shown unqualified — the correction wins by being explicit, not by suppressing the memory.


J. Context budgeting and bounded prompt

PASS, measured.

actions  included  input   protected  reserve  budget  input+reserve<=budget
     11        11    808         756      564    6000   yes
     71        38   5208         762      564    6000   yes
    251        38   5208         762      564    6000   yes

The prompt stops growing once the budget is reached: 251 actions produce exactly the same 5,208 tokens as 71. On a realistic prompt (real model, 83 actions, 8,192 budget): input 7,315 + reserve 764 = 8,079, inside budget, 41 of 83 actions included.

Protected content is never dropped for older prose, and an impossible configuration raises ContextOverflow naming both figures rather than building a prompt known to overflow — a behaviour change from M5, where nothing was reserved at all.


K. Prompt inspector

PASS for the components M6 owns, verified in a real browser rather than by reading JSX.

Component API Browser
narrator / system rules sections[narrator] shown
current authoritative state sections[narrative_state] shown
summary used + source coverage summary{branch_id,depth,source_start,source_end,trigger,model} "summary in use · covers …"
retrieved memories memories.used[] + considered listed
memory authority used[].authority "inferred" / "from the story"
memory provenance used[].source{} "turn N"
recent history sections[history], history.included/total shown
user input inside history shown
model / settings settings{} shown
token cost per component sections[].tokens per-section, with %
budget / reserve tokens{budget,output_reserve,protected,available_for_history} "… reserved for the reply"
derived-work health derived[] warning banner

M7 imported knowledge is absent, not stubbed. That is the correct state for M6 and is not scored as a defect.


L. Background failure and observability — the M2 gate

PASS. This was tested by pointing the derived endpoint at a loopback port with nothing listening, so the failure happens inside the production fire-and-forget path at the socket, not by patching application functions.

1. accepted story intact            actions 43 → 43
   head intact                      (1,42) → (1,42)
   authoritative state intact       unchanged
2. observable through the API       GET /derived → 200
     failing: ["memory", "summary"]
     memory     failed x17  ProviderError: Request to AI endpoint failed…
     summary    failed x16  ProviderError: Request to AI endpoint failed…
3. visible in the prompt inspector  context report carries "derived"
4. no half-written derived data     0 memories, 0 summaries
5. story continues                  next turn landed
6. recovery clears it               failing: []  after one healthy pass

The failure counts (x17, x16) accumulated from the real background tasks the turns scheduled, which is itself evidence that the production fire-and-forget path records rather than swallows. A structured log entry is also emitted (log.exception in derived.failed).

This satisfies the M2 requirement that a dead memory bank be discoverable. It is not a test-only capture: the same record is served by the API and rendered in the browser.


M. Real provider construction and realistic context

PASS, and not by mock.

tests/test_provider_wiring.py builds a real Settings row, round-trips it through the database, and calls the real factories — no factory is patched. A parametrised test names each Settings attribute the providers and builder read, so a renamed column fails by name rather than killing the memory bank in an unobserved task. Construction is proven inert by refusing socket.connect.

Beyond that, this review ran the whole derived pipeline against real models:

endpoint      trusted-LAN, HTTPS, through the ordinary endpoint policy
narrator/summary model   qwen2.5:3b-instruct        (warm; resident between runs)
embedding model          nomic-embed-text
result        5 real memories written, 2 real summaries written,
              real vectors, derived_status {embedding: ok, memory: ok, summary: ok}
              retrieval returned those memories and they reached the prompt

That exercises construction → request → response → derived record → retrieval → use, which is the chain the brief asks for. Secrets are not recorded here; the endpoint host is deliberately omitted.

The project's own realistic-context tests (3) pass against the same endpoint.


N. Undo / Redo / Save Point / divergence matrix

Transition Head before → after Eligible memories Eligible summary Retained on disk
normal play (1,42) 5 row 2 (b1 d30, src 15–30) —
Undo below the source (1,42) → (1,0) sentinel excluded excluded all rows kept
Redo (no divergence) restored sentinel eligible again, 0 re-embeddings eligible again all rows kept
Save Point restore (1,6) → (1,2) filtered to the restored depth row at d4 becomes ineligible kept
divergence (1,0) → (2,18) sentinel absent from prompt old row ineligible; new row carries abandoned content (§H) 1 eligible, 2 retained-ineligible

E01/E04 regression: after Undo and divergence, a fact established on the abandoned line was absent from both the authoritative document and the prompt — M5 behaviour is not regressed by the new context selection.


O. Persistence, restart and migration

Restart — PASS, across a genuine OS process boundary (two PIDs, one file):

                     build (pid 976700)      after restart (pid 976753)
actions                          43                          43
head                        (1, 42)                     (1, 42)
memories / summaries           7 / 2                       7 / 2
sentinel in prompt               yes                         yes
summary provenance     id 2, b1 d30, src 15–30    identical, same created_at
memory provenance                 —      resolves (b1 d11 src 6–11, b1 d17 src 12–17)
derived status                    —      embedding=ok memory=ok summary=ok

Migration — PASS. A genuine pre-M6 database was built (M6 tables dropped, authority column removed, stamp rewound to 88) and migrated:

stamp 88 → 91
summaries rows                     1   (the old story_summary, backfilled)
backfilled anchor          b1, depth 4, source_end 4, trigger "interval"
                           — i.e. at the campaign's summary cursor, correctly
memories.authority         all rows defaulted to "accepted_story"
campaign opens             200, 7 actions, summary eligible in the prompt
undo / redo                200 / 200
Save Point restore to d2   200; the depth-4 summary correctly becomes INELIGIBLE
new M6 turn                200, forks to branch 2 (correct post-restore divergence)
derived endpoint           200

Old rows behave correctly, not merely "migration completed" — including the case the M5 defect warned about, where a restored earlier position must not keep a later position's derived data.


P. Browser verification

Firefox 154.0.1, headless, WebDriver, against the built frontend served by the real backend on a fresh database. 16/16 passed, reproduced independently in this review.

Checks covered: normal generation; recent coherence; inspector opens; summary in use with coverage; memories listed; authority legible; provenance names the turn; token costs and reply reserve; Undo changes eligibility; Redo restores it; divergence reintroduces no abandoned memory; no abandoned summary row; abandoned memories retained not deleted; a background failure discoverable with its message; the "story itself is unaffected" note; no console errors.

Caveat. Browser check 10b ("divergence reintroduces no abandoned summary") passes because the harness never regenerates a summary on the new line. It therefore shares the blind spot described in §H and should not be read as independent confirmation that E03 holds.


Q. Local-only and security regression

PASS. No new network path.

_stream   re-checks endpoints.rejection_reason before the request      yes
complete  re-checks endpoints.rejection_reason before the request      yes
embed     re-checks endpoints.rejection_reason before the request      yes
policy refuses https://api.openai.com/v1/embeddings                    yes
policy allows http://127.0.0.1:11434/v1/embeddings                     yes

Embeddings, summarisation and memory extraction all go through the same OpenAICompatibleProvider; M6 added no HTTP client of its own. A source scan for cloud inference hosts, remote vector databases and cloud embedding services finds none outside endpoints.py, which names them in order to refuse them. Loopback binding, allowlist semantics and TLS verification are untouched. tests/test_local_only_surface.py passes.

Consistent with SECURITY-THREAT-MODEL.md §38 (local embedding models, no auto-download) and ADR 011.


R. Performance and scaling

PASS — no growth at all in the paths measured.

 51 actions,  8 memories,  3 summaries  →  20 queries, 5342 input tokens
231 actions, 38 memories, 14 summaries  →  20 queries, 5342 input tokens

Query count did not move while the story grew 180 actions, memories grew 30 and summaries grew 11. No N+1 on memory provenance, summary selection or history. The GET /derived listing resolves the eligible summary once rather than per row. Undo/Redo triggered 0 re-embeddings; returning to a lineage reuses the stored vectors.

Retrieval candidate sets are bounded by the lineage clause and memory_top_k; vectors are fetched only for the chosen rows.


S. Earlier-milestone regression check

Focused coverage over the interfaces M6 touches: 281 passed across test_narrative_state, test_head_cursor, test_save_points, test_memory_nodes, test_memory_retrieval, test_egress, test_bundle_v2, test_local_only_surface.

Full gates, on the staged tree:

backend suite     835 passed, 7 skipped, 2 warnings      (261s)
M5 baseline       794 passed, 3 skipped, 1 warning       (measured from a clean
                  export of b7005e6; the export shows 793/4 because it has no
                  built SPA, which skips one offline-assets test)
skips             7 = 3 M5 realistic + 3 M6 realistic + 1 live provider wiring;
                  all endpoint-gated, none reported as PASS
frontend lint     exit 0, 7 warnings — identical count at the M5 baseline,
                  demonstrated by running lint on the base export
frontend build    success
docker build      success

The one new warning is analysed in finding M6-F3.


T. Findings

ID Severity Title Blocks M6? Owner
M6-F1 high Abandoned summary content leaks into the active prompt via the un-anchored mirror column yes M6 corrective
M6-F2 medium No duplicate suppression; ranking is similarity-only, so F02's margin is one memory wide no M6 corrective (small) or M7
M6-F3 low M6 test fixture leaves the summariser on the default endpoint; suite attempts real localhost connections no M6 corrective (test-only)
M6-F4 low The E03 unit test and browser check share a blind spot: neither regenerates a summary after divergence no M6 corrective (test-only)
M6-F5 trivial derived_status reports embedding = ok for a pass that had nothing to do no documentation / M8

M6-F1 — abandoned summary content leaks (HIGH, blocking)

Requirement violated. CONTEXT-AND-MEMORY.md §11 ("A summary from abandoned history must never leak into active context"); acceptance test E03; BUILD-MILESTONES.md M6 scope ("summaries anchored to source turn ranges/lineage"). Named explicitly in the review brief as an M6 issue.

Evidence. §H above: real-model run (sentinel in the path-B prompt) and a deterministic reproduction isolating the route, with zero path-A actions on path B's lineage.

Cause. _update_story_summary seeds the model from adventure.story_summary, a campaign-global column with no lineage, which summaries.record rewrites on every write.

User impact. After undoing and taking a different path, the narrator is told about events from a story the reader abandoned — the exact confusion the milestone exists to prevent. It is silent: the summary row's provenance looks correct, because the row is correctly anchored.

Recommended direction (not implemented in this review): seed the summariser from summaries.current(db, adventure) — the eligible row — rather than from the mirror column, so a divergence starts from the last summary that is actually valid for the new line, or from nothing when none is.

M6-F2 — no duplicate suppression; similarity-only ranking (MEDIUM)

Requirement. CONTEXT-AND-MEMORY.md §22 requires that the same fact not be supplied repeatedly and that the builder "prefer the highest-authority concise representation"; §20 lists ranking factors beyond similarity.

Evidence. Two identical memories were both retrieved and both rendered ("duplicate suppression applied: False"). In the F02 fixture, four near-identical filler memories (0.79–0.82) outranked the one distinctive clue (0.61), which survived only because the default memory_top_k is 5 and it placed fifth. At 4 it is evicted and F02 fails.

User impact. On a long story with repetitive stretches, the memory budget can be consumed by redundant memories and the one that matters dropped. M7 will make this worse by adding imported material competing for the same budget.

Ranking is cosine + pinned only; no importance, entity overlap, recency or thread overlap.

M6-F3 — test fixture makes real network attempts (LOW)

tests/test_context_memory.py stubs embedding_provider but not summary_provider, with auto_summarize=True, so the post-turn task attempts a real connection to the default endpoint during the suite. This is the source of the one new warning (RuntimeWarning: coroutine 'connect_tcp…try_connect' was never awaited) — an abandoned httpx connection coroutine when the TestClient event loop closes. Test hygiene, not a product defect; the production loop does not close mid-task.

M6-F4 — the E03 tests cannot see the E03 defect (LOW)

Both the unit test and browser check 10b verify only that the old row is ineligible. Neither regenerates a summary after diverging, which is where the content leak occurs. Any corrective work should add a test that plays past the summary interval on the new line and asserts the sentinel's absence.

M6-F5 — misleading "ok" for a no-op pass (TRIVIAL)

When there is nothing to embed, _embed_pending does nothing and the kind is recorded ok with a fresh last_success_at. Harmless, but a reader could read it as "embedding is working" when it has never run.


U. Non-blocking debt

  • M7: imported knowledge absent, as intended; the inspector section was left unbuilt rather than stubbed.
  • M8: the Insights additions are functional, not designed.
  • M9: derived rows are not carried in an export bundle, so an imported campaign starts with no summaries or memories and rebuilds them. Authoritative history is unaffected. Not tested here beyond noting it.
  • M11: long-context evidence is a bounded fixture (up to 251 actions), not the M01 100-turn release campaign.

V. Planning-document recommendations

No active planning document was edited by this review.

Document Recommendation Why
SPECIFICATION.md NO CHANGE M6 implements it; nothing contradicted.
CONTEXT-AND-MEMORY.md CHANGE Two reasons. §22 (duplicate suppression) and the non-similarity half of §20 are not implemented — the plan is not wrong, the implementation is incomplete, and the gap should be recorded rather than the requirement softened. Separately, the "As implemented (M6)" note added under §11 currently claims lineage safety that finding M6-F1 shows is not yet achieved end to end; it should not stand unqualified until M6-F1 is fixed.
STORY-BRANCH-SEMANTICS.md NO CHANGE M6 consumes §32/§33 lineage rules; it does not change them.
DATA-MODEL.md NO CHANGE §16A accurately records the implemented tables. Add the mirror-column relationship only if M6-F1 is fixed by changing it.
TECHNICAL-DESIGN.md CHANGE (small) The M6 section should state that the summariser's input is lineage-scoped, once it is — that is the invariant M6-F1 breaks, and it is currently unstated.
BUILD-MILESTONES.md CHANGE at closeout Correctly says "implemented, awaiting review". Should record the E03 corrective work and the duplicate-suppression debt when M6 closes out.
V1-ACCEPTANCE-TESTS.md CHANGE The E03 result note currently reads PASS. On this evidence it is not yet satisfied end to end and should say so until corrected. F02's note should record the ranking fragility. Do not weaken any pass condition.
SECURITY-THREAT-MODEL.md NO CHANGE M6 added no network path; §38 and ADR 011 hold.
README.md / DEVELOPMENT.md NO CHANGE No user-facing or developer-workflow fact changed.
VERSION.md NO CHANGE No new decision was recorded; see the ADR row.
ADRs NO NEW ADR Every M6 choice implements a decision already settled — summary/source association (§11), authority and application-owned classification (§14–15), output reserve (§32), the M2 observability requirement. An ADR would record no open decision. If the M6-F1 fix changes what adventures.story_summary is, that would be worth an ADR.

W. Implications for M7

M7 builds the imported Canon / Reference / Inspiration library on this infrastructure. What it inherits:

Ready to build on:

  • A single eligibility chokepoint. lineage.Path.clause filters actions, memories and summaries alike. M7's imported knowledge is deliberately not lineage-scoped, and the builder now has one obvious place to express that.
  • Provenance and per-component token cost are first-class in the context report and the inspector, so extending F06 to files and chunks is an addition, not a redesign.
  • An explicit protected/elastic budget with a reply reserve, giving imported reference and inspiration material a defined place as low-priority elastic content (CONTEXT-AND-MEMORY.md §31).
  • Authority is already a working concept, with application-owned classification and prompt-level marking that M7's Canon/Reference/Inspiration tiers can extend rather than reinvent.
  • Derived-work failure is observable, so import and index failures can reuse derived_status instead of inventing another mechanism.

M7 must not build on:

  • M6-F1. The summariser's input is not lineage-scoped. M7 will add more derived text to the same prompt; layering imported knowledge on top of a summary that can carry abandoned content would make the resulting context harder to reason about and harder to debug. Fix before M7 starts.
  • M6-F2. Imported chunks will compete for prompt budget with story memories. Without duplicate suppression or importance weighting, adding a second corpus to a similarity-only ranker will make the F02 fragility materially worse.

X. Verdict

PASS WITH CORRECTIVE WORK REQUIRED

Question Answer
Is M6's Definition of Done met? Not fully. "Lineage-safe" fails for summary content (M6-F1); authority-aware, local, inspectable and bounded are all met.
Are F01–F08 satisfied at the level M6 owns? Seven of eight. F02 passes at the production default but is fragile (M6-F2). F05 is complete for M6-owned components.
Is E02 demonstrated end-to-end? Yes, with real embeddings and positive controls.
Is E03 demonstrated end-to-end? Yes — and it fails.
Can abandoned memory appear in an active prompt? No.
Can abandoned summary content appear in an active prompt? Yes. M6-F1.
Is heuristic memory visibly non-authoritative? Yes, in the prompt, the API and the browser; it creates no state.
Is prompt growth actually bounded? Yes, measured: 251 actions produce the same tokens as 71.
Is output reserve enforced? Yes, and an impossible budget fails clearly instead of overflowing.
Are memory/summary failures non-fatal? Yes. Story, state and head all survive; no half-written derived data.
Are background failures observable? Yes. API, inspector, browser and log, from a real socket failure.
Was a real provider-construction path exercised? Yes, unmocked, plus the full derived pipeline against real local models.
Did M6 preserve the local-only boundary? Yes. No new client, no new host, policy enforced on all three paths.
Did any M1–M5 behaviour regress? No. 281 focused regression tests pass; full suite green; lint warnings identical to baseline.
Should the project proceed to M7? Not yet. Fix M6-F1 first.
What must be corrected before M7? M6-F1 (blocking). M6-F2 strongly recommended, since M7 makes it worse. M6-F3 and M6-F4 are cheap and should ride along.

Y. Evidence appendix

Check Method Result
Backend suite pytest tests/ -q, staged tree 835 passed, 7 skipped
M5 baseline git archive b7005e6 → clean export → pytest 794 passed, 3 skipped (793/4 in the export, +1 skip for the unbuilt SPA)
Frontend lint / build npm run lint, npm run build exit 0 (7 warnings, same as baseline); build ok
Container docker build success
E02 real embeddings, sentinel, positive control then divergence sentinel absent on path B; rows retained
E03 real models, then deterministic isolation sentinel present on path B
F02 real embed + extractor, 57 actions, 14 included passes at top_k=5; clue ranks 5/5
F03/F04 builder measurements at 11 / 71 / 251 actions 5,208 tokens at both 71 and 251
F07 heuristic memory through the real pipeline labelled; no fact created
F08 / M2 gate endpoint → dead loopback port story intact; failure served by API
Provider wiring real factories, no mocks; plus full real-model pipeline 5 memories, 2 summaries, all ok
Restart two OS processes, one DB file identical state and provenance
Migration genuine pre-M6 DB, stamp 88 → 91 backfill correct; old positions behave
Performance statement counter, 51 → 231 actions 20 queries both times
Browser Firefox 154.0.1 headless 16/16
Security policy checks on all three request paths + source scan no new path

Z. Final repository state

HEAD      b7005e6fddec337f940749455739a60f0c6cb54f
branch    m6-context-memory

git status --short — 29 staged entries, nothing unstaged, nothing untracked:

A  backend/app/derived.py
A  backend/app/summaries.py
A  backend/tests/test_context_memory.py
A  backend/tests/test_context_performance.py
A  backend/tests/test_context_realistic.py
A  backend/tests/test_provider_wiring.py
A  planning/reports/M6-IMPLEMENTATION-REPORT.md
M  backend/app/context/__init__.py
M  backend/app/context/builder.py
M  backend/app/memorybank.py
M  backend/app/migrations.py
M  backend/app/models.py
M  backend/app/routers/adventures/crud.py
M  backend/app/routers/adventures/insights.py
M  backend/app/routers/adventures/turns.py
M  backend/tests/test_history_window.py
M  backend/tests/test_length_hint.py
M  backend/tests/test_memory_nodes.py
M  backend/tests/test_memory_retrieval.py
M  backend/tests/test_prompt_caching.py
M  backend/tests/test_tree_migration.py
M  frontend/src/pages/Play/panels/InsightsPanel.jsx
M  frontend/src/styles/play.css
M  planning/BUILD-MILESTONES.md
M  planning/CONTEXT-AND-MEMORY.md
M  planning/DATA-MODEL.md
M  planning/TECHNICAL-DESIGN.md
M  planning/V1-ACCEPTANCE-TESTS.md
R  planning/reports/M5-IMPLEMENTATION-REPORT.md -> planning/archive/milestone-reports/M5-IMPLEMENTATION-REPORT.md

Files changed by this review: one — planning/reports/M6-IMPLEMENTATION-REPORT.md, rewritten from the build-time document into this review report. No application code, test or planning document was modified. All probes, databases, exports and browser artifacts were created outside the repository and are not staged.

M7 was not started.



ADDENDUM — M6 Corrective Pass and Closeout

Date: 2026-09-06 Branch: m6-context-memory Base: b7005e6 HEAD: b7005e6 (no commit made) Status of the review above: unchanged. Nothing in it has been edited or withdrawn. The first M6 implementation reported E03 as passing and it was not; that record stays visible.

Result: M6 ACCEPTED — READY TO COMMIT

A1. Disposition of the findings

ID Severity Disposition
M6-F1 high, blocking Fixed. Summary generation is seeded from summaries.current. §A2.
M6-F2 medium Fixed. Redundancy suppression before the retrieval cut. §A4.
M6-F3 low Fixed. The unit fixture stubs both derived providers; the warning is gone. §A6.
M6-F4 low Fixed. A regression that fails against the pre-corrective code, plus a browser scenario that regenerates a summary. §A3.
M6-F5 trivial Fixed. Derived work that had nothing to do reports idle, not ok. §A7.

A2. M6-F1 — the fix

One line of behaviour, in _update_story_summary:

# before
current = adventure.story_summary.strip()          # campaign-global, no lineage

# after
eligible = summaries.current(db, adventure)        # the same question the
current = eligible.text.strip() if eligible else ""  # context builder asks

The invariant this establishes, now recorded in TECHNICAL-DESIGN.md:

Both summary eligibility and the prior-summary input to the summarizer are lineage-scoped.

Where no eligible summary exists at the current position, generation starts from none. Nothing is deleted, no summary is cleared on Undo or divergence, and the output row is not filtered after the fact — the correction is at the input, which is where the contamination entered.

The fate of adventures.story_summary

Every reader and writer was inventoried. The only unsafe read was the one above; all others are display or transport — the Plot panel edits it, the export bundle carries it, the API serialises it.

Decision: option 2 — a convenience mirror, never authoritative input. It is the smallest safe design: no migration, no schema change, no user-visible loss. Two things make it honest rather than merely tolerated:

  • it is kept in step both when a summary is written (summaries.record) and when the head moves (attempts.restore_state, the single chokepoint every Undo, Redo, take switch and Save Point restore passes through), so what the Plot panel shows is the summary the narrator is actually given;
  • models.py and summaries.py now say plainly that it carries no lineage and that nothing authoritative may read it, so the next person cannot reintroduce the defect by accident.

No ADR. The architectural meaning of the column did not change — it was always meant to be a mirror; the defect was that one caller treated it as a store.

A3. M6-F4 — the regression that can see the defect

test_e03_a_summary_generated_after_divergence_carries_no_abandoned_content walks the eleven steps the closeout brief specifies. It was written before the fix and confirmed to fail against the reviewed implementation:

$ pytest ...test_e03_a_summary_generated_after_divergence...   (pre-fix)
  assert E03_SENTINEL in summary_a          PASS   positive control
  assert summary_b_row["id"] != path_a_id   PASS   a new summary was generated
  assert carried_over == 0                  PASS   no path-A action on B's lineage
> assert E03_SENTINEL not in summary_b      FAIL   <-- the defect

After the fix it passes, including the stronger assertion that the summariser was never even offered abandoned prose — seeds are captured and checked, so a future filter-the-output "fix" would not satisfy it.

The browser suite gained a dedicated E03 scenario on its own campaign (checks 13a–13e): a neutral opening, 18 sentinel turns, a real summary, undo below the sentinel, 18 turns on the new line, a regenerated summary, then the prompt. Two fixture facts are asserted before the negative — that path A's summary really carried the sentinel, and that no sentinel-bearing turn remains at or below the head — so the check cannot pass vacuously.

A4. M6-F2 — redundancy suppression

The threshold was measured, not chosen. Against the configured local embedding model, on the review's own fixture plus a set of deliberately tricky pairs:

redundant pairs (near-identical filler)     cosine 0.938 – 0.996
distinct pairs (different facts)            cosine 0.349 – 0.906

REDUNDANT_SIMILARITY = 0.93 sits in that gap.

The same measurement ruled out the obvious alternative. Word overlap fires hardest on exactly the pair that must not be merged — "Mara promised to return before dawn" against "Aldric promised to return before dawn" shares 71% of its words and means something else — and is weakest (27%) on filler that plainly repeats itself. Wording is a poor proxy for sameness of fact; the embedding is a better one. That is why the implementation uses vectors alone, and why the measurement is recorded beside the constant.

Two rules bound the suppression:

  • authority is never crossed — a heuristic memory can never suppress an accepted_story one or the reverse;
  • the highest-ranked statement survives, with its provenance, and the count of suppressed candidates is reported so "why is that memory not here?" has an answer.

Greedy over the ranked list, stopping once the budget is filled, so the cost is bounded by top_k rather than by the size of the bank.

Result on the review's own fixture, real embedding model, 57 actions:

top_k   clue in prompt   retrieved   considered   suppressed
  5          yes             2            5           3
  4          yes             2            5           3
  3          yes             2            5           3

Before the fix the clue placed fifth of five and was evicted at top_k=4. It is now retrieved with two slots to spare, and the budget is no longer spent on four copies of the same sentence. top_k=4 was tested because the review showed F02 succeeding only because the default left one slot; it no longer depends on that.

Ranking itself is unchanged — cosine plus an explicit pin. The further factors CONTEXT-AND-MEMORY.md §20 contemplates remain unimplemented and are recorded there as future work rather than quietly claimed.

A5. E03 and E02, re-run independently

E03, deterministic: the review's own reproduction, which isolated the contamination route, now shows path B's summary as MEM[dry road] only, with the seed free of the sentinel and zero path-A actions on path B's lineage.

E03, real local summariser and embedder:

path A   summary id 2, branch 1, depth 30, source 15–30   sentinel present
         POSITIVE CONTROL: sentinel in the active prompt  yes
undo 21 steps to head (1,0); diverge; play on; run the real derived pass
path B   summary id 3, branch 2, depth 16, source 1–16
         "The dry, silent road continues, leading the protagonist forward…"
         sentinel in the new summary                      NO
         sentinel anywhere in the active prompt           NO
         no path-A action on path B's lineage             confirmed
retention  7 memories, 3 summaries on disk; 1 eligible, 2 retained-but-ineligible

E02, unchanged and re-verified: sentinel retrievable on the valid lineage; ineligible after Undo below its source; eligible again after Redo with 0 re-embeddings; absent from the active prompt after divergence; retained on disk throughout.

A6. M6-F3 — no accidental network calls

test_context_memory.py stubbed the embedder but not the summariser, so with auto_summarize=True every turn built a real summariser against the default endpoint. Both are stubbed now. The RuntimeWarning: coroutine 'connect_tcp…try_connect' was never awaited is gone: 2 warnings → 1, the remaining one being the pre-existing Starlette deprecation notice.

The tests that deliberately exercise real provider construction — test_provider_wiring.py, and the realistic-context tests — are untouched and still mock no factory.

A7. M6-F5 — idle

derived_status.status is now ok (did work), idle (ran, nothing pending) or failed, and last_success_at is only stamped for real work. Observed in the failure probe: embedding idle with a null success time, beside memory failed and summary failed. Nothing else changed; this is not a status redesign.

A8. What the corrective pass had to leave alone, and did

Two test fixtures stored the same vector in every memory (test_memory_nodes.py). That was harmless until redundancy suppression existed, at which point four identical vectors are four copies of one statement and the lineage assertions could no longer be read. The fixtures now use distinct directions at equal angle from the query, so ranking between them is unchanged and the tests measure what they are about. The lineage assertions themselves were not weakened.

This is worth recording because it is the one place the corrective pass changed a test that was passing: the change removes an unintended coupling, it does not relax a requirement.

A9. Verification

backend suite      836 passed, 7 skipped, 1 warning        (285s)
                   was 835 / 7 / 2 at review; +1 is the new E03 regression,
                   and the lost warning is M6-F3
skips              unchanged: 3 M5 realistic + 3 M6 realistic + 1 live wiring,
                   all endpoint-gated, none reported as PASS
frontend lint      exit 0, 7 warnings — identical to the M5 baseline
frontend build     success
docker build       success
browser            22/22 (was 16/16; +6 for the new E03 scenario)

Preserved M6 successes, re-checked after the corrective changes:

Area Result
F01 recent coherence, F03 bounded, F04 reserve unchanged; 231 actions still 5,326 tokens
F05 inspector, F06 provenance, F07 heuristic unchanged
F08 failure non-fatal and observable re-run against a dead endpoint: story, state, head intact; failure served
Save Point restore, Undo/Redo lineage unchanged
Process restart re-run: two PIDs, identical state and provenance
Pre-M6 migration re-run: stamp 88 → 91, backfill anchored, restored position correct
Query counts 51 → 231 actions: 20 queries both times, unchanged
Local-only boundary unchanged; no new client, host or path
M1–M5 focused regressions green

memories_used in some probes drops (5 → 2) purely because redundant candidates are now suppressed; the content that mattered is still present.

A10. Planning documents changed

Document Change
CONTEXT-AND-MEMORY.md §11 rewritten: lineage safety has two halves, and the input half is what M6-F1 was. §20 records that ranking is similarity plus a pin and the other factors are not implemented. §22 records what redundancy suppression does, the measurement behind the threshold, why lexical overlap was rejected, and that cross-layer duplication is still open.
TECHNICAL-DESIGN.md The corrected summary invariant, why anchoring the output alone is insufficient, the role of adventures.story_summary, and the retrieval/redundancy behaviour.
DATA-MODEL.md derived_status.status gains idle; the summary entry records that rows are also the input to the next round.
V1-ACCEPTANCE-TESTS.md E03 rewritten with the full history (baseline fail → anchored-row fail → corrected) and the required test shape, so a future test cannot regress to checking old-row eligibility. F02 records the one-slot margin the review found and the corrected result at top_k 5/4/3. Neither pass condition weakened.
BUILD-MILESTONES.md M6 marked complete and accepted; the review findings and corrective work recorded; retrieval-ranking and cross-layer duplication recorded as debt; M7 explicitly authorized.
VERSION.md New v2.7 entry covering M5 and M6 closeout, including that M5's closeout never added one, and what the package learned from M6.
planning/README.md Current state M1–M6 accepted, next M7; the stale M5 handoff text replaced with the M6 debt that actually informs M7.

No ADR was added. Every choice implements a decision the package had already settled; the one candidate — a change in the meaning of adventures.story_summary — did not occur, because the column's meaning was always "mirror".

A11. Final verdict

Question Answer
Is M6's Definition of Done met? Yes. Lineage-safe (both halves), authority-aware, local, inspectable, bounded.
Are F01–F08 satisfied at the level M6 owns? Yes, all eight; F02 no longer by a one-slot margin.
Is E03 demonstrated end-to-end? Yes — deterministic, real-model, and browser, each with positive controls.
Can abandoned summary content reach an active prompt? No.
Can abandoned memory reach an active prompt? No.
Are background failures observable? Yes, from a real socket failure.
Did any M1–M5 behaviour regress? No.
Is M7 safe to begin? Yes. Both blockers named in §W are closed.

M6 ACCEPTED — READY TO COMMIT

M7 was not started.