# M5 Implementation Review — Typed Narrative State **Reviewed:** 2026-09-03 / 2026-09-04 **Branch:** `m5-narrative-state` **Base commit:** `62a997f` (M4 closeout) **Subject of review:** the staged, uncommitted M5 working tree **Verdict:** PASS WITH CORRECTIVE WORK REQUIRED --- ## A. Scope This reviews the M5 milestone: replacing the RPG relative-delta world-state mechanism with genre-neutral typed narrative state per ADR 010, and making that state the authoritative thing the narrator is told and the reader is shown. It is a review, not a closeout and not implementation. No application code was changed. Defects found are recorded here rather than fixed, and the working tree is exactly as the implementation left it. M6 was not started. ## B. Method and independence The M5 implementation summary was treated as a claim, not as evidence. Every acceptance statement below rests on something run during this review: - the full backend suite, re-run from scratch; - purpose-built probes that drive the real FastAPI application over its real HTTP surface against a real SQLite file, with a scripted narrator; - a real Firefox 154.0.1 driven over WebDriver against the real built frontend served by the real backend; - the real model endpoint on the trusted LAN, for the tests that are skipped without one; - a real `docker build`. Where a check could only be done with a mock, it is labelled as such. The atomicity check (L01) specifically was **not** done with mocks — the failure was induced at the storage layer. ## C. Repository state ``` branch m5-narrative-state HEAD 62a997f staged 45 files changed, 4979 insertions(+), 364 deletions(-) 12 added, 33 modified unstaged / untracked none ``` The tree was in this state at the start of the review and is in this state at the end of it. No secrets, databases, or browser profiles are staged. Upstream ancestry is intact and `LICENSE` is unmodified. The new package is `backend/app/narrative/` — `model.py`, `events.py`, `validate.py`, `apply.py`, `extract.py`, `render.py`, `store.py` — plus `backend/tests/test_narrative_state.py` and `test_narrative_realistic.py`. ## D. What M5 changed The relative-delta protocol (`{"player.hp": -15}`) is gone from the prompt path. In its place is a 14-event allowlist of explicit, **absolute** narrative events (`create_entity`, `set_entity_attribute`, `set_possession`, `set_current_location`, `add_fact`, `invalidate_fact`, `open_story_thread`, …) carried in a fenced block the extraction pass strips out of the prose. State is stored twice on purpose, per DATA-MODEL §17: validated events in `state_events` for audit, and a whole-document snapshot per position in `actions.narrative_state_after` for restore. The live document sits in `adventures.narrative_state`. The legacy world-state module still exists and is still exercised by its own tests, but it no longer reaches the assembled prompt. That demotion is verified (§I below), not assumed. ## E. ADR 010 conformance Conformant, and the central ambiguity ADR 010 exists to remove is genuinely removed. There is no code path where one numeric value can be read as either an absolute or a delta: `apply.py` is an explicit `if/elif` chain over event types, each writing a named field, with no arithmetic on prior values. The state document is genre-neutral. A scan of the new package for RPG and fantasy vocabulary (`dungeon`, `hp`, `mana`, `quest`, `loot`, `potion`, …) returns nothing but one citation of AI-DnD in a module docstring explaining what ADR 010 replaced. ## F. Validation and the security boundary `validate.py` is five layers — envelope, allowlist, schema, referential, semantic — and the allowlist is checked before anything else touches the payload. This is the right order: an unknown event type is rejected before its fields are read. **H05 holds.** Hostile proposals were fed through the real turn pipeline: `execute_shell`, an `eval` event carrying `__import__('os').system(...)`, an event keyed `event_type` instead of `type` to test envelope confusion, an entity named `__class__`, and a story-thread id of `../../etc/passwd`. No marker file was created, no event was accepted, and the proposal was recorded with its rejection reasons. Nothing in the event path reaches the filesystem, a subprocess, or an attribute lookup on a Python object. Cross-campaign references are refused: an event naming an entity that exists in a *different* campaign is rejected as `unknown_reference` rather than resolved. ## G. Head, lineage, and restore **Restore does not replay the event log.** This was proven, not inferred: the `state_events` table was renamed away mid-campaign, and undo and redo both still returned 200 and moved the state correctly. Restore is a snapshot row lookup, as ADR 012 requires. Query counts for undo were also measured at two depths and did not grow. **Lineage isolation holds (E01, E04).** State established on a line that is then abandoned does not appear in the new line's state document, and does not appear in the next turn's assembled prompt. ## H. Atomicity (L01) Tested by induced failure, not by mocks: the `state_events` table was renamed away underneath a live state-writing turn. ``` before head (1,2) mara.cloak = red state_events taken away turn OperationalError: no such table: state_events after head (1,3) mara.cloak = red accepted narration rows for the failed turn: 0 previous story reachable: yes recovery after the table returns: next turn 200, state advances normally ``` No half-written state, no accepted narration without it, no inaccessible history. The head advanced by exactly one, onto the player's own submitted text — which the L01 note explicitly defines as correct under A05, not as a half-advanced head. **L01 PASS.** ## I. Context assembly The narrator is given the state document rendered as prose, not as JSON, and the legacy world-state block never appears. Verified by inspecting stored `context_snapshot` rows from real turns. One defect here, and it is the mechanism behind Finding 4: the **history** section replays each past narrator turn's raw text *including its ```state fence*. So the prompt simultaneously instructs the model that prose must not contain protocol, and shows it several past examples where prose does. ## J. Manual corrections (C04) A correction lands, is stored with `manual_correction` authority, is shown in the inspector marked as the reader's own, and is visible in the audit view. But it is only half-applied to the narrator's context. With the fact `mara-knows / "knows where the key was found"` withdrawn by an explicit correction: ``` state section contains the withdrawn fact: False ← correct full prompt contains the withdrawn assertion: True ← in the history replay any statement anywhere that it was corrected: False ``` The state section drops it; the history section replays the original `add_fact` verbatim; and nothing in the prompt tells the model the assertion was withdrawn. The narrator is left holding the contradicted claim with no indication it is contradicted. **C04 PARTIAL.** ## K. Export / import Round-tripping a campaign preserves the state document, campaign canon, save points, head depth (including an undone head, so I07 holds), every per-node snapshot (7 of 7), and correction *authority* — an imported corrected fact is still marked `manual_correction`. It does not preserve the audit trail. `state_events` and `state_proposals` are not in the bundle, so an imported campaign reports **0 audit events**. The correction's effect survives; the record of who made it and why does not. This is the round-trip half of C04's "auditable" condition. ## L. Migration from pre-M5 A genuine pre-M5 database was constructed (M5 tables dropped, columns removed, `user_version` stamped back) and then migrated. Migration itself is clean: the campaign opens, all rows and save points survive, and new M5 turns play normally. The defect is at the seam. Pre-M5 nodes have no snapshot, and `attempts.restore_state` treats a NULL snapshot as "leave the live state alone". So restoring to an old Save Point moves the transcript back without moving the state: ``` restore to the pre-M5 Save Point at depth 2 -> head (1,2) state left standing describes depth 6 (the M5 turn's entities) ``` The reader is at depth 2 and the state panel describes depth 6. §9 of the M5 brief forbids exactly this. ## M. Performance One regression, in the action-list read path. `paging.py:ACTION_LIST_COLUMNS` does not include `models.Action.state_changes`, so every row lazy-loads it individually: ``` 11 actions on the page -> 24 queries, 13 fetching state_changes one row at a time 51 actions on the page -> 64 queries, 53 fetching state_changes one row at a time ``` The comment directly above that list warns against this exact mistake for `world_delta`, in these words: *"Omitting it saves no bytes. It converts one bulk read into one lazy load per row."* The new column was added to the model without being added to the list. ## N. Frontend `npm run lint` exits 0. Every warning it prints is pre-existing at `62a997f` in files M5 did not touch. `npm run build` succeeds. `docker build` succeeds and the image contains `backend/app/narrative/`; M5 added no dependency and needed no packaging change. The stale-panel bug reported during implementation was fixed in `saveEdit` by bumping `stateKey`. That is the correct fix for that mutation, and the panel's key (`actions.length` + `stateKey`) does now cover every state-moving mutation: undo/redo, take switching, save-point restore, correction, delete. It is per-mutation rather than a single shared invalidation boundary, so a future mutation must remember to bump it — worth noting, but not a defect today. ## O. Real-browser verification Firefox 154.0.1, headless, real backend, fresh database. **16 of 16 checks passed**: inspector starts empty; shows what a turn established; groups by category; shows possession with its owner; follows a later turn, Undo, Redo, and a Save Point restore; a manual correction lands, is marked as the reader's own, survives another turn, and is explained in the audit view; a narrator edit re-derives state; edited prose carries no protocol; no console errors. That suite edits the **last** narrator turn, so it does not exercise the case in Finding 1. A second browser probe was written for that case, and it reproduces the defect visibly — see below. ## P. Realistic-model behaviour Run against one small local model (`qwen2.5:3b-instruct`) on the trusted LAN. All 3 otherwise-skipped tests pass. Over 6 turns: ``` accepted 3 | partially_accepted 1 | unparseable 2 events rejected 1 (unknown_reference) ``` Two of six turns produced a state block the pipeline could not parse. The pipeline degrades correctly — the prose still lands, the state is left alone, and the proposal is recorded `unparseable` — which is why the tests pass. But a 33% unparseable rate on a 3B model is a real datum for the prompt work ahead. This is **one** model. It is not evidence about models in general, and no broader claim is made from it. ## Q. Test suite ``` 764 passed, 3 skipped, 1 warning (197s) ``` All three skips are `test_narrative_realistic.py`, skipped for missing `AIDND_TEST_ENDPOINT` / `AIDND_TEST_MODEL`. With those set, all three pass (§P). There are no other skips and no xfails. Rollback assertions were **moved, not deleted**. `test_worldstate_integration.py` went from 6 tests to 4 and was rewritten as a demotion test, with the removed tests and the reason for each recorded in the module docstring. Their subject matter is now covered by `test_narrative_state.py` (67 tests). Two gaps: - `test_state_revert.py` — the dedicated unit test for `attempts.restore_state` / `snapshot_outcome` — has **zero** references to `narrative_state`. It still tests only the legacy column. The NULL-snapshot rule that produces the migration defect in §L lives in exactly the code this file covers, and was not extended to the column M5 made load-bearing. - No test covers editing a turn that has later **visible** story. That is the hole Finding 1 sits in. ## R. Genre neutrality (J01–J03) The backend is neutral. The new package is clean, and a science-fiction campaign plays through the same events with no fantasy vocabulary anywhere in the path. Remaining hits in the backend are either citations of AI Dungeon as the design being followed, the frozen wire-format string `ai-dnd-adventure-v2` (which must not change), or the demoted legacy world-state module's own examples. The frontend is not. `frontend/src/pages/ScenarioEditor.jsx` is still routed at `scenarios/:id` and still tells the user, in user-facing prose, that the app "will track them each turn — HP, mana, a raised alarm, quest objectives", with a JSON placeholder of `player.hp`, `npcs.gwen.stats.trust`, and `milestones`. That is the surface ADR 010 replaced, still describing the replaced design to the user. J03 asks that genre be configuration; this screen hard-codes one genre's vocabulary as the explanation of how state works. ## S. Findings ### Finding 1 — Editing a turn with later visible story breaks the state/head invariant — **SERIOUS** `STORY-BRANCH-SEMANTICS.md` §14A refuses an in-place edit only when descending story is **off screen**. An edit with a *visible* future is therefore permitted, and `_reevaluate_state()` in `routers/adventures/actions.py` then calls `narrative.store.set_current()` with the edited node's re-derived state — while the head stays at the tip. The comment there asserts the case cannot arise: *"If the head sits further along, the refusal above already ran — this node has no off-screen future."* That is incorrect. The refusal covers off-screen futures only. Reproduced in a real browser, on a four-turn campaign, by editing the first narrator turn through the UI's own ✎ control: ``` state panel BEFORE : CHARACTERS Aldric, Mara | LOCATIONS the Crooked Lantern ITEMS the silver key | POSSESSIONS held by Mara FACTS Mara knows where the key was found OPEN STORY THREADS Reach the Old Abbey state panel AFTER : "Nothing established yet." transcript : all four turns still on screen head : still (1,7), the tip ``` The database shows the invariant broken directly — the head's own snapshot is intact while the live column it is supposed to agree with is empty: ``` live narrative_state entities 0 facts 0 threads 0 snapshot at depth 7 (head) entities 4 facts 1 threads 1 snapshots at depths 2..6 entities 4 ← stale: describe prose that no longer exists ``` Three consequences: the reader sees a full transcript over an empty state panel; the next turn's prompt is built from the rewound state; and Undo/Redo restores the stale downstream snapshots, resurrecting state whose originating prose was edited away. An earlier API-level probe showed the milder form of the same bug — after editing a "red cloak" turn to "green", undoing back restored `cloak: red` while the visible prose said green, the prose/state disagreement §15 forbids. **D10 is not satisfied.** Its M5 pass condition is *"downstream state is re-evaluated"*. Downstream state is not re-evaluated; live state is rewound and downstream snapshots are left stale. Separately, §15 lists five requirements for this workflow and only 1–3 are implemented: there is no new continuation and the original narration is overwritten in place, so *"old version/future remains retained"* also fails. The UI offers ✎ (edits in place, no fork) and ⑂ (forks, but regenerates rather than using the corrected text) — **no available path satisfies §15 fully.** ### Finding 2 — N+1 on the action list — **MODERATE** `state_changes` is missing from `ACTION_LIST_COLUMNS`; 51 rows cost 53 extra queries. Details and the warning comment that predicted it: §M. ### Finding 3 — Restoring to a pre-M5 node leaves M5 state standing — **MODERATE** NULL snapshot is treated as "leave the live state alone", so head and state disagree at any migrated position. Details: §L. Same broken invariant as Finding 1, different cause. ### Finding 4 — A withdrawn fact survives in the prompt's history replay — **MODERATE** The state section drops it, the history replay keeps it verbatim, and nothing says it was corrected. Details: §I, §J. ### Finding 5 — Export/import loses the audit trail — **MINOR** `state_events` and `state_proposals` are not bundled; an imported campaign shows 0 audit events. Effects survive, records do not. Details: §K. ### Finding 6 — Extraction strips legitimate prose — **MINOR** The fence stripper removes content it should leave alone: ``` CUT a bracketed aside that merely mentions the state block CUT a story containing a legitimate ```json fence in : 'She typed it out:\n```json\n{"name": "Mara"}\n```\nThen closed the terminal.' out: 'She typed it out:\n\nThen closed the terminal.' KEPT a story containing a ```python fence ``` A story about programmers loses its code. The `python` fence surviving shows the rule is specifically over-broad on `json`. ### Finding 7 — The RPG schema editor still describes the replaced design — **MINOR** Details: §R. ### Finding 8 — Snapshot/restore unit tests were not extended to the new column — **MINOR** Details: §Q. ## T. Acceptance criteria | ID | Criterion | Result | Basis | |----|-----------|--------|-------| | C01 | Campaign canon preserved | PASS | canon survives turns, restore, and round-trip | | C02 | Possession state | PASS | `set_possession` moves ownership; shown with owner in UI | | C03 | Character knowledge not invented | PASS | `knows()` gated on recorded facts; unknown refs rejected | | C04 | Manual state correction | **PARTIAL** | reflected in state and auditable live; but the withdrawn fact survives in the history replay (F4) and the audit is lost on import (F5) | | C06 | Structured state matches accepted narration | PASS | absolute events only, no delta/absolute ambiguity; malformed proposals rejected, recorded, prose still lands | | D01 | Undo one turn | PASS | snapshot restore, verified in browser and API | | D04 | Redo | PASS | as above | | D05 | Redo invalidated by new continuation | PASS | abandoned line isolated (E01/E04 probes) | | D06 | Retry narrator response | PASS | unchanged from M3/M4, re-run green | | D07 | Select prior retry take | PASS | take switch restores that take's state | | D08 | Retry does not delete prior take | PASS | suite + browser | | D09 | Edit earlier user input | PASS | within the same mechanism as D10; no visible-future case in the fixtures | | D10 | Edit narrator output | **FAIL** | M5's own pass condition — downstream state re-evaluation — is not met; state is rewound, downstream snapshots left stale (F1); retention condition also unmet | | D11 | Named checkpoint | PASS | M4 machinery, re-verified in browser | | D12 | Restore checkpoint | PASS | state moves with the restore | | D13 | Restore does not delete later history | PASS | later rows retained and reachable | | D14 | Delete checkpoint | PASS | suite | | E01 | Abandoned future cannot affect active state | PASS | probe: no leak into state or prompt | | E04 | Scene state is lineage-safe | PASS | same probe | | H05 | Invalid state event rejected | PASS | hostile payloads produced no filesystem effect and no accepted events | | I01 | Export campaign | PASS | bundle carries state, canon, checkpoints, head | | I02 | Import exported campaign | PASS | 201, state and authority intact | | I03 | Branch/disposable history export | PASS | snapshots 7 of 7 | | I04 | Checkpoint export | PASS | save points round-trip | | I07 | Export/import preserves an undone active head | PASS | `can_redo` preserved in the copy | | J01 | Science-fiction campaign | PASS | plays through the same events, no genre coupling | | J02 | Generic entity support | PASS | `entity_type` is free-form | | J03 | Genre profiles are configuration | **PARTIAL** | backend neutral; the scenario editor still hard-codes HP/mana as the explanation of state (F7) | | L01 | Atomic turn commit | PASS | induced storage failure, not mocks — §H | | L02 | State reconstruction | PASS for undo/redo; breaks after an edit (F1) | | L03 | Checkpoint reconstruction after restart | PASS | M4's real-process-boundary test reads the M5 state document | ## U. Documentation `planning/` was updated in step with the code, and `DATA-MODEL.md` §17 correctly describes the hybrid that was actually built. Two corrections are needed: - `STORY-BRANCH-SEMANTICS.md` §14A should no longer say the refusal is "replaced by" M5's re-evaluation. It was not replaced; it was retained for off-screen futures, and the visible-future case it does not cover is unhandled. - The incorrect comment in `_reevaluate_state()` quoted in Finding 1 should be removed rather than reworded — it asserts an invariant the code does not hold. ## V. Planning-document recommendations 1. **The state-document shape deserves ratification.** ADR 010 settled the event protocol; it did not settle the document those events write into (`entities` / `facts` / `possessions` / `threads` / `scene`, each with authority and provenance). That shape is now load-bearing for the prompt, the UI, export, and migration. It should be written down as an ADR rather than left as an implementation detail of `narrative/model.py`. 2. **§15 of the M5 brief needs re-specifying before it can be implemented.** It requires a corrected narrator turn to become authoritative *and* the original to be retained. Retaining the original means forking; the current ✎ does not fork and the current ⑂ does not use the typed text. The missing operation — "fork here, using this exact text, and re-derive forward" — should be specified explicitly. 3. **D10 should record its remaining scope**, the way it already records its M3 and M5 split, rather than being left looking complete. ## W. M6 readiness M5's core is sound: the protocol is unambiguous, the security boundary holds, restore is snapshot-based and does not replay, lineage isolation is intact, and atomicity survives an induced storage failure. That is the hard part, and it works. M6 should not start on top of Finding 1. The broken invariant is `transcript position == head == authoritative state`, and every later feature that reads state at a position inherits it. Findings 2, 3, and 4 are each contained enough to fix alongside it. Findings 5–8 can ride along or be filed. Recommended sequence: fix Findings 1 and 3 together, since both are the same invariant reached by different routes; then 2 and 4; then close out M5. --- ## Evidence appendix | Check | Command / harness | Result | |---|---|---| | Backend suite | `.venv/bin/python -m pytest tests/ -q` | 764 passed, 3 skipped | | Realistic model | same, with endpoint and model set | 3 passed (674s) | | Frontend lint | `npm run lint` | exit 0, warnings pre-existing | | Frontend build | `npm run build` | success | | Container | `docker build` | success; image contains `app/narrative/` | | Browser | Firefox 154.0.1 headless over WebDriver | 16/16 | | Browser (edit case) | second probe, first narrator turn | defect reproduced | | Atomicity | `state_events` renamed away mid-turn | no half-commit | | No-replay | `state_events` renamed away, then undo/redo | both 200 | | Hostile events | 5 payloads through the real turn path | 0 accepted, no filesystem effect | | N+1 | query counter over the action list | 51 rows -> 53 extra queries | | Migration | pre-M5 DB rebuilt, stamped, migrated | restore leaves state stale | | Export/import | real bundle round-trip | state yes, audit no | --- --- # ADDENDUM — M5 Corrective Pass **Date:** 2026-09-04 **Branch:** `m5-narrative-state` **Base commit:** `62a997f` **Status of the review above:** unchanged. Nothing in it has been edited or withdrawn. This addendum records what was done about it. **Recommendation:** M5 CORRECTED — READY FOR CLOSEOUT / M6 ## A. Repository and staging state Recorded before any change was made: ``` branch m5-narrative-state HEAD 62a997f364e5387e4cc1dcc20f5a6618dee5f267 staged 45 files changed, 4979 insertions(+), 364 deletions(-) 12 added, 33 modified, nothing unstaged, nothing untracked ``` The staged M5 implementation was never reset, discarded, squashed or recreated. The corrective work is added on top of it, and the whole is now staged together: ``` staged 57 files changed, 6809 insertions(+), 487 deletions(-) 15 added, 41 modified, 1 renamed (the M4 report, into planning/archive) nothing unstaged, nothing untracked ``` LICENSE is untouched, the AI-DnD base commit `d72f7c1b` is still an ancestor of HEAD, and no database, secret, token or browser profile is staged. No commit was created: this repository's commits are signed by its owner, so the message is prepared at `.git/M5_CORRECTIVE_MSG` instead (§P). ## B. Disposition of Findings 1–8 | # | Finding | Disposition | |---|---------|-------------| | 1 | Narrator edit breaks the state/head invariant (**serious**) | **Fixed.** Narrator editing rebuilt on §§14-15 fork semantics. §C, §D, §E. | | 2 | N+1 on the action list | **Fixed.** `state_changes` added to the bulk read; 51 rows now cost a constant query count. §G. | | 3 | Restoring to a pre-M5 node leaves M5 state standing | **Fixed.** Migration 88 backfills; a missing snapshot restores the empty document. §F. | | 4 | A withdrawn fact survives in the prompt's history replay | **Fixed.** History carries prose only; withdrawals are named explicitly. §H. | | 5 | Export/import loses the audit trail | **Deferred to M9**, deliberately, and recorded in C04 and BUILD-MILESTONES. §K. | | 6 | Extraction strips legitimate prose | **Fixed**, and a second defect found while fixing it. §I. | | 7 | Scenario editor describes the replaced design | **Copy corrected; screen deferred to M8.** §K. | | 8 | Snapshot/restore tests never covered the new column | **Fixed.** `test_state_revert.py` now covers it first-class. §J. | ## C. The narrator-edit implementation The operation is no longer a write to the row being corrected. Nothing on the line being left is written to at all. ```text before after parent parent └── original narrator ├── original narrator ──> old future [retained] └── old future └── corrected narrator [active] ``` `_edit_narration` in `backend/app/routers/adventures/actions.py` maps one step to each clause of §15: 1. **Return to the state before the narration** — the preceding node's `narrative_state_after`, one row read, not a replay. 2. **Treat the edited text as the accepted output** — stored verbatim with only the protocol block stripped. No model is called. 3. **Re-evaluate the implied state** — the same extraction, allowlist, schema, reference and canon validation a generated turn faces. 4. **Create a new active continuation** — a new node, and the head on it. 5. **Retain the original narration and its future** — untouched. Two shapes, chosen by whether anything was written after the turn: - **At the tip:** the turn's attempts are still leaves, so the correction joins them as another take (`attempts.hand_over_the_prompt` + `attempts.add_attempt`) and the original stays beside it in the pager. No branch, and the head does not move. - **With story below it**, visible or not: `lineage.branch_of` → `tree.branch_at(depth - 1)` → `head.mark_superseded` → `tree.place_action`. The departed line keeps its node, its future and its live flag. **No new history mechanism was introduced.** This is the ⑂ path already in `takes.py` with the reader's text in place of a generated reply, using the same `tree`, `head`, `attempts` and `lineage` primitives M3 and M4 provide. Two consequences worth stating: - The §14A refusal is **gone for narrator turns** — the case it refused is now handled rather than blocked, because a fork writes nothing to the off-screen line. It remains for a player's own input (§13), which M5 did not change. - Editing a take the story is **not** telling stays a plain in-place edit. Such a take has no continuation of its own, so correcting its words contradicts nothing. This preserved two existing behaviours the first draft of the fix had broken. The frontend follows: `saveEdit` detects that the server answered with a different node and re-reads the window, because the shape of the story changed and only the server can say what the transcript is now. ## D. How retention was proven Not by inspection — by identity. Every test below captures the original row's id and text and the ids of every row below it *before* the edit, and asserts they are all still present and unchanged afterwards. - `test_editing_a_narrator_turn_with_visible_descendants_forks` - `test_the_old_narration_and_its_future_leave_the_active_transcript` - `test_editing_a_narrator_turn_with_an_undone_future_keeps_it` - `test_editing_a_narrator_turn_a_divergence_left_behind_keeps_that_line` - `test_an_edit_is_safe_when_a_future_is_off_screen` - `test_editing_the_latest_narrator_turn_still_works` (the original take retained and no longer live, with no branch created) In the browser, the same thing read straight out of SQLite: the original narrator row still holds its own words, every row of the old future still exists, and the correction is a different node id on a different branch id. ## E. How live-state/head consistency was proven A helper asserts the invariant directly against the database rather than through the API, resolving the head node **through the lineage** — after a fork the head branch owns one node and inherits the rest of the path: ```python node = head.node_at(db, adventure, adventure.head_depth) return normalize(adventure.narrative_state) == normalize(node.narrative_state_after) ``` It is asserted after every narrator edit, and at **every position visited** while undoing and redoing across an edit (`test_undo_and_redo_after_an_edit_stay_on_the_corrected_lineage`), which is where the review found the worst symptom — Undo restoring a snapshot from a line the reader was no longer on, so the prose said green and the state said red. That test additionally asserts that no attribute from the abandoned line ever reappears. The same equality is checked in the browser run (check 8) and across Undo/Redo there (check 9). ## F. How pre-M5 positions are handled Two halves, so that the stored data is explicit rather than relying on a fallback: - **Migration 88** — no DDL, a data pass. `_backfill_narrative_snapshots` writes the empty narrative document onto every action whose `narrative_state_after` is NULL. One statement, no row loop: the document is identical for every row, so it is encoded once with the same `compression.pack` and `narrative.model` the runtime uses, and bound as a single parameter. - **`attempts.restore_state`** — a missing narrative snapshot now restores the empty document. This covers a node arriving from an older export, which the migration never sees. The legacy RPG column deliberately keeps the opposite rule: a NULL `world_state_after` is still left alone, because nothing consults those numbers and blanking a running campaign's would help no one. The difference is now documented in the function rather than implicit. The empty document is the honest answer. A pre-M5 position established nothing in the narrative-state system because that system did not exist yet; retaining another position's state is a claim about a story that had not been told. `backend/tests/test_pre_m5_compatibility.py` builds a genuine pre-M5 database (M5 tables dropped, columns removed, `user_version` rewound to 80), migrates it, and runs the six required steps: ``` the migration backfills every existing action PASS the migrated campaign opens and keeps its history PASS restoring a pre-M5 Save Point leaves no later state standing PASS undo/redo across the pre-M5 boundary stay coherent PASS a pre-M5 campaign can be continued normally PASS ``` Restoring the old Save Point now yields `head depth 2` with an empty document and `live state == head snapshot`; redoing forward brings the M5 state back with the position it belongs to. State restoration remains a bounded row read — no head movement was turned into replay-from-root. ## G. Action-list query counts Measured with a statement counter over the real endpoint, before and after: | page | before | after | |------|--------|-------| | 11 actions | 24 queries, 13 fetching `state_changes` one row at a time | **13 queries** | | 51 actions | 64 queries, 53 fetching `state_changes` one row at a time | **13 queries** | Constant with page size. The fix is one line: `models.Action.state_changes` joins `ACTION_LIST_COLUMNS`, which is what `models.py` already said it was for ("`world_delta` has an M5 counterpart in `state_changes` for the bulk read"). It is a small per-turn column of the same order as `world_delta`, not the deferred snapshot — no large unrelated column was pulled in, and `narrative_state_after` is still asserted *not* to be fetched in bulk. `test_the_action_list_does_not_cost_a_query_per_action` asserts by measurement rather than by inspecting the column tuple, so a future column consumed during serialization is caught the same way. It was confirmed to fail without the fix (58 SELECTs for 52 rows) and pass with it. ## H. Manual corrections and the narrator's prompt Two changes, neither of which touches the stored historical records: - **Replayed history is prose only.** `_history_text` no longer reconstructs the protocol block into past turns. That reconstruction put a second, older account of the world into the same prompt as the authoritative one with nothing marking which governed, and handed back a fact the reader had explicitly withdrawn as an accepted event. The format instruction survives in `EMIT_RULE` (with a worked example) and `EMIT_REMINDER` (placed last). - **A withdrawn fact is named.** `render.for_prompt` adds a section after the facts that stand: ```text No longer true — do not treat these as established: Mara knows where the key was found — Mara never learned where the silver key was found. ``` Silence was the problem: dropping the fact left the narration that first asserted it as the only account in the prompt, and prose reads as current truth. Evidence, from the review's own Mara example run through the real turn pipeline: ``` history section contains "```state": no contains "add_fact": no contains "mara-knows": no state section under "Established": the withdrawn fact is absent under "No longer true": named, with the reader's reason ``` Nothing was deleted or rewritten: the invalidated fact keeps its status, reason and provenance in the document, and the `state_events` audit is untouched. ## I. Fence-stripping behaviour The extractor now removes the application's own protocol payload and nothing else. `state` is our label and is taken unconditionally; `json` and unlabelled fences are taken only when their contents are this protocol — judged both by parsing into a proposal *and* by plainly reading as one. | input | result | |---|---| | ```` ```state ```` block | stripped | | ```` ```state ```` block that does not parse | stripped, raw kept for the audit | | ```` ```json ```` containing `{"name": "Mara"}` | **kept** — it is the story | | ```` ```json ```` containing a real proposal | stripped | | ```` ```json ```` containing a *malformed* proposal | stripped | | ```` ```python ```` block | kept | | unlabelled fence with non-proposal JSON | kept | | dangling ```` ```json ```` that is story | kept | | dangling ```` ```json ```` that is a truncated proposal | stripped | | trailing bracketed aside mentioning "state block" | kept | | the prompt's own reminder, parroted back | stripped | The malformed-proposal row is a defect the corrective pass introduced and then caught: narrowing the rule to "must parse" meant a small model that mangled its own JSON had the wreckage shown to the reader. **The realistic-model run found it, not the unit tests** — a reminder of why that run exists. The rule became "parses as a proposal, or plainly reads as protocol", and `test_a_malformed_proposal_in_a_json_fence_never_reaches_the_reader` pins it. ## J. Test results ``` 794 passed, 3 skipped, 1 warning (backend, 206s) ``` Up from 764 passed at review time — 30 net new tests. The 3 skips are the realistic-model tests, skipped for missing `AIDND_TEST_ENDPOINT` / `AIDND_TEST_MODEL`; with those set they run (§L). New and rewritten: - `test_narrative_state.py` — seven D10 regressions, two correction-to-prompt regressions, nine fence-safety tests. - `test_pre_m5_compatibility.py` — new module, five migration regressions. - `test_state_revert.py` — seven new tests covering `narrative_state` first-class (Finding 8): the snapshot is recorded, deep-copied both ways, and written empty when there is none; restore puts it back, does not alias it, and restores the empty document when the node has no snapshot, while the legacy column keeps the opposite rule. - `test_egress.py` — the query-count regression and a guard that `narrative_state_after` stays out of the bulk read. - `test_head_cursor.py`, `test_change_visibility.py`, `test_retry_variants.py` — four tests rewritten. Each asserted behaviour this pass deliberately replaces (the §14A refusal for narrator turns; the protocol replay; edit-as-rewrite). None was deleted: each now asserts the stronger property that replaced it. Targeted runs, all green: narrator edit with visible descendants; latest-turn edit; retained original future; live-state == head-snapshot; pre-M5 Save Point restore; Undo/Redo across old and new positions; manual correction → next prompt; fence stripping; action-list query count; atomic state commit; lineage isolation. ## K. Deferred, deliberately - **M9 — export/recovery (Finding 5).** `state_events` / `state_proposals` are not carried in a bundle, so an imported campaign keeps a correction's effect (`manual_correction` authority survives) but reports zero audit events. C04 therefore passes for a live campaign and not across a round trip; this is written into C04's result and into BUILD-MILESTONES rather than left implicit. Not fixed here: bundle format work is M9's, and the corrective gate did not require it. - **M8 — browser UX (Finding 7).** The scenario editor still exposes the legacy RPG stat schema. Its copy claimed the app "will track them each turn — HP, mana, a raised alarm, quest objectives", which M5 made false; that sentence is corrected to say what is now true. The screen itself is M8's to redesign. - **§13 — editing player input.** Still an in-place edit guarded by the off-screen refusal. Bringing it onto the §§14-15 footing was outside M5's scope; recorded in §14A and BUILD-MILESTONES. - **A player's own input may still contain a protocol fence**, since extraction applies to narrator output. Observed while building the browser harness, not a review finding, and not acted on here. ## L. Realistic-model results Run against the same single small local model the review used, on the trusted LAN. All 3 otherwise-skipped tests pass (805s). ``` review run corrective run turns 6 6 accepted 3 6 partially_accepted 1 0 unparseable 2 0 events rejected 1 (unknown_ref) 0 assembled prompt 9382 chars 9922 chars ``` **This is one stochastic run of six turns against one 3B model, and no causal claim is made from it.** The prompt did change materially — replayed history no longer carries protocol blocks, and withdrawn facts are now stated — so an effect is plausible in either direction, but six turns cannot separate that from run-to-run variance. What the run does establish is that the corrective changes did not degrade emission against a real model, and that the pipeline still degrades safely: no event was rejected and no block was unparseable. The run also earned its keep by catching a real defect the unit tests missed — the malformed-proposal leak described in §I. ## M. Frontend, container ``` npm run lint exit 0 (7 warnings, all pre-existing at 62a997f, in files M5 did not touch) npm run build success docker build success ``` ## N. Real-browser results Firefox 154.0.1, headless, over WebDriver, against the real built frontend served by the real backend on a fresh database. **The standard M5 checks: 16/16 pass** — inspector empty then populated, grouped by category, possession with owner, a later turn moving it, Undo, Redo, Save Point restore, a manual correction landing and marked as the reader's own and surviving another turn, the audit view, a narrator edit re-deriving state, no protocol in the prose, no console errors. **The previously failing case: 16/16 pass.** Four turns establishing state, then the *earliest* narrator turn corrected through the UI's own ✎ control with all later story on screen — the exact case the review reproduced as broken: ``` 1-2 several turns established several facts PASS 3 an Edit control on the earliest narrator turn PASS 5 the corrected prose is what the story now tells PASS 5b the original narration is off the active transcript PASS 5c the old future is off the active transcript too PASS 6 the original narrator row still exists, with its own words PASS 6b every row of the old future still exists PASS 6c the correction is a different node on a different branch PASS 7 the panel shows what the corrected turn established PASS 7b nothing from the abandoned future is still shown PASS 8 live narrative_state equals the active-head snapshot PASS 9 Undo and Redo never resurrect the abandoned line's state PASS 10 the next prompt carries the corrected narration PASS 10b and not the abandoned future PASS 10c and no protocol block in the replayed history PASS 11 no console errors PASS ``` No console errors in either run. One harness correction worth recording, because the first run's failures were mine and not the product's: the probe initially picked the first Edit control on the page, which belongs to the opening *player* action. Editing a player turn correctly takes the unchanged in-place path, so nothing forked. Addressing the earliest **narrator** row — `.action:not(.player)` carrying the expected words — was the fix, and every check then passed. ## O. Acceptance status | ID | Before | After | Basis | |----|--------|-------|-------| | **D10** | FAIL | **PASS** | All three pass conditions demonstrated: corrected narration authoritative, state re-evaluated, old version and future retained. §C, §D, §E, §N. | | **C04** | PARTIAL | **PASS for the live campaign** | The withdrawn fact no longer returns through history and the withdrawal is stated explicitly. Auditability across export/import remains open and is recorded as M9 debt. §H, §K. | | **L02** | PASS for undo/redo; broke after an edit | **PASS** | The invariant is asserted at every position visited while undoing and redoing across an edit, and after restoring to a migrated pre-M5 position. §E, §F. | | **J03** | PARTIAL | **PARTIAL, narrowed** | Backend unchanged and neutral. The false user-facing claim is corrected; the legacy schema screen itself is M8's. §K. | | L01, E01, E04, H05, C01–C03, C06, D01–D09, D11–D14, I01–I04, I07, J01–J02, L03 | PASS | **PASS** | Re-run green in the full suite. | ## P. Planning documents changed - `STORY-BRANCH-SEMANTICS.md` — §§14-15 gain a "How this is built" section with the fork diagram, the two shapes, and the invariant. §14A is retitled *Superseded for Narrator Output*: it no longer says the finished behaviour is pending, and records why the refusal is gone for narrator turns and what still uses it. - `V1-ACCEPTANCE-TESTS.md` — D10's milestone-ownership note replaced by a PASS result naming the evidence for each of its three unchanged pass conditions. C04 gains a result recording what is fixed and what is deferred. **Neither criterion's pass conditions were altered.** - `planning/DECISIONS/013-authoritative-narrative-state-document.md` — new ADR recording the document shape, the authority/provenance fields, the events → document → snapshot pipeline, what each store is for, the invariant, and the consequences. It records what M5 built and invents nothing beyond it. - `BUILD-MILESTONES.md` — status line moved to M1-M5 complete with M6 next; the §14-15 note records that the corrective pass delivered it; a new "M5 — Outcome" section records what shipped and the debt assigned to M8, M9 and §13. - `README.md` — the play-loop feature bullet now says what a narrator correction does. - `planning/VERSION.md` — one line recording ADR 013. - `planning/reports/M4-IMPLEMENTATION-REPORT.md` → `planning/archive/milestone-reports/` (report rotation; a clean rename). ## Q. Verification summary | Check | Result | |---|---| | Backend suite | **794 passed, 3 skipped** (206s) | | Realistic model, one small local model | **3 passed** (805s); 6/6 turns accepted, 0 unparseable | | Frontend lint | exit 0, warnings pre-existing | | Frontend build | success | | Docker build | success | | Browser — standard M5 checks | **16/16** | | Browser — the previously failing edit case | **16/16** | | Action-list queries, 51 rows | 64 → **13** | | Pre-M5 Save Point restore | head, transcript and state agree | ## R. Is M5 safe to close, and is M6 safe to begin? **Yes to both.** The invariant M6 would have inherited is now held everywhere it was broken, and held by assertion rather than by argument: after a narrator edit, at every position visited across Undo and Redo, and after restoring to a migrated pre-M5 position. The two failures the review called the same invariant reached by different routes are fixed by the same rule rather than by two special cases. Nothing deferred blocks M6. Export of the audit trail is M9's own subject matter; the scenario editor is an M8 screen; §13 is an existing, guarded behaviour that M5 did not change and M6 does not build on. **M5 CORRECTED — READY FOR CLOSEOUT / M6**