Files
interactive-story/plan/17-refactor.md
parththakkar106andClaude Opus 5 f1bebe18d0 Drop the eight legacy columns SP8 left behind
Migrations 66 to 73 drop `actions.index`, `variants`, `variant_index`,
`variant_count`, `state_before`, and `world_state_before`, plus
`adventures.memory_cursor` and `summary_cursor`. `index` is a keyword in
SQLite, so migration 71 quotes it.

Nothing outside the migrations read these. `models.py`, `schemas.py`, and
`ACTION_LIST_COLUMNS` lose the same eight fields, `Adventure.actions` orders
by `id`, and `attempts.renumber`, `context.history.max_action_index`, and
`nodes.next_index` are deleted.

Two changes keep the migration replayable on a `create_all` database:

- `_split_variants_into_siblings` wrote through the live ORM table, so it
  stopped compiling once migration 66 removed five of its columns. It now
  writes through `_ACTIONS_AT_60`, a frozen `Table` with its own `MetaData`.
- Five data passes read columns these migrations drop. Each now calls
  `_has_columns` and returns early when the columns are absent.

`bootstrap` takes a `through` version so a migration test can stop at the
schema it asserts on.

555 tests pass, up from 549. Eight of the new cases assert each column is
gone after a real schema-45 database migrates all the way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0198qDK3gmgSo7EtQ4GTPqqK
2026-08-29 17:15:49 +05:30

31 KiB

Phase 17: refactor for readability

The code works and it is tested. It is also hard to read, because four files hold most of it, one schema migration is half finished, and the published guide is a hand-maintained copy of another file. This phase fixes those three things without changing what the app does.

How to use this file

This file is both the plan and the running status. Update the progress table at the end of every stage. Read the table first when you pick the work back up.

Each stage lands as its own commit on refactor-17. No stage starts until the stage before it is green.

Progress

Stage Work Status Landed
0 Hygiene: worktrees, branches, undocumented settings done, except the branch deletion 2026-08-29
1 Split the four largest files done: one test setup, and all four files split 2026-08-29
2 Remove duplication done: all seven items 2026-08-29
3 SP8: drop the legacy columns code done, all eight columns gone; the deploy and VACUUM FULL remain 2026-08-29
4 Documentation not started
5 A frontend test runner not started

Baseline

Record these numbers before you start. They are how you tell a refactor from a rewrite.

  • 549 backend tests pass in 103 seconds.
  • backend/app holds 13,486 lines across 45 files.
  • backend/tests holds 12,221 lines across 38 files.
  • frontend/src holds 9,171 lines across 21 files.
  • The app exposes 70 endpoints and applies 64 migrations on boot.

Run the suite with cd backend && .venv/Scripts/python.exe -m pytest tests/ -q.

What the review found

The code is clean at the statement level

A scan for unreferenced functions across backend/app returned only FastAPI route handlers, which the decorator references rather than the name. A scan of index.css found 5 unused class names out of 373, and every one of the 5 is either CodeMirror's or built from a template string. There is no dead code to delete and no TODO or FIXME anywhere in the tree.

Read that as a constraint. The gains in this phase come from moving code, not from finding rot.

Four files hold most of the complexity

File Lines What it holds
backend/app/routers/adventures.py 2353 Nine concerns: the turn engine, branches, takes, import and export, adventure scripts, refresh from scenario, insights, memory CRUD, and action CRUD
frontend/src/pages/Play.jsx 2280 27 components, plus a 722-line Play() holding 18 useState calls and 7 useEffect calls
frontend/src/index.css 2865 One stylesheet. The max-width: 720px block at line 2655 overrides rules written 2000 lines above it
backend/app/worldstate/engine.py 918 Four jobs: parse a delta, apply a delta, render the context sections, and instantiate a schema

The schema is half migrated

plan/14-phase-story-tree.md defines SP8, which drops the columns the tree replaced. SP8 was gated on the tree running in production. It now does. Eight columns are still written on every turn and read by nothing:

  • actions.index, variants, variant_index, variant_count
  • actions.state_before, world_state_before
  • adventures.memory_cursor, summary_cursor

