M5: genre-neutral authoritative narrative state, with review corrections
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
This commit is contained in:
JesseMarkowitz
co-authored by
Claude Opus 5
Claude Opus 5
parent
62a997f364
commit
b7005e6fdd
@@ -1,6 +1,6 @@
|
||||
# Adventure Storyteller — Production Build Milestones
|
||||
|
||||
**Status:** In implementation. M1-M4 complete and accepted (M1 and M2: 2026-09-02; M3 and M4: 2026-09-03); M5 — Genre-Neutral Authoritative Narrative State — next to brief
|
||||
**Status:** In implementation. M1-M5 complete and accepted (M1 and M2: 2026-09-02; M3 and M4: 2026-09-03; M5: 2026-09-04, after an independent review and a corrective pass); M6 — Branch-Safe Context, Summaries, and Long-Term Story Memory — next to brief
|
||||
**Base:** AI-DnD `d72f7c1bda0f34fccd84afb7a25c34eb01c901de`
|
||||
|
||||
## 1. Purpose
|
||||
@@ -591,6 +591,52 @@ treat the edited text as accepted output, re-evaluate the implied state, create
|
||||
new continuation, and retain the original. The refusal in §14A is then replaced
|
||||
by that behavior rather than kept alongside it.
|
||||
|
||||
**Delivered in the M5 corrective pass.** The first M5 implementation did the
|
||||
state half only and kept editing the row in place, which the independent review
|
||||
found broke the head/state invariant. A narrator edit now forks — the original
|
||||
narration and its whole future are retained untouched, and the correction
|
||||
becomes a new active continuation carrying its own re-derived state. The §14A
|
||||
refusal is gone for narrator turns and remains only for a player's own input
|
||||
(§13), which M5 did not change.
|
||||
|
||||
---
|
||||
|
||||
## M5 — Outcome
|
||||
|
||||
**Complete and accepted, 2026-09-04**, after an independent implementation
|
||||
review (`planning/reports/M5-IMPLEMENTATION-REPORT.md`) and the corrective pass
|
||||
recorded in that report's addendum.
|
||||
|
||||
Delivered:
|
||||
|
||||
- Genre-neutral typed narrative state per ADR 010, with the authoritative
|
||||
document shape now recorded in
|
||||
[ADR 013](DECISIONS/013-authoritative-narrative-state-document.md).
|
||||
- **D10 complete.** A narrator edit returns to the state before the turn, uses
|
||||
the reader's exact text, re-derives the state it implies, becomes a new active
|
||||
continuation, and retains the original narration and its future
|
||||
(`STORY-BRANCH-SEMANTICS.md` §§14-15).
|
||||
- **Pre-M5 compatibility.** Migration 88 backfills the empty narrative document
|
||||
onto every action written before M5, and a missing snapshot restores the empty
|
||||
document rather than leaving a later position's state standing. Old campaigns,
|
||||
Save Points, branches and transcripts stay usable, and no legacy RPG machinery
|
||||
becomes authoritative again.
|
||||
- **Manual corrections govern the narrator's context.** Replayed history carries
|
||||
prose only, and a withdrawn fact is named as no longer true rather than
|
||||
silently dropped.
|
||||
|
||||
Debt carried forward, deliberately:
|
||||
|
||||
- **M8 (browser UX):** the scenario editor still exposes the legacy RPG stat
|
||||
schema. Its copy no longer claims that schema is how the story is tracked, but
|
||||
the screen itself is M8's to redesign.
|
||||
- **M9 (export/recovery):** `state_events` and `state_proposals` are not carried
|
||||
in an export, so an imported campaign keeps a correction's effect but not its
|
||||
audit trail. C04 therefore passes for a live campaign and not across a round
|
||||
trip.
|
||||
- **§13 (editing player input)** is still an in-place edit guarded by the
|
||||
off-screen refusal. Bringing it onto the §§14-15 footing was out of M5's scope.
|
||||
|
||||
---
|
||||
|
||||
# M6 — Branch-Safe Context, Summaries, and Long-Term Story Memory
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
# ADR 013 — The Authoritative Narrative State Document
|
||||
|
||||
**Status:** Accepted; implemented in M5
|
||||
**Date:** 2026-09-04
|
||||
|
||||
## Context
|
||||
|
||||
[ADR 010](010-explicit-typed-narrative-state-events.md) settled the **protocol**:
|
||||
the model proposes change as explicit, typed, absolute events drawn from a fixed
|
||||
allowlist, never as relative deltas. It did not settle what those events write
|
||||
into.
|
||||
|
||||
That shape turned out to be load-bearing. The prompt renders it, the browser's
|
||||
state panel groups it, export carries it, migration has to produce it for
|
||||
positions that predate it, and every Undo, Redo, take switch and Save Point
|
||||
restore reads a copy of it. The M5 review recommended recording it as a decision
|
||||
rather than leaving it an implementation detail of `narrative/model.py`. This ADR
|
||||
records what was built; it does not extend it.
|
||||
|
||||
## Decision
|
||||
|
||||
### The document
|
||||
|
||||
One JSON document is the campaign's authoritative account of its own story. It
|
||||
is genre-neutral: nothing in it names a stat, a level, a currency or a class.
|
||||
|
||||
```text
|
||||
entities who and what exists — people, places, things, groups.
|
||||
Each carries a name, a type, aliases, a description, a status,
|
||||
free-form attributes, conditions, and a current location.
|
||||
facts what has been established, as subject/predicate/object.
|
||||
Each carries an id, a status, and where it came from.
|
||||
relationships how entities stand to one another, directionally.
|
||||
possessions which entity holds which item.
|
||||
threads open story threads, with a title and a status.
|
||||
scene where the story is now, and a short summary of the moment.
|
||||
```
|
||||
|
||||
Two fields appear throughout and are the reason the document can be trusted:
|
||||
|
||||
- **authority** — who established this: the story, or the reader's own
|
||||
correction. A reader's correction outranks the narration, and the prompt says
|
||||
so in words.
|
||||
- **provenance** — the branch and depth the change was made at, so a fact can be
|
||||
traced to the moment it entered the story.
|
||||
|
||||
Nothing is deleted. A fact a correction takes back is marked `invalidated`, with
|
||||
the reason and the position, because a record that vanished would audit nothing.
|
||||
|
||||
### The pipeline
|
||||
|
||||
```text
|
||||
model output
|
||||
-> extraction the protocol block is separated from the prose
|
||||
-> validation envelope, allowlist, schema, references, canon
|
||||
-> validated events recorded in `state_events`, with their provenance
|
||||
-> the document applied to produce the new authoritative state
|
||||
-> per-position snapshot stored on the node the turn produced
|
||||
```
|
||||
|
||||
Each stage has one job, and the order is deliberate: the allowlist is checked
|
||||
before any field is read, so an unknown event type is rejected before its
|
||||
contents are touched.
|
||||
|
||||
### Where it lives
|
||||
|
||||
- `adventures.narrative_state` — the current authoritative document. This is
|
||||
what the narrator is told and what the reader is shown.
|
||||
- `actions.narrative_state_after` — the whole document as it stood after that
|
||||
position played, on **every** node.
|
||||
- `state_events` / `state_proposals` — the audit: what was proposed, what was
|
||||
accepted, what was refused and why.
|
||||
|
||||
### What each is for
|
||||
|
||||
**Event history is audit and provenance, not a source of truth.** Restoring a
|
||||
position never replays it. This was proven by renaming the table away
|
||||
mid-campaign: Undo and Redo continued to work.
|
||||
|
||||
**Snapshots are what make restore bounded.** Arriving at a position is a single
|
||||
row read whose cost does not grow with the length of the story (ADR 012 §10.4).
|
||||
A position without a snapshot is a position the head cannot be restored to, so
|
||||
every node has one — including nodes written before M5, which migration 88
|
||||
backfills with the empty document. A missing snapshot restores the empty
|
||||
document rather than leaving the previous position's state standing.
|
||||
|
||||
**The document is the authority; the model only proposes.** Every event, whether
|
||||
it came from the model or from a reader's correction, passes the same
|
||||
validation. The model cannot write a field directly, cannot invent an event
|
||||
type, and cannot refer to an entity that does not exist in this campaign.
|
||||
|
||||
## The invariant
|
||||
|
||||
```text
|
||||
visible active transcript position == stored head == authoritative state
|
||||
```
|
||||
|
||||
Everything above exists to hold this. The M5 review found two ways it had been
|
||||
broken — a narrator edit that rewound live state while the head stayed at the
|
||||
tip, and a restore to a migrated position that left a later position's state
|
||||
standing — and both were fixed by making the rule absolute rather than by adding
|
||||
a special case.
|
||||
|
||||
## Consequences
|
||||
|
||||
- The document is larger than the RPG dict it replaced, so both it and the
|
||||
per-node snapshots are stored compressed.
|
||||
- Genre lives in the campaign's canon and in the words the story uses, never in
|
||||
the schema. A science-fiction campaign and a fantasy one produce the same
|
||||
shapes.
|
||||
- A reader's correction is durable and outranks narration, and the prompt states
|
||||
both what holds and what has been withdrawn.
|
||||
- Export carries the document, the canon and every snapshot. It does **not**
|
||||
yet carry `state_events` / `state_proposals`, so an imported campaign keeps a
|
||||
correction's effect but not its audit trail. Deferred to M9.
|
||||
@@ -377,37 +377,69 @@ Therefore the system must:
|
||||
|
||||
The system must not simply replace visible text while leaving stale state behind.
|
||||
|
||||
## 14A. Editing In Place, Before §14-15 Are Implemented
|
||||
### How this is built (M5 corrective pass)
|
||||
|
||||
§14 and §15 describe the finished behavior: a narrator edit becomes
|
||||
authoritative, the state it implies is re-evaluated, a new continuation is
|
||||
created, and the original narration and its future are retained. That
|
||||
requirement stands in full and is **not** weakened by this section.
|
||||
All five conditions are implemented. A narrator edit is not a write to the row
|
||||
being corrected — nothing on the line being left is written to at all. It is the
|
||||
same operation as playing the turn again from here, with the reader's words in
|
||||
place of a generated reply, and it reuses the fork, take, head and snapshot
|
||||
machinery M3 and M4 already provide rather than introducing a second history
|
||||
mechanism.
|
||||
|
||||
It is not yet built. Re-evaluating the state implied by prose a user typed
|
||||
requires the authoritative narrative-state extraction that the genre-neutral
|
||||
state milestone introduces, so the finished behavior is completed there. What
|
||||
exists in the meantime is a plain correction: it changes the words of one turn
|
||||
and re-evaluates nothing.
|
||||
```text
|
||||
before after
|
||||
|
||||
That correction is safe while everything descending from the turn is on screen,
|
||||
because the user can see what their change has to stay consistent with. It is
|
||||
not safe when a continuation descends from the turn and is **off screen** —
|
||||
undone and not yet redone, or left behind by a divergence — because the edit
|
||||
would then silently change the words that retained story was written from, and
|
||||
nothing on screen would show it. Retained history is not permitted to be made to
|
||||
disagree with itself in a way the user cannot see.
|
||||
parent parent
|
||||
└── original narrator ├── original narrator ──> old future [retained]
|
||||
└── old future └── corrected narrator [active]
|
||||
```
|
||||
|
||||
Until §14-15 are implemented, the system must therefore **refuse** an in-place
|
||||
edit of a turn that has story descending from it which is not currently being
|
||||
shown, and say why. The user resolves it the same two ways as §10:
|
||||
Two shapes, chosen by whether anything was written after the turn:
|
||||
|
||||
- **Redo**, bringing the later story back into view; or
|
||||
- **play the turn again from here**, which is the §13 shape — return to the
|
||||
parent position, continue differently, and keep the old line as retained
|
||||
history.
|
||||
- **Nothing below it.** The attempts at that turn are still leaves, so the
|
||||
correction joins them as another take and the original stays beside it in the
|
||||
pager. No branch is created, and the head does not move.
|
||||
- **A story below it**, on screen or not. That story was written as a
|
||||
continuation of the words that are there now, so it keeps them: the correction
|
||||
leaves the path just before the turn, and the departed line keeps its node,
|
||||
its future and its live flag.
|
||||
|
||||
Refusing is the minimum that keeps the invariant. It is not the destination.
|
||||
The state the correction implies is derived from the snapshot on the node
|
||||
*before* the edited turn — one row read, not a replay — and put through the same
|
||||
validation the model's own proposals face. The correction's node then carries
|
||||
that document as its own outcome, so the campaign's live state and the state
|
||||
stored at the head are the same document. That equality is the invariant:
|
||||
|
||||
```text
|
||||
visible active transcript position == stored head == authoritative state
|
||||
```
|
||||
|
||||
The M5 review found it broken by the interim implementation, which rewrote the
|
||||
row in place and rewound the campaign's live state to that position while the
|
||||
head stayed at the tip. The reader saw a full transcript over a state document
|
||||
describing an earlier moment, and the snapshots below the edit still described
|
||||
prose that no longer existed.
|
||||
|
||||
## 14A. Editing In Place — Superseded for Narrator Output
|
||||
|
||||
§§14-15 are implemented, and the refusal this section described no longer
|
||||
applies to narrator turns. It is kept because it explains why the current shape
|
||||
is the one it is.
|
||||
|
||||
The refusal existed because an in-place edit rewrote the words that a
|
||||
descending, invisible story had been written from, and retained history is not
|
||||
permitted to be made to disagree with itself where the user cannot see it. A
|
||||
fork removes the premise: the off-screen future keeps the exact narration it
|
||||
descends from, so there is nothing left to refuse. What was the unsafe case is
|
||||
now an ordinary one.
|
||||
|
||||
Two in-place edits remain, and neither can produce that disagreement:
|
||||
|
||||
- **A player's own input** (§13) is still edited in place, and is still refused
|
||||
while a story descends from it off screen. Bringing §13 onto the same footing
|
||||
as §§14-15 is not yet done.
|
||||
- **A take the story is not telling** has no continuation of its own — keeping
|
||||
one is what forking is for — so correcting its words contradicts nothing.
|
||||
|
||||
## 16. Manual State / Canon Correction
|
||||
|
||||
|
||||
@@ -517,6 +517,31 @@ Mara never learned where the silver key was found.
|
||||
- correction is auditable,
|
||||
- old transcript is not silently rewritten unless explicitly edited.
|
||||
|
||||
### Result — PASS for the live campaign (M5 corrective pass, 2026-09-04)
|
||||
|
||||
The M5 review found the first condition failing: the state section dropped a
|
||||
withdrawn fact and the replayed history handed it straight back as an accepted
|
||||
event, in the protocol's own words, with nothing saying it had been corrected.
|
||||
Two changes fixed it, and both are pinned by
|
||||
`test_c04_a_withdrawn_fact_does_not_come_back_through_history`:
|
||||
|
||||
- replayed history is prose only — the machine-readable block is no longer
|
||||
reconstructed into past turns, so a turn's record of what was true *then*
|
||||
cannot contradict what is authoritative now;
|
||||
- a withdrawn fact is named in the state section under "No longer true — do not
|
||||
treat these as established", with the reader's reason, rather than silently
|
||||
omitted. Omitting it left the narration that first asserted it as the only
|
||||
account in the prompt.
|
||||
|
||||
The old transcript is not rewritten: an invalidated fact stays in the document
|
||||
with its status, its reason and its provenance.
|
||||
|
||||
**Carried debt, deferred to M9 (export/recovery):** a campaign's `state_events`
|
||||
and `state_proposals` are not carried in an export, so an imported copy keeps
|
||||
the correction's *effect* — the fact is still marked `manual_correction` — but
|
||||
reports zero audit events. The auditable condition therefore holds for a live
|
||||
campaign and not across a round trip.
|
||||
|
||||
---
|
||||
|
||||
## C05 — Canon Beats Reference
|
||||
@@ -721,25 +746,41 @@ Mara wears a green cloak.
|
||||
- downstream state is re-evaluated,
|
||||
- old version/future remains retained/disposable.
|
||||
|
||||
### Milestone ownership
|
||||
### Result — PASS (M5 corrective pass, 2026-09-04)
|
||||
|
||||
All three pass conditions stand for v1. They are delivered across three
|
||||
milestones, and this note records which is which rather than reducing the
|
||||
requirement:
|
||||
All three pass conditions are met, and each was demonstrated rather than
|
||||
inferred.
|
||||
|
||||
- **Edit becomes authoritative on the active path.** The reader's text is stored
|
||||
verbatim, with only the protocol block stripped, and no model is called
|
||||
(`test_the_corrected_text_is_used_verbatim_not_regenerated`).
|
||||
- **Downstream state is re-evaluated.** The state is derived from the snapshot on
|
||||
the node before the corrected turn and put through the normal validation path,
|
||||
and the campaign's live document then equals the document stored at the head
|
||||
(`test_editing_a_narrator_turn_with_visible_descendants_forks`, and the
|
||||
live-state/head-snapshot assertion carried by every test in that group).
|
||||
- **Old version/future remains retained/disposable.** Nothing on the departed
|
||||
line is written to: the original node keeps its words and its live flag, and
|
||||
every row played after it still exists
|
||||
(`test_the_old_narration_and_its_future_leave_the_active_transcript`,
|
||||
`test_editing_a_narrator_turn_with_an_undone_future_keeps_it`).
|
||||
|
||||
Verified in a real browser on the case the M5 review reproduced as broken:
|
||||
correcting the earliest narrator turn with four turns of story on screen, then
|
||||
checking the transcript, the retained rows, the state panel, the live document
|
||||
against the head snapshot, Undo/Redo, and the next turn's assembled prompt —
|
||||
16 of 16 checks.
|
||||
|
||||
The delivery history, kept because it explains the shape:
|
||||
|
||||
- **M3 — safe history behavior.** Replaying a narrator turn with different text
|
||||
forks, keeps the original take and its future, and starts the new continuation
|
||||
from the correct earlier state. In-place editing of a turn is **refused** while
|
||||
story descends from it off screen, so retained history cannot be made to
|
||||
contradict itself unseen (`STORY-BRANCH-SEMANTICS.md` §14A). Delivered.
|
||||
- **M5 — authoritative state re-evaluation.** The second pass condition. Making a
|
||||
hand-typed narrator correction authoritative and re-evaluating the state it
|
||||
implies requires the narrative-state extraction pass, so it is completed there,
|
||||
and the §14A refusal is replaced by it. Outstanding.
|
||||
- **Later browser UX work — the finished editing workflow.** How the user reaches
|
||||
and confirms the operation. Outstanding.
|
||||
|
||||
D10 is therefore **not** satisfied at the end of M3, and is not scored as such.
|
||||
forks and keeps the original take and its future. In-place editing was
|
||||
*refused* while story descended from a turn off screen.
|
||||
- **M5 — authoritative state re-evaluation**, and the refusal replaced. A
|
||||
narrator edit now forks rather than rewriting a row, so the off-screen case it
|
||||
refused is simply handled (`STORY-BRANCH-SEMANTICS.md` §§14-15).
|
||||
- **Later browser UX work.** How the reader reaches and confirms the operation
|
||||
is M8's; the operation itself is complete and reachable through the ✎ control.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -272,6 +272,8 @@ Key v2 changes include:
|
||||
- AI-DnD selected as the production base at the pinned Phase 0B commit.
|
||||
- Non-destructive head-cursor Undo/Redo design selected.
|
||||
- Explicit typed/absolute narrative-state events selected for production state handling.
|
||||
- The authoritative narrative-state document — its shape, its authority/provenance fields, and the
|
||||
event/document/snapshot split — recorded in ADR 013 after M5 implemented it.
|
||||
- Imported knowledge separated from AI-DnD Story Cards.
|
||||
- Trusted-LAN Ollama inference supported in v1 while the storyteller UI/API remains loopback-bound by default.
|
||||
- Offline first-use dependencies and runtime remote assets identified as M1 hardening work.
|
||||
|
||||
@@ -0,0 +1,956 @@
|
||||
# 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**
|
||||
Reference in New Issue
Block a user