Files
JesseMarkowitzandClaude Opus 5 279a871a77 Planning: add the M4 implementation review report and rotate M3's
Reporting pass only. No application code, no test, and no product
requirement changes.

Result: PASS WITH CORRECTIVE WORK REQUIRED.

M4's Definition of Done is met and demonstrated at the API level, including
across a real two-process restart. The load-bearing constraint holds under
inspection rather than assertion: the only head-field assignment M4 added
anywhere in the backend is one line in head.py, and an exhaustive grep of the
diff finds no second mechanism that forks, prunes memories, reconstructs
state, filters the transcript or recomputes Redo.

Three corrective items, all M4's own, none in the head model:

- GET /checkpoints is an N+1 fetching whole Action rows including prose --
  measured at 53 SELECTs for 25 Save Points against 4 for the branch panel,
  in a codebase that keeps test_egress.py for this exact class of mistake;
- deleting a branch silently deletes Save Points naming it, and the branch
  panel's confirmation does not say so. M4 added the consequence to an
  existing destructive action without updating its warning;
- the shipped D11/L03 tests restart a client, not a process, so the suite is
  weaker than the acceptance items it is named for. Both pass here only
  because the report re-ran them across a real process boundary by hand.

The browser smoke test is NOT PERFORMED, for M4 and still for M3. Firefox is
a snap that hangs past 90s on a trivial headless screenshot; there is no
Xvfb, no display, no driver library. Two consecutive milestones now carry an
unperformed browser requirement, which the report raises as a standing
acceptance risk rather than a defect in either milestone's code.

Evidence recorded: 680 backend tests pass (42 M4, 130 M3 invariants, 93
security/local-only), frontend lint and build clean, Docker build clean,
migration 80 verified against a representative pre-M4 database with both
cascades and zero possible orphans, and I04 verified through a real round
trip with branch ids remapped 1->3 and 2->4.

M4 is NOT accepted by this report, and M5 is NOT authorized. That decision
belongs to whoever reviews this.

Rotation: planning/reports/M3-IMPLEMENTATION-REPORT.md moves to
planning/archive/milestone-reports/ as a pure rename, contents unedited
(git reports 100% similarity, 0 insertions, 0 deletions). Six path
references in five active documents are updated because the path changed
and for no other reason -- no status claim, no wording change.

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