Until they go, models.py carries two vocabularies for one idea, and every reader has to be told which one is live.

The published guide is a hand-written copy

docs/guide.html is 84 KB of hand-written HTML covering the same material as the 63 KB docs/GUIDE.md. Nothing generates one from the other. They have already drifted: the HTML last changed on 2026-08-20 and the Markdown on 2026-08-28, and section 3.6, "Counting visits", exists only in the Markdown. GitHub Pages publishes the HTML, so readers get the stale copy. docs/architecture.html is 87 KB of hand-written HTML with no Markdown source at all.

Duplication the tests already protect

  • worldstate.apply_delta and apply_override each re-implement the same path routing for flags., milestones., and npc.<id>.. That is six parallel branches at engine.py:573, 587, 617, 675, 697, and 740.
  • 35 of the 38 test files repeat the same 8-line temporary-database prologue. There is no conftest.py.
  • ScriptedProvider or FakeProvider is defined 12 times, once per test file that needs a fake model.
  • The debounced autosave helper is copied into Play.jsx:204, ScenarioEditor.jsx:23, and ScriptEditor.jsx:22.
  • get_adventure_or_404 is called by hand in about 20 handlers. It is a function, not a dependency, so every handler repeats the call.
  • routers/chat.py:22 imports SSE_HEADERS and sse from routers/adventures.py. One router reaches into another for shared plumbing.

One setting is documented nowhere

AIDND_TRUSTED_PROXY_HOPS is read at limits.py:55. It does not appear in backend/.env.example, README.md, or render.yaml. It sets how many proxy hops client_ip() trusts, which is what stops a rotated X-Forwarded-For header from buying a fresh rate-limit bucket. If a deployment adds a proxy hop and nobody sets this variable, the bypass comes back silently.

The unprefixed DATABASE_URL that database.py:24 also accepts turned out to be documented already, inside the AIDND_DATABASE_URL entry in backend/.env.example. No change needed there.

Settings.stream is dead

models.py:622 defines the column, and schemas.py:452 and 499 expose it. Nothing reads it in the backend or the frontend. This is item S1 in docs/self-review.md.

Local clutter

Everything here is gitignored, so it costs repository weight nothing. It costs disk and attention.

  • Two abandoned worktrees, each a full copy of the tree: .claude/worktrees/simplify-comments and .claude/worktrees/sp7-tree-ui. Both branches landed on main as squashes.
  • 21 local branches. 13 read as unmerged to git branch --no-merged, and plan/STATUS.md already records that they landed as squashes.
  • Three stale SQLite files in backend/: data.backup-2026-07-06.db, data.backup-pre-phase8.db, and scroll_fixture.db, 2.9 MB together. The live local database, backend/data.db, is not one of them and is not touched.

Stage 0: hygiene

No source changes. Do this first, so later stages run against a quiet tree.

  1. Remove both worktrees with git worktree remove, then delete the branches that landed as squashes. Verify each one first: a branch is safe to delete when git log main..<branch> names only commits whose message already appears in git log main.
  2. Delete the three stale .db files. Leave backend/data.db alone.
  3. Add AIDND_TRUSTED_PROXY_HOPS to backend/.env.example, with the reason it exists. Add it to the deploy section of README.md too, because that is where somebody putting the app behind another proxy looks.

Check: the suite still passes, and git status is clean.

What Stage 0 actually did, 2026-08-29

Both worktrees are gone, which freed about 104 MB. Removing sp7-tree-ui needed one extra step: a Vite dev server had been running out of that worktree since 2026-08-18, holding frontend/.vite open and owning port 5173. It was serving a tree 54 commits behind main. Stopping it freed the directory. Restart the real one with start.ps1.

The three stale .db files are deleted. backend/data.db is untouched.

AIDND_TRUSTED_PROXY_HOPS is now in backend/.env.example and in the README deploy section.

