M3's review recommended planning changes and, following the M2 pattern, reported rather than applied them. This applies them, and adds the ADR the review asked for. ADR 012 records the architecture rather than the requirement. ADR 005 already says that going backward must preserve abandoned history and that the user sees Undo/Redo/Retry rather than branch management; it names a movable active head as the direction and stops. What M3 settled is the shape: the head is stored rather than derived, every read of the story is capped at it in one place, one mechanism moves it, the state of a position comes off the node rather than from a replay, the first write below a moved-back head is the divergence, and whether Redo exists is decided by the lineage rather than by a flag that could be stale. The last of those is the property worth keeping — a flag can be wrong and make the story wrong; a lineage cannot. Two semantics are ratified in STORY-BRANCH-SEMANTICS.md, both of them reversals or narrowings that a reader would otherwise take for bugs. Undo now crosses fork points and continues to the campaign opening, because refusing at the fork was a consequence of deleting rows the parent line was also reading, and nothing is deleted any more. And the system refuses to switch which take is live while a later story is off screen, because doing it quietly would leave retained history continuing from words the story no longer says. A new §14A covers editing in place. §14-15 describe the finished behaviour — the edit becomes authoritative, the state it implies is re-evaluated, a new continuation is created, the original is retained — and that requirement is intact and explicitly not weakened here. It is also not built, because re-evaluating state from prose a user typed needs M5's extraction pass. §14A says what exists in the meantime and why refusing is the minimum that holds the invariant rather than the destination. TECHNICAL-DESIGN.md gains §8.7 and §9.1, recording the implemented model and the bundle behaviour as fact in the way §5.2 records M1 and M2. §10.4 gains a constraint that is easy to lose: the snapshot half of the hybrid state model is a requirement, not an optimization. Head movement is a row lookup plus a restore, which is why Undo, Redo and Save Point restore cost the same at any distance into a campaign; a state model recoverable only by replaying from the opening would make all three proportional to campaign length, on exactly the long campaigns this product is for. DATA-MODEL.md records the head as stored on the campaign rather than derived from its newest turn — two campaigns holding identical turns can be read at different places, and nothing about the turns can tell them apart — and the branch disposition as implemented: the depth a divergent write left the branch at, deliberately advisory, and carried through export because every row of an abandoned line is exported either way. BUILD-MILESTONES.md marks M3 complete and states the one condition still open. M4 is told a Save Point is a durable pointer and that restoring one is head movement with a bounds check, not a restore system: a second mover is the specific failure to avoid, because the two paths would silently disagree about what restore means. M5 gets three constraints — keep state efficiently recoverable, move the test instrumentation rather than the assertions when the world-state protocol goes, and finish the narrator edit §14A defers. V1-ACCEPTANCE-TESTS.md clarifies ownership without lowering a bar. D10 keeps all three pass conditions and is explicitly recorded as *not* satisfied at the end of M3; what changed is that the document now says which milestone delivers which condition. D03's result is recorded as a full pass rather than the partial the text allowed for, I07 gains the pre-M3 bundle clause, and L01 gains the note that resolves its apparent conflict with A05 — a failed turn does advance the head by one, onto the player's retained input, and that is A05 working rather than L01 failing. README.md described a different application: a hosted demo, guest accounts, cloud providers, Postgres, a Render blueprint, an analytics dashboard, a QuickJS scripting engine, and 549 tests. M2 removed all of that and the README was never updated — a gap M2's own debt table missed. It now describes what this fork is, including the endpoint policy and the TLS behaviour, and the numbers in it are the current ones. M3's report is included here as its own evidence record: no separate baseline report was produced, so it carries the raw counts and runtime observations as well as the review, and §W records this closeout. SPECIFICATION.md and SECURITY-THREAT-MODEL.md are unchanged. M3 altered no product requirement and touched no path in the threat model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QF5TcoB86QADgjHz1GZe8u
1544 lines
74 KiB
Markdown
1544 lines
74 KiB
Markdown
# 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.
|