1544 lines
74 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# M3 Implementation Review Report
**Milestone:** M3 — Production Non-Destructive History, Redo, and Active-Head Export
**Prepared:** 2026-09-03
**Repository:** `interactive-story` (Adventure Storyteller production fork of AI-DnD)
**Branch:** `m3-nondestructive-history`
> **This report is the primary evidence record for M3.**
> No `M3-BASELINE-REPORT.md` was produced. Where M1 and M2 split raw measurement
> from interpretation, M3 has only this document, so it carries the exact
> commands, counts and observations inline rather than citing a companion. If a
> baseline report is written later, it becomes authoritative on any observed fact
> where the two disagree.
>
> **Sections A-V record the state at review time. §W is a closeout addendum, and
> is current where the two differ.**
---
# A. Executive Result
```text
Overall M3 result: PASS WITH CORRECTIVE WORK REQUIRED
```
Every behavioural requirement of M3 is implemented and demonstrated. The
corrective work is **not a code defect**: the milestone's final commit has not
been made, and the browser smoke test the milestone prompt requires was not
performed. Both are stated in full in §B and §M.
| Question | Result |
| --- | --- |
| Is Undo non-destructive? | **Yes.** Undo moves a stored head; no delete path runs. |
| Does Undo delete zero accepted turns? | **Yes.** Measured: 15 rows before, 15 after five Undos. |
| Does Redo work? | **Yes.** Walks the retained lineage forward one whole turn. |
| Does repeated Undo/Redo round-trip exactly? | **Yes.** head 14 → 4 → 14, rows constant at 15, state exact. |
| Does divergence preserve the displaced future? | **Yes.** Every row retained; the departed branch is marked. |
| Is ordinary Redo invalidated after divergence? | **Yes.** 400, with no flag consulted — it falls out of the lineage. |
| Does Retry obey the same lineage rules? | **Yes.** Retry, add-take and the write path share `head.behind_tip`. |
| Do edit paths preserve prior history where implemented? | **Yes, with a scope judgement** — see §I. The replay path forks and retains; the in-place prose correction was deliberately not changed. |
| Does state reconstruct correctly? | **Yes.** Verified in both directions of travel. |
| Does abandoned memory remain isolated? | **Yes**, with both controls (§K). |
| Does active-head export/import work? | **Yes.** `headDepth` round-trips; import does not redo. |
| Do pre-M3 exports remain compatible? | **Yes.** Absent key ⇒ opened at the tip, which is the position such a file recorded. |
| Does the browser behavior work as intended? | **Unverified by a browser.** Code, lint, build and endpoints pass; no click-through was performed. |
| Did local-only/security behavior regress? | **No.** 93 targeted tests pass; live HTTPS trusted-LAN inference re-confirmed. |
| Should the project proceed to M4? | **Yes, after the two corrections below.** |
| Are there blockers before M4? | **Two, both process:** the M3 commit is unmade, and the browser smoke test is unrun. |
## Qualifications, stated up front
1. **The final M3 commit does not exist.** The work is staged but uncommitted.
Commits in this repository are GPG-signed and the agent preparing this work
cannot sign them. All runtime evidence in this report was therefore gathered
from the **modified working tree**, not from committed `HEAD`. §B gives the
exact file list.
2. **No browser smoke test was run.** The session had no browser to drive. An
equivalent end-to-end sequence was executed against the running application
with real inference (§M), which is strong evidence for the server but is
explicitly *not* the browser test the milestone requires.
3. **Two inherited behaviours were narrowed**, both deliberately and both
defensible, but a reviewer should ratify them: Undo now walks *past* a fork
point (§E.4), and switching the live take is refused while a retained future
hangs off that turn (§H.5).
---
# B. Repository and Provenance
| Item | Value |
| --- | --- |
| Branch | `m3-nondestructive-history` |
| Starting commit (post-M2 closeout) | `2fdd254` — *Planning: record M2 closeout decisions* |
| M3 checkpoint commit | `903fa7a` — *M3: move the story's head instead of deleting its turns* |
| Final M3 commit | **not created** |
| Current `HEAD` | `903fa7a` |
| Commit signature | `903fa7a` verifies: `G`, RSA key `7D8AE19DB5C68569`, *JesseMarkowitz* |
| Upstream ancestry | `d72f7c1bda0f34fccd84afb7a25c34eb01c901de` **is** an ancestor of `HEAD` — the pinned Phase 0B fork point is intact |
| `LICENSE` | Unmodified since `22630c8` (Phase 7). MIT, © 2026 Parth Thakkar. No M3 change |
| Working tree | **Not clean.** 11 paths staged, 0 unstaged, 1 untracked (this report) |
```
$ git log --oneline -3
903fa7a M3: move the story's head instead of deleting its turns
2fdd254 Planning: record M2 closeout decisions
8652fe7 M2 review: two regressions the green suite hid, and the reports
```
## B.1 Why the tree is not clean — mandatory disclosure
The M3 work exists in two parts:
* **`903fa7a`, committed and signed** — the head module, the head-capped
lineage, `/undo` rewritten, `/redo` added, the retry/add-take/select-variant
head rules, migrations 78–79, and the `can_undo`/`can_redo` fields.
* **Staged and uncommitted** — the export/import active head, the browser Redo
control, the rewritten inherited tests, and the M3 acceptance suite.
```
$ git status --short
M README.md
M backend/app/bundle.py
M backend/app/routers/adventures/__init__.py
M backend/app/routers/adventures/actions.py
M backend/app/routers/adventures/bundle_io.py
M backend/tests/test_attempt_siblings.py
M backend/tests/test_branch_forking.py
A backend/tests/test_head_cursor.py
M backend/tests/test_state_revert.py
M frontend/src/api.js
M frontend/src/pages/Play/index.jsx
?? planning/reports/M3-IMPLEMENTATION-REPORT.md
```
The untracked path is this report, written after the staged work and belonging to
a separate commit, as M1's and M2's review reports did.
The reason the rest is uncommitted is the repository's signing policy, not incomplete work: every commit
here is GPG-signed, signing needs a pinentry the agent shell cannot prompt from,
and an unsigned commit would break a chain in which every commit verifies. A
prepared commit message is staged for the repository owner to run.
**Consequence for this review:** every test count, measurement and runtime
observation below was produced from the staged tree. Re-running them against
`903fa7a` alone would fail — that commit's own message records five tests still
asserting the destructive contract. A reviewer validating this report should
apply the staged changes first, or review after the commit is made.
---
# C. M3 Change Inventory
## C.1 Diff statistics
Whole milestone, `2fdd254` → staged tree:
```text
files changed 21
insertions 1643
deletions 141
files added 2 (backend/app/head.py, backend/tests/test_head_cursor.py)
files deleted 0
```
By area:
| Area | Files | Insertions | Deletions |
| --- | ---: | ---: | ---: |
| Application code (`backend/app`) | 14 | 668 | 95 |
| Tests (`backend/tests`) | 4 | 907 | 28 |
| Frontend (`frontend/src`) | 2 | 54 | 11 |
| Documentation (`README.md`) | 1 | 14 | 7 |
Tests outweigh application code roughly 4:3. That is the intended shape for a
milestone whose entire subject is an invariant about what does *not* happen.
## C.2 By subsystem
**Active-head persistence.** No new storage. `adventures.head_branch_id` and
`adventures.head_depth` already existed (migration 49); M3 changed their
*meaning* from "where the story ends" to "where the story is being read", which
is why the milestone needed no adventure-table migration.
**Undo** (`routers/adventures/takes.py`). The endpoint no longer deletes the
trailing nodes, prunes memories, or recomputes the tip. It resolves a target
through `head.undo_target`, calls `head.move_to`, and returns the newest window.
**Redo** (`routers/adventures/takes.py`). New endpoint, same shape in reverse.
**Divergence/fork** (`head.fork_if_behind_head`, called from
`routers/adventures/turns.py`). Runs before every story-continuing write and
does nothing when the head is already at the tip.
**Retry / add-take** (`takes.py`). Both ask `head.behind_tip` rather than
comparing against `last_action`, which reads the capped path and would report a
turn with a retained future as a leaf.
**Edit.** The replay-with-new-text path (`POST .../actions/{id}/takes`) inherits
the add-take rules. The in-place prose edit (`PATCH .../actions/{id}`) was
deliberately not changed; see §I.
**State restoration.** Unchanged mechanism. `attempts.restore_state` reads the
per-node `world_state_after` snapshot; `head.move_to` calls it. Direction of
travel is irrelevant because the snapshot belongs to the node.
**History/context selection** (`context/lineage.py`). `Path` now caps every
entry at the head via `_cap`, and exposes `uncapped()` for the two callers
allowed to see past it. This is the single change that makes the transcript, the
assembled context, `attempts.preceding` and memory retrieval narrow together.
**Memory lineage.** No memory code changed. A memory carries the coordinate of
the node its block ends on, so the capped clause excludes it automatically.
**Export/import** (`bundle.py`, `bundle_io.py`). `headDepth` and the branch
disposition are written and read; `plan()` validates both before any row exists.
**API.** One new route (`POST /api/adventures/{id}/redo`) and two new response
fields (`can_undo`, `can_redo`) on `AdventureOut` and `ActionPage`. Distinct
`/api` paths: **36 after M2 → 37 after M3.**
**Frontend.** A Redo button, `Ctrl+Shift+Z`, both controls driven by the server
flags, and a `moveHead` helper shared by Undo and Redo.
**Schema.** Migrations 78 and 79 only (§P).
**Tests.** One new file (24 tests), three files rewritten (5 tests), one
re-export added.
**Documentation.** Three `README.md` bullets corrected.
## C.3 Was there an unexpectedly large refactor?
No. The largest single change is `backend/app/head.py` at 304 lines, of which
roughly two thirds is prose explaining the rules. No existing module was
restructured; `lineage.Path` gained a private `_cap` and a public `uncapped()`
without changing any call site's shape.
---
# D. Active-Head Architecture
## D.1 The distinction
```text
retained history: 1 -> 2 -> 3 -> 4 -> 5
retained tip = 5 the deepest live node on the lineage
active head = 3 where the story is being read
redo path = 4 -> 5
opening = 1 the floor Undo may not pass
```
Before M3 these were one value, because the only place a story could be read was
its newest row. Undo made that true by deleting everything past where it landed.
## D.2 Where the head lives and how it resolves
Stored on the adventure as `head_branch_id` + `head_depth`. It is never derived.
`lineage.path_of(db, adventure)` returns a `Path` holding the lineage entries and
the head depth. `Path.clause(model)` builds the SQL every read uses, and each
entry's depth cap is `min(entry_cap, head)`. Two consequences:
* every read of the story stops at the head, in one place rather than at each
call site;
* the head wins even against an ancestor's fork cap, which is what lets Undo walk
back through a fork point into the story a branch inherits.
`Path.uncapped()` returns the same lineage read through to its retained tip. Only
`head.retained_tip`, `head.node_at`, `head.redo_target` and `head.opening_depth`
use it — that is, Redo and the fork check. Every read of *the story* uses the
capped path.
## D.3 How descendants stay retained
Nothing marks them. They are ordinary live rows at their own depths on their own
branch; the capped clause simply does not select them. This is why the invariant
is cheap to hold: Undo has no delete path to get wrong.
## D.4 State at a position
Each node carries `world_state_after` — the state it left behind.
`head.move_to(db, adventure, depth)` sets the depth and calls
`attempts.restore_state` with the node found there. A destination with no node
(the head resting one step in front of the opening) leaves live state alone,
which is `restore_state`'s existing rule for a missing snapshot.
## D.5 How Redo availability is decided
`head.redo_target` walks the **uncapped** lineage forward from the head and takes
a whole turn — a player node plus the reply to it — so the head never lands
between the two. After a divergence the new branch *is* the lineage and the
displaced future is no longer on it, so the walk finds nothing and returns
`None`. `STORY-BRANCH-SEMANTICS.md` §8 therefore holds as a property of the
lineage rather than as a flag anyone has to clear.
## D.6 Chokepoints
| Concern | Single owner |
| --- | --- |
| Where the story is read | `lineage.Path._cap` |
| What is still retained | `lineage.Path.uncapped` |
| Is there story past the head? | `head.behind_tip` |
| Where does Undo go? | `head.undo_target` |
| Where does Redo go? | `head.redo_target` |
| Move the head, restore state | `head.move_to` |
| Does this write fork? | `head.fork_if_behind_head` |
| Record a departed branch | `head.mark_superseded` |
## D.7 Did it stay as bounded as Phase 0B suggested?
**Partly.** The spike changed three backend files. Production touched fourteen.
The difference is not scope creep in the head model — the head model itself is
`head.py` plus about 60 lines of `lineage.py` — but work the spike never
attempted: export/import, retry and add-take, the select-variant guard, the
delete-action interaction, the API flags, and the browser. The spike's central
claim (that the head cursor does not require restructuring the tree) held.
The spike also had two defects this implementation corrected rather than copied:
it moved the head to `-1` and rendered an empty transcript for an adventure
opened with a player-written `story` action, and it put the fork check only in
the write path, leaving Retry and add-take on the old one.
---
# E. Undo Results
## E.1 The central measurement
Seven turns played, then five consecutive Undos, counting rows directly:
```text
after seven turns: rows= 15 head_depth= 14
undo 1: status=200 rows= 15 head_depth= 12 shown= 13 can_redo=True
undo 2: status=200 rows= 15 head_depth= 10 shown= 11 can_redo=True
undo 3: status=200 rows= 15 head_depth= 8 shown= 9 can_redo=True
undo 4: status=200 rows= 15 head_depth= 6 shown= 7 can_redo=True
undo 5: status=200 rows= 15 head_depth= 4 shown= 5 can_redo=True
```
```text
Accepted rows deleted by Undo: 0
```
Retained rows are constant at 15. The head moves two depths per Undo — one whole
turn, being the player's action and the reply to it — and the visible transcript
shrinks by exactly two rows each time.
## E.2 Semantics demonstrated
* **Transcript** narrows to the head; retained rows are unreachable, not gone.
* **State** is restored from the destination node's snapshot. D01 asserts gold
returning from 20 to 10 on one Undo; L02 asserts the full ladder.
* **Five consecutive Undos** — `test_d02_...`, each position's state checked.
* **Campaign root** — `test_d03_...` undoes seven times to the opening, then gets
`400 Nothing to undo` and `can_undo == false`. The floor is the shallowest node
on the story, not a node of type `start`, so an adventure opened with a
player-written `story` action stops in the same place.
* **Practical unlimited Undo** — D03 is satisfied, not merely the D02 minimum:
Undo traverses to the opening with no documented limit. Cost is one indexed
query per step regardless of story length.
## E.3 Memory under Undo
No pruning. A memory whose coordinate is past the head falls outside the capped
clause, becomes unretrievable, and becomes eligible again on Redo without being
deleted or re-embedded (`test_undo_stops_retrieving_a_memory_without_deleting_it`).
## E.4 Rewritten tests that previously asserted destruction
Five tests asserted the old contract. All were rewritten, none deleted.
| Test | Was | Now |
| --- | --- | --- |
| `test_state_revert::test_undo_reverts_state_to_before_the_turn` | rows gone (`actions == ["start"]`) | state assertion kept; rows all present; head moved; `can_redo` true |
| `test_state_revert::test_undo_of_bare_continue_uses_the_node_in_front` | rows gone | story reads short, retained story intact |
| `test_state_revert::test_undo_prunes_memory_covering_removed_actions` | memory row deleted | **renamed** `..._stops_retrieving_a_memory_without_deleting_it` — unreachable, on disk, eligible again after Redo |
| `test_attempt_siblings::test_undo_takes_every_attempt_with_it` | whole sibling group deleted | **renamed** `test_undo_hides_every_attempt_and_keeps_them_all` — group leaves the story as one, every id retained, same take live after Redo |
| `test_branch_forking::test_undo_stops_at_the_fork` | Undo **refused** at a fork | **renamed** `test_undo_walks_off_a_fork_into_the_story_it_inherits` — Undo proceeds |
**The fork case is a deliberate behaviour reversal a reviewer should ratify.**
The old refusal existed because Undo deleted rows the parent branch was also
reading. With nothing deleted there is nothing to protect the parent from, a
forked branch's inherited prefix is part of the story it tells, and the floor
becomes the campaign opening rather than the fork point. This also means one
Undo on a fork can step back over a player action that lives on the parent
branch — a read moving backwards, not a write.
A third test was **added** to the same file (`test_redo_puts_back_the_state...`),
since undo and redo restoring the same snapshot from opposite directions is the
other half of the mechanism the file covers.
---
# F. Redo Results
## F.1 Round trip
Continuing the measurement in §E.1, five Redos from head 4:
```text
redo 1: status=200 rows= 15 head_depth= 6 shown= 7
redo 2: status=200 rows= 15 head_depth= 8 shown= 9
redo 3: status=200 rows= 15 head_depth= 10 shown= 11
redo 4: status=200 rows= 15 head_depth= 12 shown= 13
redo 5: status=200 rows= 15 head_depth= 14 shown= 15
round trip: rows=15 head_depth=14
```
```text
Undo -> Redo returns to the exact prior story position and state: YES
```
Head returns to 14, the transcript to all 15 rows, rows never changed.
## F.2 State restoration
`test_l02_state_matches_the_position_in_both_directions` records the
instrumented value at each position going back and coming forward:
```text
going back: [40, 30, 20, 10, 0]
coming forward: [10, 20, 30, 40, 50]
```
The sequences interlock exactly, which is the property that matters: a position's
state does not depend on the direction it was reached from.
## F.3 At the tip, and with no path
At the retained tip, `can_redo` is `false` and `POST /redo` returns
`400 Nothing to redo`. Same after a divergence. The control reports rather than
guesses — there is no "choose a descendant" heuristic anywhere.
## F.4 Ambiguity when several descendants exist
There is none, and the reason is structural rather than a tie-break rule. Redo
walks the **lineage**, which is a single path — one branch plus its ancestors
capped at their fork depths. Sibling branches are not on it. Where several
*takes* sit at one coordinate, the walk selects `live`, of which there is exactly
one per coordinate by construction (`_write_nodes` enforces it even on import).
---
# G. Divergence / Abandoned Future
## G.1 The sequence
`test_d05_a_new_turn_below_the_head_retires_redo_and_keeps_the_future` performs
exactly the required scenario. Four turns, all row ids recorded, two Undos, then
a new write.
Afterwards:
```text
1 -> 2 -> 3
|\
| 4A -> 5A retained, on the branch they were written on
|
4B active
```
Verified in the test:
* **4A/5A still exist** — the recorded id set is a subset of the id set after.
* **4B is active** — the transcript's last entry is the new text.
* **Ordinary Redo does not reach A** — `can_redo` false, `POST /redo` → 400.
* **Nothing deleted** — asserted on ids, not counts.
* **A's state is not current** — `test_e01_e04_...` establishes gold 510 on the
abandoned line, undoes, diverges with a +1 turn, and asserts 11: the hoard
belonged to a story this one is not telling.
## G.2 When the fork happens
On the **first write below a moved-back head**, never on Undo. `head.fork_if_behind_head`
returns immediately when the head is at the tip, so a story that is never undone
forks exactly as often as it did before M3 — the branch table does not fill up
with one branch per turn. Undo alone must not fork, because moving the head is
not a decision to abandon anything: the reader may be reading, or about to Redo.
## G.3 How the displaced continuation is marked
`branches.superseded_at` and `branches.superseded_depth` — the disposition
`DATA-MODEL.md` §5 describes, stored as the fact that produced it (the depth the
story departed at, and when) rather than as a word. The shallowest departure wins
if a branch is left twice.
**Nothing reads these columns to decide behaviour.** Redo is decided by the
lineage, so a stale or hand-edited value cannot make the story wrong. They exist
so the cleanup and discarded-history features `STORY-BRANCH-SEMANTICS.md` §28–29
defer to a later version have something to select on, and so a divergence is
observable in a test — which
`test_the_departed_branch_records_where_the_story_left_it` does, asserting the
recorded depth equals the head the story left at.
## G.4 What does *not* exist
**No user-facing branch-tree feature was built.** The inherited branch list and
`/fork` endpoint are unchanged from M2. The browser exposes Undo, Redo, Retry and
edit; it exposes no branch ids and requires no branch management. There is no
merge, no cleanup, and no discarded-history recovery screen.
---
# H. Retry and Alternate Takes
Retry remained non-destructive, and the Phase 0B failure mode — a later Undo
destroying the alternatives Retry had preserved — **cannot recur**, because Undo
has no delete path at all. `test_undo_hides_every_attempt_and_keeps_them_all`
covers it directly: three takes at one coordinate, one Undo, all three rows
retained, and after Redo the same take is still live.
| Behaviour | Result | Evidence |
| --- | --- | --- |
| Take A → Retry → Take B | B generated from the same parent state | `test_d06_d08_...`: two `ai` rows at the same depth, state one turn's worth |
| Take A retained | Yes | first take's id still present |
| Continuing from an alternate take | Forks; unselected takes retained | inherited `after_id` path, `test_branch_forking` (18 tests) |
| Undo after Retry | Whole group steps behind the head, nothing deleted | `test_undo_hides_every_attempt_and_keeps_them_all` |
| Retry while head is behind tip | **Branches instead of amending** | `test_d07_...`: rows all retained, branch count 1 → 2 |
| Divergence from an alternate take | Old line keeps its continuation | `test_branch_forking::test_the_old_line_still_has_its_continuation` |
## H.5 Shared placement logic — and one narrowing
Normal write, Retry and add-take now share `head.behind_tip` as the single
"does this need a branch?" predicate. Retry and add-take fork at
`action.depth - 1` (leaving the path just in front of the turn, so the new take
lands at the same coordinate under the same parent); the write path forks at the
head. Both record the departed branch through `head.mark_superseded`.
`add_take` previously decided "is this the tip?" by comparing `last_action`
against the target id. `last_action` reads the **capped** path, so under a
moved-back head it reports the node at the head as the newest one — and amending
in place would have left a retained future descending from a take that is no
longer live. The comparison is now `and not head.behind_tip(...)`.
**One capability was narrowed.** `POST .../variant` (switch which take is live)
is refused with 400 while the head is behind the retained tip:
> "This turn has a later story that was undone but kept. Redo first, or use
> another take to start a new line from here."
Switching the live take in place cannot be made safe by branching — it changes
which text the retained future descends from. Refusing is consistent with the
milestone's instruction that the control report cleanly rather than guess, and it
leaves the user two working routes. `test_switching_a_take_is_refused_while_a_kept_future_hangs_off_it`
covers it. **A reviewer should ratify this**: it is a small loss of an inherited
capability in a state the inherited product could not reach.
---
# I. Edit Semantics
The current product offers two distinct operations, and the distinction decides
what M3 owed here.
| Control | Endpoint | Meaning |
| --- | --- | --- |
| `✎` | `PATCH .../actions/{id}` | Correct the text of one node. Creates no continuation, regenerates nothing. |
| `⑂` | `POST .../actions/{id}/takes` | Play this turn again, differently. Returns to the parent state and continues. |
## I.1 Earlier user input (D09)
**Tested, through `⑂`.** `test_d09_d10_replaying_a_turn_forks_and_keeps_the_old_line`
runs the acceptance text verbatim — "I accuse Mara of stealing the key." replayed
as "I quietly ask Mara whether she has seen the key." — and asserts:
* the old future's row ids are all still present,
* the story now tells the edited input and no longer tells the old narration,
* the instrumented state is 1, not 20: nothing leaked from the abandoned line.
This is exactly the semantics `STORY-BRANCH-SEMANTICS.md` §13 specifies —
"return to the parent story state and create a new continuation using the edited
input" — reached through the control the product already had.
## I.2 Narrator output (D10)
**Partly covered, and a reviewer should decide whether that suffices.**
Replaying an AI turn through `⑂` regenerates it, forks, and retains the original
take — the retention and continuation halves of D10. What is *not* implemented is
§14/§15's "the user types corrected narrator prose, and the state implied by that
prose is re-evaluated": the `✎` path writes the new text and re-evaluates
nothing.
M3 did not change `✎`, on these grounds:
* it deletes nothing, so it does not violate the non-destructive contract M3 owns;
* it is the pre-existing typo-correction affordance, unchanged since before M2;
* §15's "re-evaluate state changes implied by that output" requires a state
extraction pass over user-authored text, which is M5 machinery;
* the milestone prompt says to implement only what M3 requires under the existing
product surface and not to build the M8 editing UX.
**Not claimed:** no test exercises a hand-typed narrator correction followed by
state re-evaluation, because the behaviour does not exist. Against the current
D10 wording ("edit becomes authoritative on active path, downstream state is
re-evaluated"), M3 satisfies the first and third clauses and not the second. §T
recommends the acceptance test be re-scoped or explicitly assigned to M5.
## I.3 A residual sharp edge
`PATCH .../actions/{id}` will still edit the text of a node that has a retained
future descending from it. Nothing is deleted and no state is recomputed, so the
future silently continues from changed words. This is inherited behaviour, not
introduced by M3 — the same was true pre-M3 for any mid-story node — but the
head model makes the state reachable more often. Recorded as non-blocking debt
(§S.3).
---
# J. State Reconstruction
## J.1 Behaviour across operations
| Operation | Behaviour | Evidence |
| --- | --- | --- |
| Undo | Restores the destination node's `world_state_after` | D01; §E.1 |
| Redo | Restores the destination node's snapshot — same value, other direction | D04; L02 ladder |
| Divergence | New continuation starts from the head's state, not the abandoned line's | `test_e01_e04_...`: 510 → 10 → 11 |
| Retry | Rolls back to before the turn's output, then regenerates | `test_d06_d08_...`; `attempts.roll_back_before` |
| Edit (replay) | Same as divergence — parent state, then continue | `test_d09_d10_...`: 1, not 20 |
| Failed turn | Nothing written; state untouched | `test_l01_...` |
The mechanism is unchanged from M2. M3 added no state code; it changed which node
`restore_state` is called with.
## J.2 Stale state does not survive
The strongest single piece of evidence is `test_e01_e04_...`, which is E01 and
E04 measured through one number. A hoard worth 500 is established only on the
abandoned line; after Undo the value is 10; after a divergent +1 turn it is 11.
Had any of the head resolution, the snapshot restore, or the fork ordering been
wrong, the number would be 511 or 510.
## J.3 The instrumentation caveat — important
Every state assertion in this milestone is expressed through the inherited
RPG-shaped `world_state` and its per-turn "+10 gold" scripted replies. **This is
deterministic instrumentation, not an endorsement of RPG state.** M5 replaces the
protocol with genre-neutral typed narrative events (ADR 010).
What these tests actually validate is *positional* — that the state associated
with a story position is restored when the head moves to it, in both directions,
and that an abandoned line's state does not survive a divergence. When M5 lands,
the **instrumentation must move, not the assertions**: the same properties need
to hold over whatever carries state then. This extends the note M2 attached to
M5 about eight rollback tests; M3 adds roughly a dozen more in
`test_head_cursor.py`, all of them marked in the file's own docstring.
## J.4 M5 implication discovered
One, and it is a constraint rather than a problem: **M5's state representation
must be recoverable per node from a snapshot, not only replayable from an event
log.** `head.move_to` is a row lookup plus a restore, at any distance, in either
direction — that is what makes Undo O(1) rather than O(story). A pure event-log
model would have to replay from the opening on every Undo. `TECHNICAL-DESIGN.md`
already selects a hybrid (validated events plus snapshots/cache); M3 turns that
from a preference into a requirement, and §T recommends recording it.
---
# K. Memory and Lineage Isolation
Both controls were run. This section is the one where "nothing was returned" is
indistinguishable from "retrieval is broken" unless the positive control runs
too, so both are reported.
## K.1 Negative control
`test_e02_an_abandoned_memory_is_unreachable_and_still_on_disk`:
1. three turns played; a memory reading *"Mara reveals she is a spy."* is attached
to the node at the tip, the way the summarizer attaches one, with the memory
cursor anchored there;
2. retrieval through the same clause `memorybank` uses returns it — establishing
that retrieval works before anything is undone;
3. Undo;
4. retrieval returns the empty set;
5. a divergent turn is written;
6. retrieval still returns the empty set — and `memories` still holds one row.
The revelation is unreachable from the new line and was never deleted.
## K.2 Positive control
`test_e02_positive_control_the_memory_returns_on_the_line_it_belongs_to`:
same setup, Undo (unreachable), then **Redo** — and the memory is retrievable
again, with no re-embedding, because nothing was removed. This is what
distinguishes working isolation from broken retrieval.
A second positive control is embedded in the rewritten
`test_undo_stops_retrieving_a_memory_without_deleting_it`, which asserts a
two-memory set narrowing to `{"k"}` and widening back to `{"k", "m"}` — proving
the clause is discriminating by coordinate rather than returning nothing.
## K.3 Summaries (E03)
**Tested, at the level the milestone allows.**
`test_e03_a_summary_anchor_cannot_claim_coverage_past_the_head` asserts that the
summary cursor, resolved against a head-capped path, cannot report a stretch in
the abandoned future as already covered: coverage measured after the divergence
is strictly less than the depth the anchor was written at. The stretch is
therefore re-derived for the new line rather than carried into it.
**Not claimed:** no end-to-end run generated real summaries over a long story and
inspected the assembled prompt for abandoned content. E03's full scenario
("continue until summary is used again") is a long-run behaviour that belongs
with M6 and M11. What M3 demonstrates is the mechanism that makes leakage
impossible — coverage cannot be claimed past the head — not a long-run
observation of it. The summary subsystem itself was not redesigned, as required.
## K.4 Why no memory code changed
A memory carries the coordinate of the node its block ends on. The capped clause
excludes memories past the head for the same reason it excludes actions past the
head. `STORY-BRANCH-SEMANTICS.md` §33 holds as a consequence of the head rather
than as its own mechanism — which is also why Undo needs no memory pruning and
Redo needs no re-embedding.
---
# L. Export / Import Active-Head Round Trip
## L.1 The field
```json
"headDepth": 3
```
A top-level integer beside the existing `headBranch`. The pair is the exported
active head.
This required moving `headDepth` across `bundle.py`'s own stated rule about what
a bundle carries. That rule is *"a bundle carries what was chosen, never what is
derived"*, and the module previously listed the head depth explicitly as derived
— "the tip of the head branch, a fact about the nodes that arrived with it".
That was true while Undo deleted. It is false now: the same tree exports
identically whether the user undid three turns or none, so where the reader
stopped is a decision no import can recompute. The module docstring was rewritten
to say so.
## L.2 The undone-head case (I07)
`test_i07_an_undone_head_survives_export_and_import`: five turns, two Undos,
export, import, and then:
* the imported campaign's story, read through its own head, equals the source's
undone story exactly;
* `can_redo` is true on the import response;
* the imported adventure holds the same number of action rows as the source —
the retained future arrived;
* two Redos walk it forward to the *whole* five-turn story.
Confirmed again at runtime on the live application (§M.3): exported
`headDepth = 1` with 9 nodes across 3 branches; the imported campaign opened
showing 2 rows with `can_undo=false, can_redo=true`, identical to the source.
**Import did not advance to the newest retained turn.**
"Fresh data directory" is modelled as an adventure that shares no row with the
original: the import allocates its own branch rows and nodes and resolves the
file's local branch numbers against them, which is the whole of what the round
trip has to get right. The test file states this assumption explicitly.
## L.3 The pre-M3 case
`test_a_bundle_written_before_m3_opens_at_its_tip` deletes the key from a real
export and imports it: the campaign opens at the tip of its head branch.
The fallback semantics matter and are worth stating precisely: **this is not a
degraded path.** A bundle written before M3 was written when the head could not
be anywhere but the tip, so deriving the tip reproduces the position that file
actually recorded. Version 1 bundles take the same path. No `FORMAT` bump was
needed, since an absent key is unambiguous.
## L.4 Hand-edited files
`_planned_head_depth` validates in `plan()` — no session, no side effects, before
any row exists — with two bounds. A depth past the head branch's retained tip is
refused with `400 ... but branch N ends at M`; a non-integer or a value below
`NO_DEPTH` is refused. A depth *behind* the tip is of course accepted: that is
the feature. `test_a_bundle_that_reads_past_its_own_story_is_refused` covers it.
## L.5 Branch disposition (added during implementation)
The round trip initially lost `superseded_at`/`superseded_depth`: the imported
tree had all nine rows and all three branches, and zero of them marked. Every row
of an abandoned line arrives either way, so the disposition is the only thing
distinguishing it from an active one — a restored backup would have had nothing
for the later cleanup and recovery features to select on. Both keys are now
exported, planned and written, omitted when the branch is active, and taken as
absent if only one is present (a time with no depth cannot say what was
displaced). Two tests cover it.
This is a small addition beyond the literal M3 scope list, justified as part of
making active-head history round-trip correctly, and it is a decision rather than
a derivation, so it sits on the correct side of the module's rule.
## L.6 I01–I03
* **I01/I02** — `test_i01_i02_a_campaign_round_trips`: three turns, exported and
imported, story and state identical.
* **I03** — `test_i03_the_bundle_carries_the_history_the_story_no_longer_tells`:
after a divergence, the bundle carries both branches, the import reproduces the
full row count and both branches, and the active line is the one that was
active.
---
# M. Browser UX Verification
## M.1 Changes
* **Redo control** — `↷ Redo` beside `↶ Undo`, on `Ctrl+Shift+Z`.
* **Enablement** — both buttons read the server's `can_undo`/`can_redo`. The
client cannot derive either: Undo stops at the campaign opening, which may be
off the top of the loaded window, and Redo depends on the retained future,
which the client is never sent. The flags now ride on `AdventureOut`, on every
`ActionPage` (including a scrolled-up page), and on the import response.
* **After divergence** — an accepted turn sets `{undo: true, redo: false}`
locally, because the turn arrives over SSE and that is the same answer the
server would give: writing below a moved-back head retires the old future, and
writing at the tip never had one.
* **Retry** — unchanged; it resyncs through `getAdventure`, which carries the
flags.
* **No branch ids or branch management** are exposed anywhere.
* **Incidental fix:** moving the head now bumps `stateKey`, so the state panels
re-read. Undo has rolled the world state back since long before M3 and the
drawer kept showing the previous position's numbers.
## M.2 The browser smoke test — NOT PERFORMED
```text
Browser used: none
Application rendered: no
Manually clicked: no
```
The session had no browser available to drive. **This is an outstanding M3
acceptance requirement**, not a judgement that it was unnecessary. The Redo
control is verified by its endpoint, by lint, and by a successful production
build — not by a click.
No frontend testing framework was added, and none should be added for M3: the
project has no frontend tests at all (M2 debt, assigned to M8), and introducing
one for a single button would be the wrong place to start.
**Required to close:** run the application and perform, by hand —
```text
generate several turns
Undo
Undo
Redo
Retry
Undo
create a new continuation
verify Redo is unavailable
reopen the campaign
```
## M.3 What was done instead
The identical sequence was executed end-to-end against the running application
(`uvicorn` on loopback, a fresh database, real streamed inference from
`qwen2.5:3b-instruct` on the trusted-LAN Ollama over HTTPS with a privately
issued certificate). Observed:
```text
1. three turns rows_shown=6 can_undo=True can_redo=False retained=6
2. undo, undo rows_shown=2 can_undo=False can_redo=True retained=6
3. redo rows_shown=4 can_undo=True can_redo=True
4. retry rows_shown=4 can_undo=True can_redo=False retained=7
5. undo rows_shown=2 can_undo=False can_redo=True
6. divergent turn rows_shown=4 can_undo=True can_redo=False retained=9
7. POST /redo -> 400
8. undo to the opening rows_shown=2 can_undo=False can_redo=True
export headDepth=1 nodes=9 branches=3
9. process restart, campaign reopened rows_shown=2 can_undo=False can_redo=True
10. import the bundle opens at 2 rows, can_redo=True, 9 rows retained
```
Two observations worth noting. In step 2, `can_undo` correctly becomes false
after two Undos because this adventure's opening *is* its first turn — the floor
is reached. In step 4, a Retry from behind the tip branched and correctly retired
Redo.
This exercises every server-side behaviour the browser test would, through the
same HTTP contract the browser uses, with real model output and a real restart.
It does not exercise the DOM: the buttons' disabled states, the keyboard
shortcuts, and the state-panel refresh are unverified by observation.
---
# N. Regression and Build Results
## N.1 Backend
```text
$ .venv/bin/python -m pytest tests/ -q
631 passed, 1 warning in 121.53s
```
| | |
| --- | --- |
| Collected | 631 |
| Passed | 631 |
| Failed | 0 |
| Skipped | 0 |
| Duration | ~2 min |
The one warning is inherited and unrelated (`StarletteDeprecationWarning` about
`httpx` in `fastapi.testclient`). No expected exceptions, no xfails, no skips.
For continuity: 648 tests at M1, 604 at M2 (M2 removed subsystems), 607 after
this milestone's checkpoint commit plus the test rewrites, 631 with the M3
acceptance suite added.
## N.2 Frontend
```text
$ npm run lint # oxlint
7 warnings, 0 errors
```
All seven pre-date M3 and none is in a file M3 touched: one unused import in
`components.jsx` and six `react(only-export-components)` fast-refresh warnings in
`components.jsx` and `SchemaEditor.jsx`.
```text
$ npm run build
✓ 49 modules transformed
dist/assets/index-B3_m6gAO.css 59.90 kB │ gzip: 12.13 kB
dist/assets/index-CLLBHDif.js 395.85 kB │ gzip: 120.14 kB
✓ built in 448ms
```
395.85 kB, unchanged from M2's post-removal figure to three significant figures.
## N.3 Docker
```text
$ docker build -t storyteller-m3 .
DONE — storyteller-m3:latest, 310MB
```
Built twice: once after the frontend changes and again after the final backend
changes. Both succeeded.
## N.4 Targeted M3 tests
`backend/tests/test_head_cursor.py` — 24 tests, all passing, named for the
acceptance items they discharge.
| Requirement | Test |
| --- | --- |
| Zero-row deletion on Undo | `test_the_m3_invariant_undo_deletes_zero_accepted_turns` |
| One Undo, transcript + state | `test_d01_one_undo_returns_the_transcript_and_the_state` |
| Five-plus Undo | `test_d02_five_consecutive_undos_each_land_where_they_should` |
| Undo to the root, then stop | `test_d03_undo_walks_back_to_the_campaign_opening_and_stops` |
| Multiple Redo | `test_d04_redo_restores_the_continuation_and_its_state` |
| Divergence | `test_d05_a_new_turn_below_the_head_retires_redo_and_keeps_the_future` |
| Divergence is observable | `test_the_departed_branch_records_where_the_story_left_it` |
| Retry retains takes | `test_d06_d08_retry_keeps_the_earlier_take_and_reuses_the_parent_state` |
| Retry under a moved head | `test_d07_a_retry_from_behind_the_tip_branches_instead_of_amending` |
| Take switch refused safely | `test_switching_a_take_is_refused_while_a_kept_future_hangs_off_it` |
| Edit → new continuation | `test_d09_d10_replaying_a_turn_forks_and_keeps_the_old_line` |
| State reconstruction | `test_l02_state_matches_the_position_in_both_directions` |
| Abandoned state not current | `test_e01_e04_a_fact_from_the_abandoned_future_is_not_current` |
| Memory negative control | `test_e02_an_abandoned_memory_is_unreachable_and_still_on_disk` |
| Memory positive control | `test_e02_positive_control_the_memory_returns_on_the_line_it_belongs_to` |
| Summary isolation | `test_e03_a_summary_anchor_cannot_claim_coverage_past_the_head` |
| Atomicity on failure | `test_l01_a_failed_turn_accepts_no_narration_and_strands_no_state` |
| Export/import round trip | `test_i01_i02_a_campaign_round_trips` |
| Retained history exported | `test_i03_the_bundle_carries_the_history_the_story_no_longer_tells` |
| **Undone head round trip** | `test_i07_an_undone_head_survives_export_and_import` |
| **Legacy bundle** | `test_a_bundle_written_before_m3_opens_at_its_tip` |
| Corrupt head refused | `test_a_bundle_that_reads_past_its_own_story_is_refused` |
| Disposition round trip | `test_the_bundle_carries_which_branches_the_story_left` |
| Half a disposition | `test_a_bundle_with_half_a_disposition_imports_as_active` |
Contract files named by the milestone, all green:
```text
test_head_cursor 24 test_take_edit 5
test_story_tree_baseline 24 test_take_state 5
test_bundle_v2 21 test_delete_state 5
test_take_parentage 19
test_branch_forking 18
test_attempt_siblings 15
test_retry_variants 15
test_state_revert 12
```
---
# O. M2 Security / Local-Only Regression Check
M3 touched no networking, no provider, no CORS and no CSP code. The check is
therefore targeted rather than a full rerun, and that is stated as a reason, not
an omission: a packet capture re-measures egress, and no code on any egress path
changed. M1's capture and M2's endpoint-policy evidence remain the runtime record.
| Property | Status | Evidence |
| --- | --- | --- |
| Storyteller loopback-bound by default | Intact | `start.sh --host 127.0.0.1`; `docker-compose.yml` publishes `127.0.0.1:8000:8000`; `test_local_only_surface` (32) |
| Ollama the only inference backend | Intact | No provider code changed; `test_endpoint_policy` (31) |
| Endpoint allowlist present | Intact | `app/endpoints.py` unchanged; ADR 011 behaviour |
| Request-time enforcement present | Intact | `test_endpoint_policy` covers the DB-edited-behind-the-API case |
| Trusted-LAN support present | Intact | **Re-confirmed live**: HTTPS to a second machine with a privately issued certificate, `/api/settings/test` → `{"ok":true,...}` |
| TLS verification enabled, no bypass | Intact | `app/tlstrust.py` unchanged; `test_tls_trust` (6) |
| No cloud-provider code | Intact | The only cloud host names in `app/` are `endpoints.py`'s **denylist**, present so the refusal message says *why* |
| No QuickJS/scripting | Intact | No `/api/script*` route; no scripting module |
| No auth/analytics/hosted routes | Intact | Route census: **zero** paths matching auth, script or analytics |
| Restrictive CORS / `/api` 404 | Intact | `app/main.py` unchanged; `test_egress` (14), `test_offline_assets` (10) |
Route census, from the generated OpenAPI document: **37 distinct `/api` paths, 53
operations.** M2 closed at 36 paths; M3 adds exactly one, `POST /api/adventures/{adventure_id}/redo`.
Targeted security suites: **93 tests, all passing.**
One note for completeness: `app/auth.py`'s module docstring still mentions
`AIDND_MULTI_USER` while explaining what upstream had and why this build does
not. It is prose, not a code path, and the variable is read nowhere.
---
# P. Database / Migration Assessment
## P.1 Changes
```text
(78, "ALTER TABLE branches ADD COLUMN superseded_at TIMESTAMP")
(79, "ALTER TABLE branches ADD COLUMN superseded_depth INTEGER")
```
That is the entire schema footprint. `LATEST_VERSION` moves 77 → 79.
## P.2 How M2 databases migrate
Both statements are `ALTER TABLE ... ADD COLUMN` with no `NOT NULL` and no
default, so both are additive and instantaneous. **No backfill.** `NULL` means
active, which every branch in an M2 database is: before M3 the head could not sit
behind the tip, so no branch had ever been superseded.
## P.3 How the active head is established for existing campaigns
**It already is.** `adventures.head_depth` has existed since migration 49 and has
always held the tip. An M2 campaign therefore opens with head == tip, which is
exactly where it was left, and `can_redo` is false until the user undoes
something. No migration, defaulting or repair was needed — this is the property
that let M3 change the column's *meaning* without touching its data.
## P.4 Rollback to pre-M3 software
**Schema-safe, semantically unsafe. This asymmetry should be recorded.**
The two new columns are additive, so older code that never selects them reads the
database without error. But a campaign whose `head_depth` sits behind its
retained tip would be read by pre-M3 code as though it were not — pre-M3
`lineage.Path` used the tip only to estimate coverage and never capped a read —
so the story would silently appear fully redone, and the next turn would be
written at the tip. No data is lost; the reader's position is.
Recommended framing for release notes: a database that has been opened by M3 may
be *read* by pre-M3 software, but any campaign left in an undone state will
appear redone there.
## P.5 No unrelated cleanup
Confirmed. The inert legacy tables and columns M2 left behind are untouched, as
its debt entry requires (cleanup deferred until the schema settles after M3/M5).
No migration was renumbered, edited or removed.
---
# Q. Problems Found During Implementation
## Q.1 Bugs introduced by M3 — found and fixed before final testing
### Q.1.1 Missing import broke 67 tests
```text
Problem `crud.get_adventure` called `head.can_undo` with no import of
`head`; NameError on every adventure read.
Root cause A field added to the response without the module it needs.
Impact 67 of 72 failures in the checkpoint tree. Would have been caught
by any run of the suite; it was committed without one.
Fix Added `head` to the existing `from ... import` line.
Test added None specific — 67 existing tests cover the path.
Risk None remaining.
```
### Q.1.2 Branch disposition lost on export/import
```text
Problem A round trip produced a tree with all rows and all branches, and
zero of them marked superseded.
Root cause The columns were added to the model and the migration but not to
the bundle format.
Impact A restored backup could not distinguish abandoned history from
active history — the only thing the later cleanup and recovery
features have to select on.
Fix Export, plan and write `supersededAt`/`supersededDepth`; both keys
or neither.
Test added test_the_bundle_carries_which_branches_the_story_left
test_a_bundle_with_half_a_disposition_imports_as_active
Risk None. No read depends on these columns.
```
### Q.1.3 `list_actions` returned the flags as false
```text
Problem The paged endpoint built its ActionPage by hand and omitted
can_undo/can_redo, which default to false.
Root cause Two constructors for one response shape; only one was updated.
Impact Scrolling up the transcript would have greyed out a Redo that was
still available.
Fix Both flags on every page.
Test added None specific; covered indirectly.
Risk Low. The two constructors remain separate — noted as debt (S.3).
```
### Q.1.4 The import response reported no history
```text
Problem POST /adventures/import returned can_undo/can_redo false, so a
campaign imported while undone opened with Redo greyed out.
Root cause The endpoint returned the ORM object directly.
Impact Would have made I07 invisible to the user at exactly the moment it
matters.
Fix Build AdventureOut and set both flags.
Test added Asserted inside test_i07_an_undone_head_survives_export_and_import.
Risk None.
```
## Q.2 Inherited bugs exposed by M3
### Q.2.1 Undo left the state panels stale
```text
Problem The browser's undo() never bumped stateKey, so the world-state
drawer kept showing the previous position's numbers.
Root cause Pre-existing. Undo has rolled world_state back since before M3.
Impact Cosmetic but misleading — the visible state contradicted the story.
Fix moveHead() bumps stateKey for both Undo and Redo.
Test added None (no frontend tests exist).
Risk Unverified by a browser; see M.2.
```
### Q.2.2 `delete_action` would have dragged a moved-back head forward
```text
Problem delete_turn is followed by tree.refresh_head, which recomputes the
tip — which since M3 is not the head. An unrelated delete would
have silently redone an undone story.
Root cause A function that computed one value now used where two exist.
Impact Would have been a silent Redo.
Fix Record the head before, restore it after unless the delete removed
the ground under it.
Test added Covered by the existing delete suites (test_delete_state, 5).
Risk Low.
```
### Q.2.3 `last_action` reports a non-leaf as newest
```text
Problem last_action reads the capped path, so under a moved-back head it
names the node at the head as the newest — and retry/add-take used
that to decide whether to amend in place.
Root cause A helper whose answer was unambiguous before the head could move.
Impact Would have amended a turn with an accepted future, leaving that
future descending from a take no longer live.
Fix Both call sites now also ask head.behind_tip.
Test added test_d07_..., test_switching_a_take_is_refused_...
Risk Low, but the helper's name still suggests more than it delivers.
Noted as debt (S.3).
```
## Q.3 Planning assumptions that proved wrong
### Q.3.1 The bundle's own rule needed amending
`bundle.py` named the head depth as *derived* and listed it as an example of what
a bundle deliberately does not carry. True while Undo deleted; false afterwards.
The module docstring was rewritten rather than worked around. §T recommends
mirroring this in `DATA-MODEL.md`.
### Q.3.2 "Undo stops at the fork" was correct only for destructive Undo
The inherited refusal protected a parent branch from a child's deletes. With no
deletes it protects nothing and prevents a legitimate read. Reversed; see §E.4.
### Q.3.3 D10 assumes an editing surface the product does not have
See §I.2. Not a defect — an acceptance item written against an intended UX rather
than the current one.
### Q.3.4 The head is not the only thing a failed turn moves
L01 was framed as "no half-advanced head". A failed turn *does* advance the head
by one, onto the player's retained input, because A05 deliberately keeps that
text. The two requirements meet without conflicting, but only once L01 is read as
being about *accepted narration*. The test records this explicitly.
## Q.4 Non-blocking debt discovered
* `README.md` is substantially stale from **M2** — it still advertises JavaScript
scripting, a QuickJS sandbox, optional accounts, `AIDND_MULTI_USER`, an
analytics dashboard and "549 backend tests". M3 corrected only the three
bullets it falsified. **This is not in M2's debt table**, which is itself worth
noting: the M2 review missed it.
* `POST /adventures/import` returns the whole `actions` relationship rather than
a head-capped window, so its payload contains every branch's rows. Inherited,
harmless in practice (the UI reads only `id` and re-fetches), but wrong in
shape and now more visibly so.
* `ActionPage` is constructed in two places.
---
# R. Deviations From the M3 Plan
| # | Planned | Actual | Why | Architectural? | Planning change? |
| --- | --- | --- | --- | --- | --- |
| 1 | *"Prevent Undo from walking before the campaign opening/root semantics"* — inherited code also refused at a fork | Undo walks past a fork to the campaign opening | With nothing deleted there is nothing to protect the parent branch from, and the inherited prefix is part of the forked story | No — it realises §5's "unlimited Undo across retained history" | `STORY-BRANCH-SEMANTICS.md` §5 could state it |
| 2 | Not mentioned | `POST .../variant` refused while a retained future hangs off the turn | Switching the live take in place cannot be made safe by branching | No | Worth a line in §10 |
| 3 | *"Mark displaced futures/takes as retained/disposable"* | Two columns nothing reads | Redo is decided by the lineage; a flag that decided behaviour could make the story wrong | No — it is the §5.E "implementation-appropriate metadata" | `DATA-MODEL.md` §5 could record the representation |
| 4 | *"Export active head coordinate/depth"* | Also exports the branch disposition | A restored backup could not otherwise distinguish abandoned from active history | No | Bundle format note |
| 5 | *"Retry/add-take/edit paths use the same safe fork/head rules"* | Replay path yes; in-place prose edit unchanged | It deletes nothing and re-evaluates nothing; §15's re-evaluation is M5 machinery | Possibly — see §I.2 | D10 should be re-scoped |
| 6 | Browser smoke test required | Not performed | No browser available in the session | No | None — the work remains outstanding |
| 7 | Commit the M3 work | Staged, not committed | Signed commits need a key the agent cannot use | No | None |
## R.1 Deliberately not implemented
* Named checkpoints or Save Points (M4).
* Any branch-tree, checkout or merge UI.
* Abandoned-history cleanup, retention pruning or a recovery screen.
* Genre-neutral narrative state (M5) — the RPG world-state protocol is untouched.
* Memory, summarization or embedding redesign (M6).
* Imported knowledge (M7).
* Broader browser redesign (M8) — one button was added.
* Bundle-format redesign (M9) — two additive keys, no `FORMAT` bump.
* Media hooks (M10).
* Legacy schema or migration-history cleanup.
## R.2 Drift check
**No drift into M4–M10 was found.** The one addition beyond the literal scope
list (§L.5) serves the milestone's own export requirement and does not implement
any part of a later milestone.
---
# S. Technical Debt After M3
## S.1 Must resolve before M4
**Neither item is a code defect.**
1. **The M3 commit must be made.** The tree is staged and unsigned-commit-blocked.
M4 cannot branch from a milestone that has no commit, and the runtime evidence
in this report describes a tree that does not yet exist in history.
→ **blocker**
2. **The browser smoke test must be run** (§M.2). It is an explicit M3 acceptance
requirement and the only requirement with no evidence behind it.
→ **blocker**
Nothing in the head model itself blocks M4.
## S.2 Later planned debt (already assigned)
| Item | Milestone |
| --- | --- |
| RPG world-state instrumentation in ~20 history tests — move the instrumentation, keep the assertions | M5 |
| Genre-neutral state must remain snapshot-recoverable per node (§J.4) | M5 |
| Long-run summary isolation over a real story (§K.3) | M6 / M11 |
| No frontend tests at all | M8 |
| `Settings.model` defaults to `""` with nothing prompting for it | M8 |
| Inert legacy tables/columns awaiting a cleanup migration | after M5 |
| `docs/*.html` still links Google Fonts | M8 or a doc pass |
## S.3 Newly discovered debt
| Item | Kind |
| --- | --- |
| `README.md` stale from M2 — advertises scripting, accounts, analytics, `AIDND_MULTI_USER`, a wrong test count | **planning update** — belongs in M2's debt table and a doc pass; not M3's to rewrite |
| In-place `PATCH .../actions/{id}` can edit a node with a retained future descending from it, changing text that future was written from (§I.3) | **non-blocking** — inherited; decide in M5 or M8 |
| `POST /adventures/import` returns every branch's rows rather than a head-capped window | **non-blocking** — inherited, harmless in the UI |
| `ActionPage` built in two places; a third would drift again | **non-blocking** |
| `last_action` reads the capped path but is named as though it reports the newest node (§Q.2.3) | **non-blocking** — a rename would be clearer |
| Pre-M3 software reads an M3 database but shows undone campaigns as redone (§P.4) | **planning update** — release-note material |
Nothing here is a "code could be prettier" item promoted to a blocker.
---
# T. Planning Document Recommendations
Recommendations only. **No planning document was modified by this report.**
### `planning/SPECIFICATION.md`
**No change recommended.** M3 altered no product requirement; it implemented one.
### `planning/TECHNICAL-DESIGN.md`
**Change recommended.** Add a section recording the active-head model as
implemented, in the way §5.2 records the M1/M2 architecture as fact: the head as
stored coordinate, the capped lineage as the single chokepoint, `uncapped()` as
the deliberate two-caller exception, and state as a per-node snapshot restored by
row lookup. Add the **M5 constraint from §J.4** — narrative state must remain
recoverable per node from a snapshot, not only replayable from an event log, or
Undo becomes O(story).
### `planning/BUILD-MILESTONES.md`
**Change recommended.** Mark M3 COMPLETE with the capabilities later milestones
inherit rather than build: non-destructive head movement, Redo, lineage-decided
divergence, the head-capped read, the active-head bundle. Add a note to **M4**
that a Save Point is a durable coordinate and that restoring one is
`head.move_to` plus a bounds check, so M4 should not introduce a second
head-movement path. Extend the existing **M5** instrumentation note to cover
`test_head_cursor.py`.
### `planning/STORY-BRANCH-SEMANTICS.md`
**Change recommended.** Three points, all small:
* §5 — state that Undo traverses a fork point into the story a branch inherits,
and that the floor is the campaign opening rather than the fork. This reverses
inherited behaviour and should be recorded as intended.
* §10/§11 — record that switching the live take is refused while a retained
future descends from that turn, and why branching is the safe alternative.
* §33 — note that memory lineage safety now holds as a *consequence* of the
head-capped path rather than as a separate mechanism; no pruning, no
re-embedding.
### `planning/CONTEXT-AND-MEMORY.md`
**Change recommended, minor.** Record that context assembly, transcript,
`attempts.preceding` and memory retrieval all narrow through one capped path, so
a memory past the head is unreachable and becomes eligible again on Redo. Note
that E03 is discharged at the mechanism level in M3 and awaits a long-run
observation in M6/M11.
### `planning/DATA-MODEL.md`
**Change recommended.** §5's branch disposition is now concrete: `superseded_at`
+ `superseded_depth`, `NULL` meaning active, shallowest departure winning, and
explicitly advisory — nothing reads them to decide behaviour. Also record that
the bundle carries both the active head depth and the disposition, and amend the
"derived, not carried" characterisation of the head depth (§Q.3.1).
### `planning/V1-ACCEPTANCE-TESTS.md`
**Change recommended.**
* **D10** — re-scope. The retention and continuation halves pass; "downstream
state is re-evaluated" for hand-typed narrator prose requires M5 state
extraction. Either split D10 into a retention clause (M3) and a re-evaluation
clause (M5), or state that D10 is satisfied through the replay path and that
free-text narrator correction is deferred.
* **L01** — clarify that a failed turn *does* advance the head by one onto the
player's retained input, and that the invariant is about accepted narration.
As written, L01 and A05 appear to conflict.
* **D03** — record that unlimited Undo is achieved, not merely the D02 minimum.
* **I07** — add the legacy-bundle clause explicitly (a bundle with no head field
opens at its tip, and that is the position it recorded).
### `planning/SECURITY-THREAT-MODEL.md`
**No change recommended.** M3 touched no path in the threat model. §10A's policy
and its two residual limits are unaffected.
### ADRs
**One new ADR recommended: `012-active-head-history.md`.** The head-cursor model
is a foundational architectural decision — head as stored position; retained tip
distinct from active head; capped reads with one narrow exception; divergence on
first write rather than on Undo; disposition metadata that decides nothing; the
head as an exported decision rather than a derived value. It is currently
recorded only in code comments and this report, and M4 will build directly on it.
Existing ADRs need no change. **ADR 005** (branch-preserving history) is
fulfilled rather than revised; **ADR 010** gains the snapshot constraint noted
above, which could equally live in `TECHNICAL-DESIGN.md`.
---
# U. M4 Readiness Assessment
1. **Is active-head movement stable enough to build Save Points on?**
Yes. Every head movement in the application goes through `head.move_to`, and
every "does this write fork?" decision through `head.fork_if_behind_head`.
631 tests pass, including a 15-row/five-Undo/five-Redo exact round trip.
2. **Can a Save Point be a durable pointer to an accepted story position?**
Yes, and it is the natural representation: `(branch_id, depth)` is exactly
what the head already is, and `head.node_at` resolves it. A Save Point is a
named row holding a coordinate.
3. **Is there a clear implementation path for restoring one?**
Yes: validate that the coordinate is on the current lineage, then
`head.move_to`. The state comes back with it, because it comes from the node.
M4 should reuse that function rather than adding a second mover — the Phase 0B
spike's mistake was exactly a second, divergent path.
4. **Will restoring a Save Point preserve later history?**
Yes, by construction. Restoring is head movement, and head movement deletes
nothing. D13 ("restore does not delete later history") is already satisfied by
the mechanism M4 will use. Writing after a restore forks through the same
check, which is D13's other half.
5. **Are there unresolved M3 bugs that would make Save Points unsafe?**
None found. The four bugs in §Q.1 are fixed and covered. The debt in §S.3 is
inherited and unrelated to head movement.
6. **Should M4 proceed?**
```text
PROCEED TO M4 AFTER THESE CORRECTIONS
```
The corrections are the two items in §S.1 — **commit the M3 work**, and **run the
browser smoke test** — plus the reviewer's ratification of the three narrowings
flagged in §A. None is a code change. If the browser test reveals a defect in the
Redo control, that becomes M3 corrective work; the backend contract it exercises
is already covered by 24 targeted tests and a live end-to-end run.
M4 needs no scope change. It should be briefed with one instruction added: reuse
`head.move_to` and `head.fork_if_behind_head` rather than introducing a parallel
path for checkpoint restore.
---
# V. Final Repository State
```text
Branch: m3-nondestructive-history
Starting commit: 2fdd254 (post-M2 closeout)
M3 commit: 903fa7a (checkpoint) + staged work, NOT committed
Current HEAD: 903fa7a
Commit signature: 903fa7a verifies (G, RSA 7D8AE19DB5C68569)
Working tree: NOT CLEAN — 11 paths staged, 0 unstaged, 1 untracked (this report)
Backend tests: 631 passed, 0 failed, 0 skipped
Frontend lint: 7 warnings, 0 errors (all pre-existing)
Frontend build: success, 395.85 kB
Docker build: success, storyteller-m3:latest, 310MB
Browser smoke test: NOT PERFORMED — no browser available
Undo deletes accepted rows: 0 (15 rows before, 15 after five Undos)
Redo: exact round trip, head 14 -> 4 -> 14
Divergence preserves old future: yes, all row ids retained; branch marked
Memory isolation: yes, negative and positive controls both pass
Undone-head export/import: yes, headDepth round-trips; no silent Redo
Legacy bundle import: yes, absent key opens at the tip
M2 security regression: none; 93 targeted tests pass; live HTTPS LAN inference re-confirmed
Recommended next milestone: M4, after the two corrections
Blockers: 2 — the M3 commit is unmade; the browser smoke test is unrun
```
```
$ git status --short
M README.md
M backend/app/bundle.py
M backend/app/routers/adventures/__init__.py
M backend/app/routers/adventures/actions.py
M backend/app/routers/adventures/bundle_io.py
M backend/tests/test_attempt_siblings.py
M backend/tests/test_branch_forking.py
A backend/tests/test_head_cursor.py
M backend/tests/test_state_revert.py
M frontend/src/api.js
M frontend/src/pages/Play/index.jsx
?? planning/reports/M3-IMPLEMENTATION-REPORT.md
```
## V.1 Why M3 is not described as closed
Two reasons, both stated above and neither a code defect:
1. **The tree is not clean.** The final commit is prepared but unmade, because
commits here are GPG-signed and signing needs a key the implementing agent
cannot use. Until it is made, `HEAD` does not contain the export/import work,
the browser control, or the acceptance suite — and every measurement in this
report was taken from the staged tree, not from `HEAD`.
2. **One acceptance requirement has no evidence.** The browser smoke test was not
performed. Its server-side equivalent was, with real inference and a real
restart, but that is not the same test and is not reported as such.
M3's *behaviour* is complete and demonstrated. M3's *closure* requires a signed
commit and a click-through.
---
# W. Closeout Addendum (2026-09-03)
Sections A–V above record the state at review time. This section records what
the closeout task changed afterwards. Where the two disagree, this section is
current.
## W.1 The narrator-edit gap is closed
§I.3 and §S.3 recorded an inherited hazard: an in-place edit could rewrite a turn
while a continuation descended from it that the reader could not see, silently
changing the words that retained story was written from.
**Behaviour chosen: refuse, and say why.** `PATCH /api/adventures/{id}/actions/{id}`
now returns 400 when live story descends from the target turn and is not on the
path currently being read. The message names both resolutions the user has —
Redo to bring the later story back, or play the turn again to start a new line.
The predicate is one question rather than two. A descendant is invisible either
because it sits past the head on this lineage (undone) or because it sits past a
fork on a branch the story left (displaced), and both are "a live node, deeper
than this one, descending from it, not on the path being read". Only the deepest
live node on each descending branch is examined, because visibility is monotone
in depth.
A first attempt scoped the check to branches *other than* the active one and
failed the divergence case, correctly: the departed branch usually remains an
**ancestor** of the branch now being read, so "other branches" is the wrong set.
The test that caught it is kept.
What was deliberately **not** built: the fork-on-edit behaviour of
`STORY-BRANCH-SEMANTICS.md` §14, and the state re-evaluation of §15. §15 needs an
extraction pass over user-typed prose, which is M5's. The v1 requirement is
unchanged and unweakened; §14A now records the interim behaviour, and
`BUILD-MILESTONES.md` assigns the completion to M5.
Seven regression tests cover it, including the two cases that must stay
**allowed** — a correction at the tip, and a correction mid-story where every
descendant is on screen — because a guard that over-fires would remove a
capability the product legitimately has.
## W.2 Planning documents updated
| Document | Change |
| --- | --- |
| `DECISIONS/012-active-head-non-destructive-history.md` | **New ADR.** The architecture M3 selected to implement ADR 005's requirement: head stored not derived, reads capped in one place, one movement mechanism, state from the node, divergence on first write below the head, Redo decided by the lineage, advisory disposition metadata, the head as an exported decision. |
| `STORY-BRANCH-SEMANTICS.md` | §5 — Undo crosses fork points to the campaign opening, and why the old refusal was a consequence of deletion. §10 — refusing to switch a take while a later story is off screen, with the two resolutions. **New §14A** — in-place editing before §14-15 exist, stating explicitly that the full requirement stands. |
| `TECHNICAL-DESIGN.md` | **New §8.7** and **§9.1** recording the implemented model and bundle behaviour as fact. §10.4 gains the M3 constraint: the snapshot half of the hybrid is a requirement, not an optimization. |
| `DATA-MODEL.md` | §4 — the active head is stored on the campaign, not derived. §5 — `disposition` as implemented, and the two properties worth carrying (nothing reads it to decide behaviour; it survives export). §29 — the export as implemented, and why the head moved from derived to chosen. |
| `BUILD-MILESTONES.md` | M3 marked COMPLETE with inherited capabilities, the outstanding browser condition, and carried debt. **M4 note** — a Save Point is a durable pointer; restore by reusing M3's head movement, never a second restore path. **M5 note** — keep state efficiently recoverable, move the instrumentation not the assertions, and finish the narrator edit. |
| `V1-ACCEPTANCE-TESTS.md` | D03 — result recorded as full pass, not partial. **D10 — milestone ownership stated without weakening any pass condition; D10 is explicitly not satisfied at the end of M3.** I07 — the pre-M3 bundle clause added, with the refusal case. L01 — the head/A05 apparent conflict resolved. Status line to v1.2. |
| `README.md` | Corrected to describe the current application: the scripting, accounts, analytics, hosted-demo, cloud-provider, Postgres and Render material is removed; the endpoint policy and TLS behaviour described; the architecture map, migration count and test count corrected. |
`SPECIFICATION.md` and `SECURITY-THREAT-MODEL.md` were **not** changed. M3 altered
no product requirement and touched no path in the threat model.
## W.3 Superseding facts
| §A / §V said | Now |
| --- | --- |
| Backend tests: 631 | **638** — seven narrator-edit guard tests added |
| `test_head_cursor.py`: 24 tests | **31** |
| Edit paths: "Yes, with a scope judgement" | Unchanged in substance; the unsafe in-place case is now refused rather than documented as debt |
| §S.3 debt: in-place edit hazard | **Resolved** |
| §S.1 blocker: browser smoke test | **Still open** — see W.4 |
| §S.1 blocker: commit unmade | Resolved at closeout if the signed commits below are made |
## W.4 The browser smoke test is still unperformed
No browser was available in the closeout session either. The strongest available
equivalent was performed and is reported in §M.3 — the full sequence driven
against the running application with real streamed inference from a trusted-LAN
Ollama over HTTPS, including a process restart and a bundle round trip.
This is not the required test and is not counted as one. The DOM-level behaviour
of the Redo control — its enabled and disabled states, `Ctrl+Shift+Z`, and the
state panels refreshing when the head moves — remains unverified by observation.
`BUILD-MILESTONES.md` records M3 as complete with this condition stated openly
rather than marking it closed.
It does not block M4, which touches none of that wiring. It does block calling
M3 fully closed.