Still owed. The 19 squash-landed branches are still there. git branch -D is blocked by the permission classifier, which is a reasonable guard on a destructive command. All 19 are verified safe by the rule above. Run this to clear them:

git branch -D bugfix-code-review docs-story-tree fix-prepend-autoscroll \
  fix-silent-clamps-and-milestone-ids fix-store-refusals-and-delta-wording \
  handover-2026-08-17 measure-post-vacuum-sizes phase-14-story-tree \
  phase-7-public-repo phase-8-accounts sp1-tree-schema sp10-memory-bank-eviction \
  sp2-branch-clause sp3-node-cursors sp4-sibling-nodes sp5-fork-on-continue \
  sp6-bundle-v2 sp7-tree-ui sp7b-take-pager worktree-keep-fixture-and-status \
  worktree-simplify-comments

worktree-keep-fixture-and-status is the one that needed checking by hand. Its commit subject appears nowhere in main, but the --keep flag it adds is on main at tools/stress_session.py:764, along with both gotchas the message describes, at :686 and :692. Only the message differs.

The remote branches are left alone. Deleting those is a separate decision.

Stage 1: split the four largest files

Every change in this stage moves code. None of it changes behavior. The 549 tests are the check, and they must pass without being edited, except where this section says otherwise.

routers/adventures.py becomes a package

Split it into backend/app/routers/adventures/:

Module Holds Source lines
__init__.py The APIRouter, the shared dependencies, and re-exports
paging.py ACTION_LIST_COLUMNS, ACTION_PAGE, action_window, annotate_takes, current_window 27-146, 1203-1261
crud.py List, create, get, patch, delete, plus the script-state and world-state readers 225-580
turns.py The turn engine: generate_turn, _generate_turn, run_player_turn, the turn lock, and the SSE helpers 581-1004
takes.py Retry, variants, takes, forking, and undo 1005-1192, 1503-1778
branches.py The branch endpoints 1193-1502
bundle_io.py Export and import 1779-1837
scripts.py The per-adventure script endpoints 1838-1941
refresh.py Refresh from scenario 1942-2152
insights.py The context dry run and the per-action context 2153-2189
memories.py Memory bank CRUD 2190-2279

Target no file above 450 lines.

The tests couple to the module, so read this before you start. Twelve test files call monkeypatch.setattr(adventures, "OpenAICompatibleProvider", ...). monkeypatch replaces a name in the module where the calling code looks it up, so re-exporting from __init__.py does not keep those patches working. Once _generate_turn lives in turns.py, the target becomes adventures.turns.OpenAICompatibleProvider.

Do Stage 2's conftest.py work first if you want that to be a one-line change instead of twelve. Otherwise retarget all twelve here. The other patched names are adventures.limits, adventures.check_demo_cap, adventures.generate_turn, and adventures._active_turns, each used once.

Keep these importable from the package root, because tests import them by name: ACTION_PAGE, SNIPPET_MAX, _snippet, world_delta_of, acquire_turn_lock, retry_action, undo_turn, and _active_turns.

_active_turns is module-level mutable state guarded by a lock. It must live in exactly one module, turns.py, and every other module must import the module and reach through it. If two modules import the set by value, the lock guards two different sets and the turn lock stops working.

What the router split actually did, 2026-08-29

backend/app/routers/adventures.py is now a package of 14 modules. The largest is turns.py at 443 lines. Four modules exist that the table above does not list, because the plan's eleven still mixed unrelated work:

Extra module Why it exists
deps.py Holds the APIRouter and the ownership check. It imports nothing else in the package, so every endpoint module can import the router without importing its siblings.
scenario_text.py Copying a scenario's text and cards has two callers, crud.create_adventure and refresh. Leaving it in either one made the other import an endpoint module.
nodes.py Story-tree navigation that four modules use: last_action, next_index, next_depth, stand_on, db_tip, delete_turn.
actions.py The three action endpoints. They page and delete rather than play a turn, so they do not belong in crud.py.

Two decisions differ from the plan above.

