Replaces AI-DnD's RPG relative-delta world state with the genre-neutral typed
narrative state of ADR 010: explicit, absolute, allowlisted events proposed by
the model, validated by the application, applied to one authoritative document,
and snapshotted per position so restore stays a row read.
This commit includes the corrective pass that followed the independent review
in planning/reports/M5-IMPLEMENTATION-REPORT.md. The invariant it exists to
hold is:
visible active transcript position == stored head == authoritative state
Narrator editing (D10, STORY-BRANCH-SEMANTICS §§14-15)
A narrator edit no longer rewrites a row. It returns to the state before the
turn, takes the reader's exact text as the accepted narration, re-derives the
state that text implies, and becomes a new active continuation — while the
original narration keeps its words, its live flag and its whole future as
retained history. At the tip the correction is another take; with story below
it, it forks. No new history machinery: this is the existing fork/take/head
path with the reader's text in place of a generated reply. The §14A refusal
is therefore gone for narrator turns, and remains only for player input.
Pre-M5 positions
Migration 88 backfills the empty narrative document onto every action written
before M5, and a missing snapshot now restores the empty document instead of
leaving the previous position's state standing. Restoring to an old Save
Point no longer leaves a later position's entities and facts on screen.
Narrator context
Replayed history carries prose only; the machine-readable block is no longer
reconstructed into past turns, where it contradicted the authoritative state
in the same prompt. A fact withdrawn by a manual correction is now named as
no longer true, with the reader's reason, rather than silently dropped.
Also
- state_changes joins the action-list bulk read, removing one query per row.
- Extraction takes only the application's own protocol payload: an ordinary
```json or ```python block in a story survives, and a mangled proposal
still does not reach the reader.
Planning: ADR 013 records the authoritative document shape; §§14-15/14A, D10,
C04 and BUILD-MILESTONES are updated to describe what exists. Debt is recorded
against M8 (scenario editor UX) and M9 (export of the audit trail).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWU4gTfLYY6Qq9U7aa9Qw2
957 lines
46 KiB
Markdown
957 lines
46 KiB
Markdown
# 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**
|