diff --git a/README.md b/README.md index 210f393..27996be 100644 --- a/README.md +++ b/README.md @@ -3,25 +3,27 @@ [![CI](https://github.com/parththakkar106/AI-DnD/actions/workflows/ci.yml/badge.svg)](https://github.com/parththakkar106/AI-DnD/actions/workflows/ci.yml) [![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) -An AI Dungeon-style interactive storytelling app that runs entirely on your own machine, with -your own AI model. Create scenarios, play open-ended adventures where an LLM narrates the -world, and extend the engine with **JavaScript scripts compatible with real AI Dungeon -scripting**. +An interactive storytelling app that runs entirely on your own machine, with your own model. +Create scenarios and play open-ended adventures where a local LLM narrates the world, keeps +track of what is true, and remembers what happened. -> ### ▢️ Try it live: **[parththakkar106.github.io/AI-DnD](https://parththakkar106.github.io/AI-DnD/)** -> The project page loads instantly and launches the hosted demo in one tap. Play a scenario as -> a guest: no sign-up and no API key needed. The demo runs on a free tier that sleeps, so the -> first load after it's been idle takes about 30 to 60 seconds to wake up. +This is the **Adventure Storyteller** fork of [AI-DnD](https://github.com/parththakkar106/AI-DnD). +It is deliberately narrower than its upstream: single-user, local-only, and pointed at a model +you run yourself. The hosted deployment, the accounts and sessions, the cloud provider support, +the Postgres path, and the JavaScript scripting engine have all been removed rather than +disabled. What is left is a storyteller you can run offline. + +> **Local-only, by design.** The app talks to one place β€” an Ollama-compatible endpoint on this +> machine or on a machine you control on your own network β€” and it refuses to be pointed at a +> public address. There is no telemetry, no account, no cloud inference, and nothing is fetched +> at runtime from the Internet. > -> For the internals, read the **[design notes](https://parththakkar106.github.io/AI-DnD/guide.html)**. -> They walk through the context budgeting, the world-state referee, and the memory bank, and -> state the reasoning behind each one ([Markdown version](docs/GUIDE.md)). +> For the internals, read the **[design notes](docs/GUIDE.md)**. They walk through the context +> budgeting, the world-state referee, and the memory bank, and state the reasoning behind each +> one. Some sections still describe upstream subsystems this fork has removed. -Built with FastAPI and SQLAlchemy on the backend and React (Vite) on the frontend, running on -SQLite locally and Postgres in the cloud. It works with **any OpenAI-compatible endpoint**: -Ollama and LM Studio locally, or OpenRouter, OpenAI, Groq, or vLLM in the cloud. Endpoint, key, -and model are all runtime settings, and OpenRouter's free-tier models make the whole experience -cost nothing. +Built with FastAPI and SQLAlchemy on the backend and React (Vite) on the frontend, storing +everything in one SQLite file. ![The play screen, with the world-state rail open](docs/images/play-world-state.jpg) @@ -33,7 +35,7 @@ isn't the live one starts a new branch.* ## Features - **The full play loop.** Do / Say / Story / Continue actions, streamed AI responses (SSE), - retry, undo, and edit. Reasoning models are supported: "thinking" streams into a collapsible + retry, undo, redo, and edit. Reasoning models are supported: "thinking" streams into a collapsible πŸ’­ panel with its own token budget. - **A branching story tree.** The story is a tree, not a list. Any turn can hold more than one **take**, and `β€Ή 2/4 β€Ί` steps between them. Stepping is free: the story below simply empties, @@ -55,30 +57,34 @@ isn't the live one starts a new branch.* - **Insights: total prompt transparency.** Every turn stores the exact prompt sent to the model. Open πŸ” on any AI action to see each context component, its token cost, and why it was included. -- **JavaScript scripting, AI Dungeon-compatible.** `onInput` / `onModelContext` / `onOutput` - modifiers share `state` and a `worldEntries` API, and run in an embedded quickjs sandbox - (`backend/app/scripting/`). Real AI Dungeon scripts import and run as is. An in-app CodeMirror - editor is included. - **Auto-summarization and Memory Bank.** The modern AI Dungeon memory system: AI-generated memories every few actions, a running story summary, and embedding-based retrieval that pulls old-but-relevant facts back into context, with similarity scores visible in Insights (`backend/app/memorybank.py`). -- **Undo and retry that actually roll back state.** Undo and retry roll back the world state - and script state to a per-node snapshot, not just the text, and prune the memories that - covered the removed turns. Nothing a retry replaces is discarded: the old attempt stays as - another take of that turn, one keystroke and one click from becoming a branch of its own. -- **Import and export.** AI Dungeon-compatible formats for scripts and scenarios; JSON for - everything else. An adventure exports as `ai-dnd-adventure-v2`, which carries the whole tree: - every branch, every take, and the fork points, since those were chosen rather than computed. - Files saved in the old single-line format still import. -- **Optional accounts for hosted deployments.** By default the app is single-user with zero - auth friction. Set `AIDND_MULTI_USER=1` and visitors play instantly as guests (signed - session cookie), can register (email and password) at any point to keep their data, and each - user gets isolated data plus their own encrypted-at-rest API key. A server-funded **shared - demo key** with a daily turn cap lets people try it without bringing a key - (`backend/app/auth.py`). Each new guest is also given a copy of a short pre-played - adventure, so the first screen shows real turns and their world-state changes without - spending a demo turn (`backend/app/starter.py`). +- **Undo, Redo, and retry that roll back state and delete nothing.** Undo moves where the story + is being read; it removes no accepted turn, so Redo can walk forward into the turns it stepped + over. Both restore the world state from a per-node snapshot rather than just the text, and a + memory derived from a turn now behind the head stops being retrieved without being deleted or + re-embedded. Writing a new turn below a moved-back head is the moment the story forks: the + displaced future stays on the line it was written for, and ordinary Redo stops offering it. + Nothing a retry replaces is discarded either β€” the old attempt stays as another take of that + turn, one keystroke and one click from becoming a branch of its own. +- **Import and export.** AI Dungeon-compatible scenario format; JSON for everything else. An adventure exports as `ai-dnd-adventure-v2`, which carries the whole tree: + every branch, every take, the fork points, which branches the story has left behind, and the + position it is being read at β€” all of them chosen rather than computed, which is the rule for + what a bundle carries. A campaign exported after two Undos imports still undone, with its + retained future intact, instead of silently reopening at its newest turn. Files that predate + the head position, and files saved in the old single-line format, still import. +- **Single user, no accounts.** There is no sign-up, no login, no session and no API key + anywhere in the product. The storyteller API binds to loopback and is unauthenticated by + design, because the only person who can reach it is the person running it. A new install + starts with a short pre-played adventure, so the first screen shows real turns and their + world-state changes rather than an empty page (`backend/app/starter.py`). +- **A refusal you can rely on.** The inference endpoint is checked against an address + allowlist when you save it and again before every request, so a public endpoint is refused + even if the setting is edited in the database directly. TLS verification is never traded + against reachability: a privately issued certificate is verified against your machine's own + trust store, and there is no bypass switch. ## Screenshots @@ -86,10 +92,10 @@ isn't the live one starts a new branch.* |---|---| | ![Insights panel](docs/images/insights.jpg) | ![Scenario editor](docs/images/scenario-editor-npcs.jpg) | | **Insights**: the exact prompt for the next turn, broken into components with token counts and the trigger word that pulled each story card in. | **Authoring**: stats with ranges, per-turn caps, cooldowns, and word-labeled bands; NPCs the AI addresses by id. | -| ![Script editor](docs/images/script-editor.jpg) | ![Home](docs/images/home.jpg) | -| **Scripting**: the three AI Dungeon hooks with shared persistent `state`, run in a quickjs sandbox. | **Home**: continue a story in progress or start from a scenario. | -| ![The branch map](docs/images/branch-map.jpg) | ![The branches panel](docs/images/branches-panel.jpg) | -| **The tree**: one lane per line, from the moment it left its parent to the moment it ends. The horizontal axis is the story's own clock, so a short branch reads as short. | **Branches**: every line the story has taken, and the three things you can do to one. A line the one you're reading was forked from can't be deleted, and says so. | +| ![Home](docs/images/home.jpg) | ![The branch map](docs/images/branch-map.jpg) | +| **Home**: continue a story in progress or start from a scenario. | **The tree**: one lane per line, from the moment it left its parent to the moment it ends. The horizontal axis is the story's own clock, so a short branch reads as short. | +| ![The branches panel](docs/images/branches-panel.jpg) | | +| **Branches**: every line the story has taken, and the three things you can do to one. A line the one you're reading was forked from can't be deleted, and says so. | | ## Quick start @@ -121,58 +127,62 @@ Open http://localhost:5173. ## Connect a model -Open **Settings** in the app and point it at any OpenAI-compatible endpoint: +Open **Settings** in the app and point it at a local Ollama-compatible endpoint: -| Provider | Endpoint URL | Notes | +| Where the model runs | Endpoint URL | Notes | |---|---|---| -| Ollama (local) | `http://localhost:11434/v1` | free, private; also serves embedding models for the Memory Bank (e.g. `nomic-embed-text`) | -| LM Studio (local) | `http://localhost:1234/v1` | free, private | -| OpenRouter | `https://openrouter.ai/api/v1` | `:free` models cost nothing (no embeddings on the free tier) | -| OpenAI / Groq / vLLM / … | provider's `/v1` URL | anything speaking `/v1/chat/completions` | -| Claude Code CLI (local) | `http://127.0.0.1:8787/v1` | your Claude subscription instead of an API key; see [Playing against Claude locally](#playing-against-claude-locally) | +| Ollama, same machine | `http://localhost:11434/v1` | the default, and the simplest thing that works | +| Ollama, a machine on your own network | `http://:11434/v1` or `https:///v1` | see below | +| LM Studio, same machine | `http://localhost:1234/v1` | | -Model name, API key, generation parameters, and (optionally) summary and embedding models for -the Memory Bank are all configured there too. No config files and no rebuild are needed. +Model name, generation parameters, and (optionally) summary and embedding models for the +Memory Bank are configured there too. No config files and no rebuild are needed. There is no +API key field, because there is nothing to authenticate to. -### Playing against Claude locally +### What the endpoint policy allows -`backend/tools/claude_shim.py` serves an OpenAI-compatible endpoint backed by the -`claude` command line tool, so you can play the demos against a real model without an -API key. Each request spawns one `claude --print` process, which suits the turn engine: -the app assembles the whole prompt every turn and expects a stateless endpoint. +The address is checked when you save it and again before every request. Only loopback and +private-network addresses are accepted; every public address is refused, by address rather than +by hostname, so a name that resolves outward is refused too. A well-known cloud inference host +is named in the error message only so the refusal says *why*. + +Running the model on a second machine you control is supported and expected β€” that machine +does the inference while the storyteller itself stays bound to loopback on yours. If that +machine serves HTTPS with a certificate from a CA you installed, it works: certificates are +verified against your operating system's trust store as well as the bundled one. Verification +itself is never relaxed, and there is no option to turn it off. + +### Playing against a local shim + +`backend/tools/claude_shim.py` serves an OpenAI-compatible endpoint on `127.0.0.1:8787` +backed by a command-line tool, which is useful for testing the turn engine against a stronger +model. Each request spawns one process, which suits the engine: the app assembles the whole +prompt every turn and expects a stateless endpoint. ```sh cd backend -.venv/Scripts/python.exe tools/claude_shim.py # listens on 127.0.0.1:8787 +.venv/bin/python tools/claude_shim.py # listens on 127.0.0.1:8787 ``` -In Settings, choose the OpenAI-compatible provider, set the base URL to -`http://127.0.0.1:8787/v1`, put any non-empty string in the API key field, and pick -`sonnet`. The shim ignores the key and authenticates as you, through the CLI. Set the -reasoning budget to `0` or `-1`: a positive budget sends a `reasoning.max_tokens` field -that Claude 5 models reject. +Set the base URL to `http://127.0.0.1:8787/v1` and pick a model the tool offers. Set the +reasoning budget to `0` or `-1`: a positive budget sends a `reasoning.max_tokens` field that +some models reject. Embeddings are not served β€” leave the embedding model blank, or point the +Memory Bank at an endpoint that serves one. -Embeddings are not served. Leave the embedding model blank, or point the Memory Bank at -a real endpoint. - -Run it against a local backend only. The endpoint has no authentication, and anything -reaching it spends your Claude quota. `app/netguard.py` blocks localhost endpoints when -`AIDND_MULTI_USER` is set, so a deployed instance cannot be pointed at it. +The shim has no authentication and spends whatever quota backs it, so run it on loopback and +leave it there. ## How a turn works ``` player input - β†’ onInput script modifier β†’ assemble context: [narrator prompt] + [world state + stat guide] + [AI instructions] + [plot essentials] + [story summary] + [retrieved memories] + [triggered story cards] + [history along this branch, token-budgeted] + [author's note] + [player action] - β†’ onModelContext script modifier β†’ snapshot context (Insights) β†’ provider adapter β†’ AI (streamed) β†’ extract + referee the world-state delta block, strip it from the prose - β†’ onOutput script modifier β†’ store & render ``` @@ -180,18 +190,17 @@ player input ``` frontend/ React + Vite SPA ──HTTP/SSE──► backend/ FastAPI - β”œβ”€ routers/ auth, scenarios, adventures, story cards, scripts, chat, settings, analytics, debug - β”œβ”€ models.py SQLAlchemy: User, Scenario, Adventure, Branch, Action, StoryCard, Script, Settings, Memory - β”œβ”€ migrations.py hand-rolled, versioned via PRAGMA user_version (64 and counting) - β”œβ”€ auth.py guest/registered users, sessions, shared demo key - β”œβ”€ security.py password hashing, cookie signing, API-key encryption + β”œβ”€ routers/ scenarios, adventures, story cards, chat, settings, debug + β”œβ”€ models.py SQLAlchemy: Scenario, Adventure, Branch, Action, StoryCard, Settings, Memory + β”œβ”€ migrations.py hand-rolled, versioned via PRAGMA user_version (79 and counting) + β”œβ”€ endpoints.py the inference-endpoint address policy + β”œβ”€ tlstrust.py one TLS context: the OS trust store unioned with certifi's β”œβ”€ tree.py forking, promotion, and where a node is placed + β”œβ”€ head.py the active head: where the story is read, and what moving it costs β”œβ”€ attempts.py the takes of one turn, grouped by parent β”œβ”€ context/ prompt assembly under a token budget + lineage/history windowing β”œβ”€ worldstate/ the stat engine: clamps, cooldowns, bands, milestones - β”œβ”€ scripting/ quickjs sandbox + AI Dungeon API surface β”œβ”€ memorybank.py auto-summarization + embedding retrieval - β”œβ”€ analytics.py buffered visit counters + the owner's dashboard query β”œβ”€ bundle.py the export/import formats, v2 (tree) and a v1 reader β”œβ”€ providers/ OpenAI-compatible adapter, streaming └─ data.db SQLite (path overridable via AIDND_DB_PATH) @@ -202,9 +211,10 @@ development, Vite proxies `/api` to FastAPI. ## Tests -549 backend tests: unit tests plus full HTTP integration through the real quickjs scripting -engine, with the LLM provider mocked. CI runs them on every push, alongside the frontend -lint/build and a Docker image build. +638 backend tests: unit tests plus full HTTP integration through the real turn engine, with +the model provider mocked. They run with no route to the Internet, which is a requirement +rather than a convenience β€” an offline claim proved on a machine that has been online once +proves nothing. ```sh cd backend && pip install -r requirements.txt -r requirements-dev.txt @@ -233,57 +243,19 @@ most interesting engineering in the repo. the number of SQL clauses is bounded by the context window rather than by the number of forks. -## Visit analytics - -The hosted demo keeps its own analytics: an owner-only dashboard at `/analytics` shows -traffic, which shared scenarios get played, turns and demo-key spend, errors, and a funnel -from *visited* to *played a turn* to *signed up*. It is visible only to the emails listed in -`AIDND_ANALYTICS_EMAILS`, and the route returns 404 for everyone else. - -This is built into the app rather than added with a third-party script, for reasons specific -to this project: the CSP allows only `script-src 'self'`, ad blockers block the popular -trackers, and none of those trackers can see the measurement that matters here, a turn. Counts -are aggregated in memory and flushed as UPSERTs, so a visit is a write and never a read, and -every dashboard query is a `GROUP BY` that returns tens of rows regardless of traffic volume. -That matters: see the egress note above for what reading rows per request costs on this stack. - -## Deploy (Render) - -The repo ships a [`render.yaml`](render.yaml) blueprint: one Docker web service that serves -the SPA and API same-origin, backed by external [Neon](https://neon.tech) Postgres. The free -Render tier has no persistent disk, so the database lives off-box. - -1. Create a **Neon** project and copy its pooled connection string. -2. In Render, choose **New β†’ Blueprint** and point it at this repo. Render reads - `render.yaml`. -3. Fill in the secrets it prompts for (`sync: false` vars): `AIDND_DATABASE_URL` (the Neon - string); `AIDND_DEMO_API_KEY` and `AIDND_DEMO_MODELS` to offer a no-signup demo; and - `AIDND_ANALYTICS_EMAILS` (your own account's email) to see the Visitors dashboard. - `AIDND_SECRET_KEY` is generated automatically and stays stable across deploys. -4. Deploy. Pushes to `main` auto-deploy after this. The health check is `/api/health`. - -On the free tier the service sleeps after about 15 minutes idle, and the first request after -that takes about 30 to 60 seconds to wake it. Point any keep-warm pinger at `/api/health`, -which deliberately doesn't touch the database: waking the database around the clock costs far -more than the cold start saves. - -If you put another proxy or CDN in front of Render, set `AIDND_TRUSTED_PROXY_HOPS` to the -number of proxies in the chain. It defaults to 1. The rate limiter reads the client IP that -many entries from the right of `X-Forwarded-For`, because the trusted edge appends the real -one last. Leave it at 1 behind two proxies and the limiter reads an entry the caller -supplied, so anyone can rotate the header for a fresh rate-limit bucket per request. - ## Repo notes -- `plan/` holds the phased implementation plan this project was built from, kept as a build - log. All fourteen phases are complete. The later files (11, 12, 14) also serve as design - notes for the state-revert, world-state, and story-tree work. - [`plan/STATUS.md`](plan/STATUS.md) is the running thread: what shipped, what was measured, - and what is owed next. -- [`docs/GUIDE.md`](docs/GUIDE.md) holds design notes: how each subsystem works and why it was - built that way, with the measurements behind the decisions. It is also rendered as a - [reading page](https://parththakkar106.github.io/AI-DnD/guide.html). -- `backend/.env.example` lists the few environment variables the backend reads. +- `planning/` is this fork's own package: the product specification, the architecture + decisions, the milestone plan, the acceptance contract, and a review report for every + milestone shipped. Start at [`planning/README.md`](planning/README.md). +- `plan/` holds the *upstream* project's phased implementation plan, kept as a build log. The + later files (11, 12, 14) still serve as design notes for the state-revert, world-state, and + story-tree work this fork inherited. +- [`docs/GUIDE.md`](docs/GUIDE.md) holds upstream's design notes: how each subsystem works and + why it was built that way, with the measurements behind the decisions. Sections covering + scripting, accounts and hosted deployment describe subsystems this fork removed. +- `backend/.env.example` lists the two environment variables the backend reads. Everything + about the model is a runtime setting on the Settings page instead. - [`docs/self-review.md`](docs/self-review.md) records a full-codebase self-review pass and what came out of it. All correctness findings are resolved. diff --git a/planning/BUILD-MILESTONES.md b/planning/BUILD-MILESTONES.md index 55bbced..0ec3b11 100644 --- a/planning/BUILD-MILESTONES.md +++ b/planning/BUILD-MILESTONES.md @@ -247,6 +247,47 @@ Must cover: History operations are non-destructive, Redo works, divergence preserves old futures, and export/import reopens at the exact active head. +## Status: COMPLETE + +Accepted 2026-09-03. Evidence: `planning/reports/M3-IMPLEMENTATION-REPORT.md`, +which is M3's primary evidence record β€” no separate baseline report was produced, +so that document carries the raw counts and runtime observations as well as the +review. The architecture is recorded in **ADR 012**. + +**Capabilities M3 delivered, which later milestones inherit rather than build:** + +- **Undo that deletes zero accepted turns** β€” measured directly on row identity: + 15 rows before five Undos, 15 after, +- **Redo**, round-tripping exactly (head 14 β†’ 4 β†’ 14) in transcript and in state, +- a **single head-movement mechanism** every position change goes through, and a + **single capped lineage** that bounds every read of the story, +- **divergence decided by the lineage** rather than by a flag: the first write + below a moved-back head forks, the displaced future keeps its rows, and + ordinary Redo stops offering it with nothing to invalidate, +- **branch-scoped memory isolation preserved for free** β€” a memory past the head + is unretrievable and becomes eligible again on Redo, with no pruning and no + re-embedding, +- **an export that carries the reader's position**, so a campaign exported after + two Undos imports still undone, with pre-M3 bundles opening at their tip, +- **Undo across fork points** to the campaign opening, and refusal to change what + a turn says while story descends from it off screen β€” both ratified in + `STORY-BRANCH-SEMANTICS.md` (Β§5, Β§10, Β§14A). + +**Outstanding closeout condition:** the required **browser smoke test has not +been performed** β€” no session in which M3 was implemented or reviewed had a +browser available. The equivalent sequence was driven end-to-end against the +running application with real inference and a process restart, and every +server-side behaviour it covers passes; the DOM-level behaviour of the Redo +button, its disabled states and its keyboard shortcut remain unverified by +observation. This does not block M4, which touches none of that wiring, but it +remains an open M3 item until a human runs it. + +**Debt carried forward, none of it blocking M4:** full narrator-edit state +re-evaluation is deferred to M5 (`STORY-BRANCH-SEMANTICS.md` Β§14A records the +interim refusal); `POST /adventures/import` returns every branch's rows rather +than a head-capped window (inherited, harmless in the UI); `ActionPage` is +constructed in two places. Full table in the implementation report Β§S. + --- # M4 β€” Named Save Points / Checkpoints @@ -283,6 +324,32 @@ Add durable user-facing Save Points on top of the active-head model. The user can create a named Save Point, continue, restart, restore it, and continue differently without losing later history. +## Note from M3 β€” reuse the head machinery, do not build a second one + +A Save Point is **a durable named pointer to a recoverable story position**, and +nothing more. M3 made that position a stored coordinate and made moving to one a +row lookup plus a state restore, so restoring a Save Point is head movement with +a bounds check β€” not a restore system of its own. + +Concretely, M4 should: + +- store the coordinate, the name, and the metadata around them, and no copy of + any story; +- restore by calling M3's head-movement mechanism, so that state, transcript, + context and memory eligibility all move together exactly as they do for Undo + and Redo, and so that later history is retained rather than deleted (D13 is + already satisfied by the mechanism); +- let the existing fork-on-first-write-below-the-head rule handle divergence + after a restore, rather than forking at restore time; +- validate that a Save Point's coordinate is still on the lineage being read + before moving to it. + +A second restore path is the specific failure to avoid. The Phase 0B spike put +the fork check in the write path and left Retry and add-take on the old one, and +M3's cost was reconciling them; a parallel checkpoint mover would recreate that +divergence in a place where the two paths would silently disagree about what +"restore" means. See ADR 012. + --- # M5 β€” Genre-Neutral Authoritative Narrative State @@ -369,6 +436,41 @@ dropped. Evidence: `planning/reports/M2-IMPLEMENTATION-REPORT.md` Β§K.2, Β§Q. +## Note from M3 β€” three constraints this milestone must satisfy + +**1. Keep state efficiently recoverable at a retained position.** +M3's head movement is a row lookup plus a state restore, which is why Undo, Redo +and β€” from M4 β€” Save Point restore all cost the same regardless of how far into a +campaign the position is. A state model recoverable only by replaying events from +the campaign opening would make every one of those operations proportional to +campaign length, on exactly the long campaigns this product exists for. + +`TECHNICAL-DESIGN.md` Β§10.4 already selects a hybrid of validated events plus +snapshots/cache. M3 turns the snapshot half from a preference into a requirement: +keep per-node snapshots, or an equivalent cache with the same property, while +adding the typed event model. See ADR 012. + +**2. Move the instrumentation, keep the assertions.** +The M2 note above applies with more force after M3: `test_head_cursor.py` adds +roughly a dozen more tests that express position and rollback through the +inherited gold counter. What they measure is *positional* β€” that the state +belonging to a story position is restored when the head moves to it, in either +direction, and that an abandoned line's state does not survive a divergence. +Those properties must still hold over whatever carries state after M5. The file's +own docstring says so. + +**3. Complete the narrator edit.** +`STORY-BRANCH-SEMANTICS.md` Β§14-15 requires that a narrator edit become +authoritative and that the state it implies be re-evaluated. That requirement is +intact and unimplemented: re-evaluating state from prose a user typed needs this +milestone's extraction pass. M3 shipped the safe interim behavior only β€” an +in-place edit is refused when story descends from the turn and is off screen +(Β§14A), so retained history cannot be made to disagree with itself unseen. +M5 is where Β§14-15 is finished: return to the state before the edited narration, +treat the edited text as accepted output, re-evaluate the implied state, create a +new continuation, and retain the original. The refusal in Β§14A is then replaced +by that behavior rather than kept alongside it. + --- # M6 β€” Branch-Safe Context, Summaries, and Long-Term Story Memory diff --git a/planning/DATA-MODEL.md b/planning/DATA-MODEL.md index 4ad34bf..9fce8b6 100644 --- a/planning/DATA-MODEL.md +++ b/planning/DATA-MODEL.md @@ -74,6 +74,14 @@ story_profile: tense: optional string ``` +The active head is **stored on the campaign, not derived** from its newest turn. +This is the concept M3 implemented; the current implementation carries it as a +branch reference plus a depth on that branch rather than as a turn id, which is +an equivalent coordinate and is what the export format records. What matters +conceptually is that the position is a decision the campaign remembers: two +campaigns holding identical turns can be being read at different places, and +nothing about the turns themselves can tell them apart. + Campaigns also store durable narrator rules and model configuration. Potential model roles: @@ -109,7 +117,26 @@ Rules: - Redo moves the active head forward while the prior continuation remains selected, - a new write below the retained tip creates a new continuation and leaves the old future retained/disposable. -Detailed behavior will be defined separately in `STORY-BRANCH-SEMANTICS.md`. +`disposition` above is conceptual. As implemented in M3 it is stored as **the +fact that produced it** rather than as a word: a branch records the depth a +divergent write left it at, and when. No value means active; a value means the +story past that depth is retained history no active head is reading. The +shallowest departure wins if a branch is left more than once. + +Two properties of that representation are deliberate and worth carrying in this +document: + +- **Nothing reads it to decide behavior.** Whether Redo is available, what the + transcript shows, and which continuation a write belongs to are all decided by + the lineage. A stale or hand-edited disposition therefore cannot make the story + wrong; it can only mislead a cleanup or recovery feature about what is + abandoned. +- **It survives export and import.** Every row of an abandoned line is exported + either way, so the disposition is the only thing distinguishing it from an + active one in a restored campaign. + +Detailed behavior will be defined separately in `STORY-BRANCH-SEMANTICS.md`; +the architecture is recorded in ADR 012. ## 6. Turn @@ -655,6 +682,15 @@ A campaign export should be capable of preserving: The physical container format remains an implementation choice, but the export must preserve the exact active branch **and active head position**, even when the head is behind a retained tip after Undo. A ZIP containing a database plus manifest remains a strong candidate. +As implemented in M3, the export carries the active branch, the active head +position on it, and each branch's disposition, alongside the whole retained turn +graph. The governing rule for this package is that an export carries what was +*chosen* and recomputes what is *derived* β€” and the active head moved from the +second category to the first, because once Undo stops deleting, two campaigns +with identical turns can be being read at different positions and no import can +tell which. An export written before the field existed is opened at its retained +tip, which is the position such a file recorded. + ## 30. Deletion vs Archival The system must distinguish: diff --git a/planning/DECISIONS/012-active-head-non-destructive-history.md b/planning/DECISIONS/012-active-head-non-destructive-history.md new file mode 100644 index 0000000..e933793 --- /dev/null +++ b/planning/DECISIONS/012-active-head-non-destructive-history.md @@ -0,0 +1,116 @@ +# ADR 012 β€” Active-Head Non-Destructive History + +**Status:** Accepted; implemented in M3 +**Date:** 2026-09-03 + +## Decision + +Where the story is being read and how much story is retained are **two separate +facts**, stored separately and answered by different code. + +- The **active head** is a stored position β€” a branch and a depth on it. It is + where the story currently ends as far as the reader, the narrator prompt, and + every feature built on them are concerned. +- The **retained tip** is the deepest node still kept on the same lineage. It may + be ahead of the head. + +Undo and Redo move the active head. They delete nothing, restore nothing from a +log, and recompute nothing. Every ordinary read of the story is bounded by the +head; retained story beyond it stays live in the database, reachable by Redo, +and available to a divergence. + +Concretely, and as implemented: + +1. The active head is persisted on the campaign, not derived from the newest + row. It is a decision, and no read may re-derive it. +2. Lineage resolution caps every entry at the head, in one place, so the + transcript, the assembled context, take/parent resolution and memory + retrieval narrow together and cannot disagree. +3. Reading past the head is possible through one narrow, named exception, and + only two callers may use it: Redo, and the check that decides whether a write + must fork. +4. The state belonging to a position is recorded on the node that produced it, so + moving the head is a row lookup plus a restore β€” the same cost at any + distance, in either direction. +5. Undo alone never forks. The **first write below a moved-back head** is the + divergence: it creates a new continuation, and the displaced future stays + where it was written, on the line it was written on. +6. Whether an ordinary Redo exists is decided by the lineage, not by a flag. After + a divergence the displaced future is no longer on the lineage, so there is + nothing ahead to walk into and no state to invalidate. +7. A branch the story has left records the depth it was left at and when, as + metadata that **nothing reads to decide behavior**. It exists so a divergence + is observable and so later cleanup and recovery features have something to + select on. +8. Derived work β€” memories, summary coverage β€” is anchored to the node it came + from and is therefore filtered by the same capped lineage. Undo prunes + nothing; Redo re-derives nothing. +9. Export carries the active head, because it is a chosen position rather than a + fact about the newest row. Import honors it. A file that predates the field is + opened at its tip, which is the position such a file recorded. + +## Context + +ADR 005 states the **product requirement**: returning to an earlier point +preserves abandoned future history rather than erasing it, and the user sees +Undo/Redo/Retry/Save Point rather than branch management. It names a movable +active head as the implementation direction and stops there. + +This ADR records the **architecture selected to implement it**, as built and +demonstrated in M3. It does not restate or revise ADR 005. + +The production base shipped a destructive Undo: it deleted the trailing turns, +pruned the memories covering them, and let the tip fall back to whatever +survived. That made the head a derived value, made Redo impossible, and β€” as +Phase 0B found β€” let an export silently reopen an undone campaign at its newest +retained turn. + +## Alternatives Considered + +- **Keep the head derived and mark rows inactive.** Rejected: every read would + need its own filter, and the filters would drift. Capping the lineage once is + what makes the whole application agree about where the story ends. +- **Rebuild state by replaying events from the opening.** Rejected: it makes the + cost of Undo proportional to campaign length, and long campaigns are the case + this product exists for. +- **Fork on Undo rather than on the first write below the head.** Rejected: + moving the head is not a decision to abandon anything β€” the user may be + reading, or about to Redo β€” and forking on every Undo fills the branch table + with branches nobody chose. Redo could not survive it. +- **Decide Redo from a stored flag.** Rejected: a flag can be stale or + hand-edited, and a wrong value would produce a wrong story. Deriving it from + the lineage cannot. + +## Reason + +The head is the smallest thing that can move. Making it a stored position rather +than a derived one turns Undo from an operation that destroys accepted story +into one that changes a coordinate, and everything else β€” Redo, divergence +preserving the old future, memory isolation, an export that reopens where the +user left it β€” follows from that single change rather than needing machinery of +its own. + +## Consequences + +- **Undo deletes zero accepted rows.** This is the invariant the architecture + exists to hold, and it is asserted directly on row identity. +- **Retained history accumulates.** v1 requires no automatic cleanup; the + disposition metadata is what a later cleanup or recovery feature will select + on. +- **Any operation that changes what the story says at a position must ask + whether story descends from that position and is off screen.** Switching the + selected take and editing a turn's text in place both must refuse in that + situation rather than act silently. See `STORY-BRANCH-SEMANTICS.md` Β§10 and + Β§14A. +- **Undo crosses fork points**, because a forked story includes the story it was + forked out of and nothing is being deleted. The floor is the campaign opening. +- **Save Points must reuse this mechanism.** A named save point is a durable + coordinate; restoring one is head movement with a bounds check. Introducing a + second restore path would reintroduce exactly the divergence this ADR removes. +- **The narrative-state model must keep state efficiently recoverable at a + position** β€” a per-node snapshot or an equivalent cache β€” or Undo, Redo and + Save Point restore all become proportional to campaign length. This is a + constraint on ADR 010's engine, not a reversal of it. +- **Every feature that reads story must read it through the capped lineage.** + Anything that queries rows directly will see retained history the story is not + telling. diff --git a/planning/README.md b/planning/README.md index f3df5f2..a10ef76 100644 --- a/planning/README.md +++ b/planning/README.md @@ -1,7 +1,7 @@ # Adventure Storyteller Planning Package -**Status:** Phase 0 complete; architecture selected; **Milestones M1 and M2 implemented and accepted (2026-09-02)**. -**Production coding:** Underway, milestone by milestone. M1 and M2 are done; M3 is the next milestone to brief. +**Status:** Phase 0 complete; architecture selected; **Milestones M1, M2 and M3 implemented and accepted (M3: 2026-09-03)**. +**Production coding:** Underway, milestone by milestone. M1, M2 and M3 are done; M4 is the next milestone to brief. This package contains the current product requirements, final Phase 0 architecture decisions, detailed subsystem designs, acceptance tests, research evidence, and the production milestone plan for the local-only interactive-story project. @@ -168,8 +168,13 @@ Milestone M2 COMPLETE (2026-09-02) policy | v -Milestone M3 NEXT β€” brief not yet prepared - non-destructive undo/redo +Milestone M3 COMPLETE (2026-09-03) + non-destructive undo/redo, see planning/reports/M3-*.md and ADR 012 + active-head export one open condition: the browser smoke test + | + v +Milestone M4 NEXT β€” brief not yet prepared + named Save Points | v Implement and review milestone-by-milestone @@ -179,12 +184,43 @@ Implement and review milestone-by-milestone **One milestone at a time. Do not begin a milestone before its brief exists.** -M1 and M2 are complete and accepted; the evidence is in `reports/M1-*.md` and -`reports/M2-*.md`. **No M3 brief has been prepared.** The current action is to -write one, informed by the post-M2 corrections below and by -`reports/M2-IMPLEMENTATION-REPORT.md` Β§Q, which records that M3's chokepoints -were left untouched or simplified by M2 and that the Phase 0B undo/redo spike -still applies. +M1, M2 and M3 are complete and accepted; the evidence is in `reports/M1-*.md`, +`reports/M2-*.md` and `reports/M3-IMPLEMENTATION-REPORT.md` β€” the last of which +is M3's primary evidence record as well as its review, since no separate M3 +baseline report was produced. **No M4 brief has been prepared.** The current +action is to write one, informed by the post-M3 corrections below, by the note +`BUILD-MILESTONES.md` now attaches to M4, and by **ADR 012**, which records the +head-movement mechanism M4 must reuse rather than reimplement. + +One M3 condition remains open and is not a blocker for M4: the required +**browser smoke test has not been performed**, because no session in which M3 +was implemented or reviewed had a browser available. See +`reports/M3-IMPLEMENTATION-REPORT.md` Β§M and Β§W.4. + +### Post-M3 corrections applied (2026-09-03) + +M3's review recommended planning changes and, following the M2 pattern, reported +rather than applied them. All are now applied, together with the closeout work +the milestone itself required: + +| Document | Correction | +| --- | --- | +| `DECISIONS/012-active-head-non-destructive-history.md` | **New ADR.** The architecture selected to implement ADR 005: head stored not derived, one capped read path, one movement mechanism, state from the node, divergence on first write below the head, Redo decided by the lineage, advisory disposition metadata, and the head as an exported decision. | +| `STORY-BRANCH-SEMANTICS.md` Β§5 | Undo crosses fork points and continues to the campaign opening. The old refusal was a consequence of destructive deletion, not a product decision. | +| `STORY-BRANCH-SEMANTICS.md` Β§10 | Switching which take is live is refused while a later story is off screen, with the two resolutions the user has. | +| `STORY-BRANCH-SEMANTICS.md` Β§14A | **New.** In-place editing before Β§14-15 exist: refuse when story descends from the turn and is not on screen. States explicitly that the full narrator-edit requirement stands and is completed in M5. | +| `TECHNICAL-DESIGN.md` Β§8.7, Β§9.1 | **New.** The implemented active-head model and bundle behaviour, recorded as fact. | +| `TECHNICAL-DESIGN.md` Β§10.4 | Constraint from M3: the snapshot half of the hybrid state model is a requirement, or head movement becomes proportional to campaign length. | +| `DATA-MODEL.md` Β§4, Β§5, Β§29 | The head as campaign-stored rather than derived; the branch disposition as implemented and deliberately advisory; the export as carrying a chosen position. | +| `BUILD-MILESTONES.md` M3 | Marked COMPLETE, with inherited capabilities, the open browser condition, and carried debt. | +| `BUILD-MILESTONES.md` M4 | Note: a Save Point is a durable pointer; restore by reusing M3's head movement rather than building a second restore path. | +| `BUILD-MILESTONES.md` M5 | Note: keep state efficiently recoverable at a position; move the test instrumentation rather than the assertions; complete the narrator edit. | +| `V1-ACCEPTANCE-TESTS.md` D03, D10, I07, L01 | D03's result recorded as a full pass; **D10's milestone ownership stated without weakening any pass condition**; I07's pre-M3 bundle clause added; the apparent L01/A05 conflict resolved. | +| `README.md` | Corrected to describe the current local-only single-user application. The scripting, accounts, analytics, hosted-demo, cloud-provider, Postgres and Render material described subsystems M2 removed. | + +`SPECIFICATION.md` and `SECURITY-THREAT-MODEL.md` were deliberately **not** +changed. M3 altered no product requirement and touched no path in the threat +model. ### Post-M2 corrections applied (2026-09-03) diff --git a/planning/STORY-BRANCH-SEMANTICS.md b/planning/STORY-BRANCH-SEMANTICS.md index 255bfe8..54ee137 100644 --- a/planning/STORY-BRANCH-SEMANTICS.md +++ b/planning/STORY-BRANCH-SEMANTICS.md @@ -123,6 +123,29 @@ If the selected base architecture makes unlimited Undo substantially harder or u Phase 0B demonstrated repeated non-destructive Undo well beyond the minimum five-step requirement. The selected head-cursor design should therefore support Undo across retained active-lineage history up to the root unless a later implementation defect forces a documented exception. +### Undo crosses fork points (settled in M3) + +Undo continues backward through story a branch **inherited** from the line it +forked from, up to the campaign opening. It does not stop at the fork. + +This reverses the behavior of the pre-M3 base, and the reversal follows from the +history model rather than from a change of mind about the product. Undo used to +delete the turns it stepped over, and the turns before a fork belong to the +parent line's story as well, so refusing at the fork was the only way to stop +one line's Undo from destroying story another line was still telling. Undo now +moves the reading position and deletes nothing, so there is nothing to protect +the parent from: a forked story includes the story it was forked out of, and +walking back through it is a reader moving backward, not a branch reaching into +another branch's history. + +The only floor is the campaign opening. There is no pre-campaign position to +reach, and Undo at the opening reports that there is nothing to undo. + +One consequence is worth stating plainly for anyone reading a transcript: a +single Undo on a forked story can step back over a turn that was originally +written on the line it forked from. Nothing about that turn changes; it simply +stops being part of what is currently being told. + ## 6. State Restoration on Undo Undo must restore more than visible transcript text. @@ -242,6 +265,28 @@ User action Inactive takes should remain retained initially. +### Selecting a take while a later story is off screen (settled in M3) + +Selecting a different take is a change to what the story says at a position that +already has a story after it. While that later story is on screen, the choice is +plainly visible and the user can see what they are changing. + +It is not, once the later story has been moved out of view β€” undone and not yet +redone, or left behind by a new continuation. Silently switching the take +underneath it would leave retained history continuing from words the story no +longer says, and the user would have no way to see that it had happened. + +The system must therefore refuse to switch the selected take in that situation +and say why, rather than switching it quietly. The user resolves it by deciding +what they mean: + +- **Redo**, bringing the later story back into view, and then choose freely; or +- **play the turn again from here**, which starts a new continuation and keeps + the old one as retained history. + +The same rule and the same two resolutions apply to editing a turn's text in +place; see Β§14A. + ## 11. Retry vs Branch Retry should not be presented to the user as β€œcreating a branch.” @@ -332,6 +377,38 @@ Therefore the system must: The system must not simply replace visible text while leaving stale state behind. +## 14A. Editing In Place, Before Β§14-15 Are Implemented + +Β§14 and Β§15 describe the finished behavior: a narrator edit becomes +authoritative, the state it implies is re-evaluated, a new continuation is +created, and the original narration and its future are retained. That +requirement stands in full and is **not** weakened by this section. + +It is not yet built. Re-evaluating the state implied by prose a user typed +requires the authoritative narrative-state extraction that the genre-neutral +state milestone introduces, so the finished behavior is completed there. What +exists in the meantime is a plain correction: it changes the words of one turn +and re-evaluates nothing. + +That correction is safe while everything descending from the turn is on screen, +because the user can see what their change has to stay consistent with. It is +not safe when a continuation descends from the turn and is **off screen** β€” +undone and not yet redone, or left behind by a divergence β€” because the edit +would then silently change the words that retained story was written from, and +nothing on screen would show it. Retained history is not permitted to be made to +disagree with itself in a way the user cannot see. + +Until Β§14-15 are implemented, the system must therefore **refuse** an in-place +edit of a turn that has story descending from it which is not currently being +shown, and say why. The user resolves it the same two ways as Β§10: + +- **Redo**, bringing the later story back into view; or +- **play the turn again from here**, which is the Β§13 shape β€” return to the + parent position, continue differently, and keep the old line as retained + history. + +Refusing is the minimum that keeps the invariant. It is not the destination. + ## 16. Manual State / Canon Correction The user should be able to correct authoritative story state without rewriting prose. diff --git a/planning/TECHNICAL-DESIGN.md b/planning/TECHNICAL-DESIGN.md index 7ac55d1..ef4c605 100644 --- a/planning/TECHNICAL-DESIGN.md +++ b/planning/TECHNICAL-DESIGN.md @@ -361,6 +361,58 @@ Abandoned history must: - stop influencing current state/context/memory/summary, - remain available for future recovery/cleanup features. +### 8.7 As implemented in M3 + +M3 built this model. The following is fact rather than direction, and ADR 012 +records it as the architectural decision. Sections 8.1-8.6 stand; this says how +they were realised. + +**The head is stored, not derived.** A campaign carries a branch and a depth, +and that pair is the active head. No read may recompute it from the newest row β€” +that was the pre-M3 behavior, and it is what made Redo impossible and made an +export reopen an undone campaign at its tip. + +**Lineage reads are capped at the head, in one place.** The path abstraction that +already resolved a branch's ancestry now also limits every entry to the head, so +the transcript, the assembled narrator context, take/parent resolution and memory +retrieval narrow together. There is exactly one way to read past the head β€” a +named, uncapped view of the same lineage β€” and only two callers may use it: Redo, +and the check that decides whether a write must fork. Any new feature that reads +story rows directly, rather than through the capped lineage, will see retained +history the story is not telling. + +**Head movement is one mechanism.** Undo, Redo, and anything later that restores +a position resolve a target depth and then call a single move operation, which +sets the coordinate and restores the state recorded at it. Undo and Redo differ +only in which way they resolve the target. Both step over a whole turn β€” a +player's action and the reply to it β€” so the head never rests between the two +halves of one turn. + +**State comes from the node, not from a replay.** Each node records the state it +left behind, so moving the head is a row lookup plus a restore: the same cost at +any distance, in either direction, and identical whether the position is reached +from in front of it or from behind. This is the property Β§10.4's hybrid storage +must preserve. + +**Divergence is a property of the lineage, not a flag.** The first write below a +moved-back head forks; Undo alone never does. After the fork, the displaced +future is no longer on the lineage being read, so ordinary Redo finds nothing +ahead and reports that it has nowhere to go. Nothing has to be invalidated, +cleared, or kept in step. + +**A branch the story leaves records the depth and time it was left**, as metadata +nothing reads to decide behavior (Β§8.6's "implementation-appropriate metadata"). +It makes a divergence observable and gives later cleanup and recovery features +something to select on; because no decision depends on it, a stale or hand-edited +value cannot make the story wrong. + +**Operations that change what the story says at a position must ask whether +story descends from that position and is off screen.** Switching which take is +live, and editing a turn's text in place, both refuse in that situation rather +than act silently, because retained history must not be made to disagree with +itself in a way the user cannot see. See `STORY-BRANCH-SEMANTICS.md` Β§10 and +Β§14A. + ## 9. Export / Import and Head Position AI-DnD's current export carries branch information but reconstructs the imported head at the branch tip. @@ -383,6 +435,31 @@ For compatibility with earlier bundles, import may fall back to the retained tip Export/import regression tests must include an undone campaign and verify the imported story reopens at the exact exported head rather than silently redoing later turns. +### 9.1 As implemented in M3 + +The bundle carries the active head depth beside the active branch, and the import +honors it. This moved the head depth across the format's own rule about what a +bundle carries: a bundle carries what was *chosen* and recomputes what is +*derived*, and before M3 the head depth was genuinely derived β€” the newest row was +the only place a story could be read. It is a decision now, because the same tree +exports identically whether the user undid three turns or none, so the file has to +say. + +A file that does not state a head is opened at the tip of its active branch. That +is a fallback only in form: such a file was written when the head could not be +anywhere else, so deriving the tip reproduces the position it actually recorded. +Pre-tree bundles take the same path. No format version bump was required, because +an absent field is unambiguous. + +The head depth is validated before any row is written β€” a depth past the branch's +own retained story is a file disagreeing with itself and is refused, while a depth +*behind* it is the feature. + +The bundle also carries which branches the story has left, and at what depth. +Every row of an abandoned line is exported either way, so without that metadata a +restored campaign could not distinguish abandoned history from active history β€” +which is precisely what a later cleanup or recovery feature has to select on. + ## 10. Authoritative Narrative State ### 10.1 Do not retain the RPG state protocol as the product model @@ -457,6 +534,17 @@ Selected direction: Events provide audit/reconstruction value. Snapshots/cache make normal reads, Undo/Redo, and context construction fast. +**Constraint added by M3 (see ADR 012).** The snapshot half is not an +optimization to be traded away. M3's head movement is a row lookup plus a +restore, which is why Undo, Redo and β€” later β€” Save Point restore cost the same +at any distance into a campaign's history. A state model that could only be +reconstructed by replaying events from the campaign opening would make every one +of those operations proportional to campaign length, on exactly the long +campaigns this product exists for. Whatever M5 introduces must keep the +authoritative state at a retained position efficiently recoverable β€” a per-node +snapshot, or an equivalent cache with the same property β€” while adding the typed +event model. + ## 11. Context and Memory Retain AI-DnD's useful lineage-aware memory foundation, but align it with the product authority model. diff --git a/planning/V1-ACCEPTANCE-TESTS.md b/planning/V1-ACCEPTANCE-TESTS.md index e79c3db..7abc6ae 100644 --- a/planning/V1-ACCEPTANCE-TESTS.md +++ b/planning/V1-ACCEPTANCE-TESTS.md @@ -1,7 +1,8 @@ # Adventure Storyteller β€” V1 Acceptance Tests -**Status:** v1.1 planning/release contract β€” updated after Phase 0B, and after M2 for -the security contract (H10 strengthened, H12 added) +**Status:** v1.2 planning/release contract β€” updated after Phase 0B, after M2 for +the security contract (H10 strengthened, H12 added), and after M3 for history +ownership and results (D03, D10, I07, L01) **Purpose:** Define black-box acceptance tests for finalist evaluation during Phase 0B and for the eventual v1 release. ## 1. Test Philosophy @@ -575,6 +576,17 @@ All retained turns can be traversed backward safely. ### Partial System supports at least five but has a documented technical limit. +### Result (M3) +**Pass, not partial.** Undo traverses to the campaign opening and then reports +that there is nothing to undo. No technical limit applies: each step is one +indexed query regardless of story length, because the position is a stored +coordinate rather than a replay. The floor is the campaign opening β€” there is no +pre-campaign position to reach. + +Note also that Undo continues backward through story a branch inherited from the +line it forked from; it does not stop at a fork. See +`STORY-BRANCH-SEMANTICS.md` Β§5. + --- ## D04 β€” Redo @@ -689,6 +701,26 @@ Mara wears a green cloak. - downstream state is re-evaluated, - old version/future remains retained/disposable. +### Milestone ownership + +All three pass conditions stand for v1. They are delivered across three +milestones, and this note records which is which rather than reducing the +requirement: + +- **M3 β€” safe history behavior.** Replaying a narrator turn with different text + forks, keeps the original take and its future, and starts the new continuation + from the correct earlier state. In-place editing of a turn is **refused** while + story descends from it off screen, so retained history cannot be made to + contradict itself unseen (`STORY-BRANCH-SEMANTICS.md` Β§14A). Delivered. +- **M5 β€” authoritative state re-evaluation.** The second pass condition. Making a + hand-typed narrator correction authoritative and re-evaluating the state it + implies requires the narrative-state extraction pass, so it is completed there, + and the Β§14A refusal is replaced by it. Outstanding. +- **Later browser UX work β€” the finished editing workflow.** How the user reaches + and confirms the operation. Outstanding. + +D10 is therefore **not** satisfied at the end of M3, and is not scored as such. + --- ## D11 β€” Named Checkpoint @@ -1358,6 +1390,21 @@ No external API credentials are embedded in campaign export. - import does not silently Redo to the newest retained turn, - Redo/recovery behavior remains coherent after import. +### Also required β€” an export written before the head was carried + +Import an export produced by a build that recorded no active head, and confirm it +opens at the retained tip of its active branch. + +This is compatibility, not a degraded path, and the distinction matters when +reading a result: such a file was written when the head could not be anywhere but +the tip, so opening it there reproduces the position it actually recorded. An +import that refused it, or that guessed some other position for it, would be the +failure. + +An export whose stated head lies beyond the story it contains is a file +disagreeing with itself and must be refused rather than opened at a guessed +position. + --- # J. Genre Independence @@ -1490,6 +1537,20 @@ No condition exists where: - branch head advances incorrectly, - previous story becomes inaccessible. +### Note on the head, and on A05 + +"The head advances incorrectly" must be read together with A05, or the two appear +to contradict each other. A failed turn **does** move the active head forward by +one, onto the player's submitted text, because A05 deliberately retains that text +so the player can try again. That is correct behavior, not a half-advanced head. + +What this test forbids is the head moving past a turn that did not happen: an +accepted narration with state written only partway, or a position that implies a +reply the story never received. Assert on the accepted narration and the +authoritative state, not on whether the head moved at all. One Undo from that +position steps back over the stranded input and leaves the story on a complete +turn. + --- ## L02 β€” State Reconstruction diff --git a/planning/VERSION.md b/planning/VERSION.md index 28e4538..9742865 100644 --- a/planning/VERSION.md +++ b/planning/VERSION.md @@ -1,8 +1,49 @@ # Planning Package Version -**Package:** Adventure Storyteller Planning Package v2.2 +**Package:** Adventure Storyteller Planning Package v2.3 **Revision date:** 2026-09-03 -**Status:** Phase 0 complete; architecture selected; **Milestones M1 and M2 implemented and accepted**; M3 not yet briefed. +**Status:** Phase 0 complete; architecture selected; **Milestones M1, M2 and M3 implemented and accepted**; M4 is next to brief. + +## v2.3 β€” Post-M3 Closeout (2026-09-03) + +M3 replaced destructive Undo with a stored active head. Its review is +`reports/M3-IMPLEMENTATION-REPORT.md`, which is also M3's primary evidence +record β€” no separate baseline report was produced β€” and whose Β§W records this +closeout. + +In summary: + +- the architecture is recorded as **ADR 012 β€” Active-Head Non-Destructive + History**: the head is stored rather than derived, every read of the story is + capped at it in one place, one mechanism moves it, state comes from the node + rather than from a replay, the first write below a moved-back head is the + divergence, and Redo is decided by the lineage rather than by a flag. ADR 005 + is unchanged: it states the product requirement, and ADR 012 states the + architecture chosen to implement it, +- two history semantics are **ratified** in `STORY-BRANCH-SEMANTICS.md`: Undo + crosses fork points to the campaign opening (Β§5), and the system refuses to + switch which take is live while a later story is off screen (Β§10), +- a **new Β§14A** records the interim in-place-editing rule β€” refuse when story + descends from the turn and is not on screen β€” and states explicitly that + Β§14-15's full narrator-edit requirement stands and is completed in M5, +- `TECHNICAL-DESIGN.md` gains **Β§8.7** and **Β§9.1** recording the implemented + model and bundle behaviour as fact, and a constraint on Β§10.4: the snapshot + half of the hybrid state model is a requirement, because head movement must + not become proportional to campaign length, +- `DATA-MODEL.md` records the head as campaign-stored, the branch disposition as + implemented and deliberately advisory, and the export as carrying a chosen + position rather than a derived one, +- `BUILD-MILESTONES.md` marks **M3 complete**, states the one outstanding + condition (the browser smoke test), tells **M4** to reuse M3's head movement + rather than build a second restore path, and gives **M5** three constraints, +- `V1-ACCEPTANCE-TESTS.md` records D03's full pass, states **D10's milestone + ownership without weakening any pass condition**, adds I07's pre-M3 bundle + clause, and resolves the apparent L01/A05 conflict, +- `README.md` is corrected to describe the current local-only single-user + application rather than the upstream hosted one. + +`SPECIFICATION.md` and `SECURITY-THREAT-MODEL.md` are unchanged: M3 altered no +product requirement and touched no path in the threat model. ## v2.2 β€” Post-M2 Closeout (2026-09-03) diff --git a/planning/reports/M3-IMPLEMENTATION-REPORT.md b/planning/reports/M3-IMPLEMENTATION-REPORT.md new file mode 100644 index 0000000..d85ca61 --- /dev/null +++ b/planning/reports/M3-IMPLEMENTATION-REPORT.md @@ -0,0 +1,1543 @@ +# 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.