The package root does not re-export acquire_turn_lock or _active_turns. The plan said to keep them importable, but that makes a broken patch look like a working one. Rebinding adventures.generate_turn changes the alias and leaves every caller reading the original, and the test still passes. Leaving those names off the package root raises AttributeError instead. Eighteen test call sites and four in backend/tools/ now say adventures.turns.<name>. The package root still re-exports the pure helpers, so adventures.ACTION_PAGE, adventures.undo_turn, and chat.py's from .adventures import SSE_HEADERS, sse are unchanged.

world_delta_of lives in turns.py. It reads as a world-state helper and sat beside the world-state endpoints, but _generate_turn is its only caller.

_active_turns behaved as the plan warned. Every module reaches it as turns._active_turns, and a check confirms the four modules see one set object and one lock.

One test coupled to the router for an unrelated module: it called adventures.worldstate.instantiate. It imports app.worldstate directly now.

The split moved text rather than retyping it. An AST comparison against the pre-split file confirms all 86 definitions are identical, once the turns. prefix is normalized away. The 549 tests pass, and the OpenAPI schema still lists the same 35 operations.

worldstate/engine.py becomes a package

Split backend/app/worldstate/engine.py into four modules under backend/app/worldstate/:

  • schema.py: has_schema, instantiate, reconcile, band_label, npc_name, npc_triggers, _initials.
  • parse.py: extract_delta, _tolerant_load, render_delta_block, applied_delta, refusals, render_refusals.
  • apply.py: apply_delta, apply_override, and the shared path resolver Stage 2 introduces.
  • render.py: render_state_section, render_reference, _stat_line, _describe_stat.

worldstate/__init__.py re-exports every public name it exports today, so no call site changes. The engine is imported as a module, not monkeypatched, so this split carries none of the router's test coupling.

pages/Play.jsx becomes a directory

Split it into frontend/src/pages/Play/:

  • index.jsx: the page component.
  • usePlaySession.js: the 18 useState calls and 7 useEffect calls that drive one adventure, behind one hook.
  • panels/: PlotPanel, MemoryPanel, ScriptsPanel, InsightsPanel, BranchPanel.
  • drawers/: StatusDrawer, WorldStateDrawer, and the StatRow, StatGroup, StateTree, and StateValue parts they use.
  • reports/: StateChangeChips, WorldStateReport, ScriptReport, CacheReport, TokenBreakdown.
  • TakePager.jsx, RefreshModal.jsx.

There is no frontend test runner until Stage 5, so this split is verified by npm run lint, npm run build, and by driving the Play screen in a browser. Drive it. plan/STATUS.md records that the last two Play bugs were both found by hand and were unreachable from any test that existed.

index.css becomes a directory

Split it into frontend/src/styles/, imported in order from index.css: tokens.css, base.css, nav.css, cards.css, play.css, drawers.css, schema-editor.css, insights.css, modals.css, and theme.css.

Move each @media block next to the rules it overrides, rather than leaving one max-width: 720px block at the end. Keep the source order identical when you move rules, because CSS resolves ties by order and the file relies on that in at least two known places, recorded in plan/STATUS.md and in the comments at index.css:1011 and 1082.

Check: 549 tests pass, npm run lint and npm run build are clean, and the Play screen works in a browser at desktop and at 500 px wide.

What Stage 1 actually did to the frontend, 2026-08-29

Play.jsx is now frontend/src/pages/Play/, twelve files. The largest is index.jsx at 781 lines. index.css is now an @import list over frontend/src/styles/, eighteen files.

Two decisions differ from the plan above.

usePlaySession.js does not exist yet. The page component still owns all of the session state. Moving eighteen useState calls and seven useEffect calls is a rewrite, not a move, and no frontend test would catch a mistake in it today. It waits for Stage 5.

The @media blocks stayed where they were. responsive.css still holds one max-width: 720px block, at the end of the import order. Moving a @media block next to the rules it overrides moves it earlier in the cascade, which changes which of two equal-specificity rules wins. Nothing in the test suite would catch that. Do this after Stage 5.

Each split is verified by a different proof, because neither one has a test:

  • CSS: the parts rebuild index.css byte for byte, and the built bundle is identical before and after at 56686 bytes.
  • JSX: every non-blank line of the original appears exactly once, in order, across the twelve files. A name-resolution check confirms every identifier each file references is defined or imported there, with no unused imports.

The line split stranded a comment at six of the boundaries. A leading comment sits above the section it describes, so each boundary cut one loose and left it at the end of the file before it. All six moved to the section they describe.

npm run lint and npm run build are clean, and 549 tests pass. Driving the Play screen covered the story view, all five panels, both drawers including the world-state edit form, the branch map, the refresh dialog, and the take pager, which stepped onto a take that lives on another branch and switched to it. The console reported no errors. The extension cannot resize the render viewport and the app sends X-Frame-Options: DENY, so the narrow-width check ran by setting the max-width media queries to all in the live stylesheet. All 71 narrow rules found their elements: the nav collapses to one button, the panel tabs move onto the title row, a panel fills the screen, the composer stacks, and both drawers become edge tabs.

Stage 2: remove duplication

Each item here is small and is covered by tests that already exist.

  1. One path resolver in worldstate. apply_delta and apply_override route flags.<name>, milestones.<id>, npc.<id>.<stat>, world.<stat>, and player.<stat> with parallel code. Extract a resolver that returns the container, the stat definition, and the kind, then let the two functions differ only in the write rule. Their rules genuinely differ, so do not merge the functions themselves. apply_override ignores cooldown, max_delta_per_turn, and the rule that a counter cannot decrease, and it lets a milestone be un-set. tests/test_worldstate.py covers both.
  2. Add backend/tests/conftest.py. Move the temporary-database prologue there, so the other 35 files drop 8 lines each. The prologue has to run before from app.main import app, because main.py calls bootstrap(engine) at import time. A conftest.py runs before any test module, which satisfies that.
  3. One fake provider. Move ScriptedProvider and FakeProvider to conftest.py as one fixture, replacing 12 copies.
  4. Move the SSE helpers out of the router. Put sse, SSE_HEADERS, and turn_error in backend/app/sse.py, so chat.py stops importing from adventures.
  5. Make get_adventure_or_404 a dependency. About 20 handlers repeat the call. A Depends removes the line from each one and puts the ownership check in the signature, where a reader looking for it expects it.
  6. One useDebouncedSave hook in frontend/src/hooks/, replacing the three copies.
  7. Delete Settings.stream. Remove the column, the two schema fields, and add the migration that drops it. This is item S1 in docs/self-review.md. Mark it applied there.

Check: 549 tests pass. Test count may drop if consolidating fixtures removes a duplicate case. If it does, say which case and why in the commit message.

What Stage 2 actually did, 2026-08-29

All seven items landed. Items 2 and 3 went in early, with the test setup in 32cd7c1, because the conftest had to exist before the router split could move any test. Items 1, 4, and 5 are 2c57b1c. Items 6 and 7 are e0bf2b6.

Item 1, the path resolver. _resolve returns a _Target naming the kind, the section, the key, the stat definition, and the character, or a rejection. The two callers now differ only in the write rule, which is what the item asked for. Building the container is a method on _Target rather than part of resolving, because a rejected path must not leave an empty section behind.

A refactor here is hard to check by reading, so it was checked by running. A differential harness fed 3960 generated payloads through the old and the new implementation and compared the state and the report. Ignoring fix, there are zero differences. 674 rejections gained a fix string and none lost one, all of them in apply_override, which had been the terser of the two. That is an improvement rather than a regression: apply_delta already worded those strings, and the world-state editor renders them.

Item 5, the ownership dependency. All 32 handlers converted, not the 20 the review estimated. Two proofs, because tests alone would not catch a change in the order of the checks or in the public HTTP surface:

  • An AST pass confirmed the get_adventure_or_404 call was the first statement in every one of the 32 handlers. If it were not, hoisting it into a dependency would move work that used to run after something else.
  • The generated OpenAPI document was diffed against one built from a git clone at HEAD. The only difference is that rename_branch now lists branch_id before adventure_id, which is parameter order in the spec and not a route change.

Seven tests in test_state_revert.py needed updating. They call handlers as plain functions rather than over HTTP, so they have to pass adventure= now. That file is the only one that does this.

Item 7 needed a migration guard. Migration 65 drops settings.stream, and it is the first migration that drops a column. _column_already_there already existed for the ADD COLUMN case. Dropping needs the mirror, _column_already_gone, because create_all builds the current schema, which is already missing the column, and fixtures like test_tree_migration.pre_tree stamp an old version against a database built that way and replay. Verified by running migration 65 twice, once against a database that still had the column and once against one that did not.

One duplicate was left alone. FakeProvider in test_chat.py is not a copy of ScriptedProvider. It records the key, model, and endpoint it was constructed with, which is how the chat tests assert on what would have gone over the wire. fakes.ScriptedProvider streams replies and records prompts. Merging them would give one class two unrelated jobs.

Stage 3: SP8, drop the legacy columns

This is the only stage that touches the production database. Follow plan/14-phase-story-tree.md, which specifies it.

  1. Confirm nothing reads the eight columns. ACTION_LIST_COLUMNS in the adventures package lists index, variant_count, and variant_index today, so that tuple changes here. bundle.py:37 and context/history.py:424 both describe index as unread; verify that rather than trusting the comment.
  2. Add the migration that drops them. Remove the columns from models.py, and remove the fields from schemas.py and from ActionOut.
  3. Deploy, then run VACUUM FULL actions; on the direct Neon endpoint, not the -pooler one. Dropping a column rewrites toasted values, and nothing reclaims that space on its own. plan/STATUS.md records what happened the last time nobody ran it: the database reached 144.2 MB against a 512 MB tier.
  4. VACUUM FULL takes an ACCESS EXCLUSIVE lock, so the app blocks on actions for the duration. It took 5.5 seconds at 144 MB.

Check: 549 tests pass, /api/health answers {"ok":true} after the deploy, and one existing adventure opens, takes a turn, retries it, and pages back through the transcript.

What Stage 3 actually did, 2026-08-29

Steps 1 and 2 landed. Step 3, the deploy and the VACUUM FULL, is still open, because it runs against production and belongs with the release, not with the branch.

Step 1, the read audit. Nothing outside the migrations reads the eight columns. TakePager.jsx reads action.take_count and action.take_index only, which is what makes dropping the three payload fields safe. The comments in bundle.py and context/history.py were accurate.

Step 2, the drop. Migrations 66 to 73 drop one column each. index is a keyword in SQLite, so migration 71 quotes it. models.py, schemas.py, and ACTION_LIST_COLUMNS lost the same eight, Adventure.actions now orders by id, and attempts.renumber, context/history.max_action_index, and nodes.next_index are deleted.

Two problems came out of the migration passes rather than the DDL:

  • _split_variants_into_siblings wrote through Base.metadata.tables["actions"], the live ORM table, so it stopped compiling the moment migration 66 removed five of its columns. It now writes through _ACTIONS_AT_60, a frozen Table carrying its own MetaData. That declaration is a snapshot of a past schema and must not be updated to track models.py.
  • Five passes read columns that migrations 66 to 73 drop. A create_all database replays every migration against the current schema, so each pass now calls _has_columns and returns early when the columns are absent. This is the rule _column_already_there applies to DDL, applied to the passes.

bootstrap gained a through argument. A migration test that asserts on something a later migration removes stops at the version it is about, rather than reading a schema several versions newer.

One pre-existing bug found and not fixed. bundle._write_nodes never sets Action.parent_id, and paging.annotate_takes groups on parent_id, so an imported adventure's take pager reads 1 of 1. The frontend has read only take_count since SP9, so this predates Stage 3 and is not a regression from it. test_retry_variants.py documents it.

Check: 555 backend tests pass, up from 549. test_tree_migration.py gained eight parametrized cases asserting each column is gone after a real schema-45 database migrates all the way. A create_all database would pass those without running the migration, which is why the fixture is a frozen pre-tree one.

Stage 4: documentation

  1. Generate docs/guide.html from docs/GUIDE.md. Write a small build script that renders the Markdown into the existing hand-written HTML shell, keeping the current styles and metadata. After that, one edit updates both. Note in README.md that the HTML is generated and that you edit the Markdown.
  2. Decide what docs/architecture.html is. It has no Markdown source. Either give it one and generate it the same way, or state at the top of the file that it is hand-written, so the next person does not look for a source that does not exist.
  3. Split plan/STATUS.md. It is 63 KB, and most of it is dated session logs. Keep "Pick up here", "Things worth remembering", and "Running things" in STATUS.md. Move the dated entries to plan/history/, newest file first.
  4. Add docs/DEVELOPING.md. Record how to run the app, how to run the tests, where each subsystem lives, and the invariants a newcomer breaks first:
    • Anything that reads adventure.actions during generation takes exclude_action_id. It leaks into four places, not one.
    • memory_cursor and summary_cursor are positions into story_actions(), and Memory.source_start and source_end are Action.index values. The two spaces diverge as soon as anything is deleted.
    • /auth/me must not raise. It is the SPA's bootstrap call, so anything it touches that can raise takes the whole frontend down.
    • The global field rule in the stylesheet keys off input[type=...], so a bare <input> and any new input type get browser default styling until you add them.
    • A modal opened from .side-panel needs createPortal, because the panel's filling transform animation makes it the containing block for position: fixed.

Check: the generated HTML matches the Markdown section for section, including section 3.6. Every link in README.md and docs/index.html resolves.

Stage 5: a frontend test runner

plan/STATUS.md names the missing runner as the reason this project keeps finding UI bugs by hand. Two shipped bugs were unreachable from any test that existed.

  1. Add Vitest, React Testing Library, and jsdom. Add a test script to frontend/package.json.
  2. Add the step to the existing frontend job in .github/workflows/ci.yml, between lint and build.
  3. Write the first tests against the two bug classes that already recurred:
    • The take pager renders when a retry's reply arrives over SSE. The stream builds its own ActionOut, so it was the one payload that never carried the annotation.
    • Retaking a player turn does not produce > You > You. The editor is seeded with stored text that is already formatted.
  4. Add a test for usePlaySession from Stage 1, since that hook now holds the state the page used to hold inline.

Check: npm test passes locally and in CI.

Risks

Risk Where How you catch it
A monkeypatch silently stops patching, and a test passes while calling a real provider Stage 1, routers/adventures split After retargeting, break the fake on purpose and confirm the tests that use it fail
_active_turns ends up imported by value into two modules, so the turn lock guards two sets Stage 1 tests/test_state_revert.py reaches for adventures._active_turns. Keep that import path working, and confirm a second concurrent turn still returns 409
A CSS rule changes meaning because the split reorders it Stage 1, stylesheet split Compare the built CSS before and after. Order within each section must not change
The column drop rewrites the table and nobody reclaims the space Stage 3 Run VACUUM FULL actions; on the direct endpoint, and read sizes from sum(octet_length(col)) rather than n_live_tup
Splitting Play.jsx breaks something no test covers Stage 1 Drive the Play screen by hand at both widths. Stage 5 exists to shrink this risk for next time

What this phase does not do

  • It does not replace the hand-rolled migration runner with Alembic. 64 migrations run correctly, and docs/GUIDE.md section 2.6 already explains the choice.
  • It does not move bootstrap(engine) out of import time in main.py. The current behavior means a failed migration means no service, which is deliberate. Stage 2's conftest.py removes the boilerplate that import-time bootstrapping forces on tests, which is the part that actually hurts.
  • It does not change any API shape, any database content, or any prompt.