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
`backend/app/routers/adventures.py` held 2353 lines and 35 endpoints. It is now
a package of 14 modules, the largest 443 lines.
The split moves text rather than rewriting it. An AST comparison against the
old file confirms all 86 definitions are identical, and the OpenAPI schema
still lists the same 35 operations.
Names a test replaces now live in `turns.py` only, and other modules reach them
as `turns.<name>`. Rebinding a re-exported alias changes the alias and leaves
every caller reading the original, so the package root does not re-export them.
A patch aimed at the old target raises `AttributeError` instead of passing while
doing nothing. Tests and the fixtures in `backend/tools/` say
`adventures.turns.<name>`.
The same rule keeps the turn lock working. One module owns `_active_turns`, so
one lock guards one set.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Dix4oGV3njgWRdu7P9t6r
Every test module carried the same eight-line prologue redirecting the
database to a temp file. Only the first one to be imported ever took
effect: `app.database` reads `AIDND_DB_PATH` at import and builds `engine`
from it once, so by the time the second module ran the engine already
existed. The other 34 copies created a temp file that nothing opened and
nothing deleted, and leaked one per module per run.
`conftest.py` now does it once, which is early enough because pytest
imports conftest before any test module. It also deletes the file when the
run ends. The tests still share one database, exactly as they already did:
each `client` fixture calls `create_all` on setup and `drop_all` on
teardown, so no test sees another test's rows.
`tests/fakes.py` holds the one `ScriptedProvider`. Nine modules each had a
copy, and the copies had drifted into four feature sets, so a test that
needed to raise a provider error had to be written in one of the files
whose copy supported that. The shared one is the superset. The two
`FakeProvider` copies were the same class with a fixed reply, so they use
it too. `test_chat.py` keeps its own, which implements `chat` rather than
`generate` and records what it was constructed with.
An autouse fixture resets the fake's class state between tests, so a stale
reply list can no longer reach the next test.
435 lines out of the suite. 549 tests pass. Verified live by sabotage:
breaking the shared fake fails 13 tests across four modules.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Dix4oGV3njgWRdu7P9t6r
* Rewrite comments in Google developer documentation style
Rewrite the comments and docstrings across the backend core modules so they
read plainly. The previous prose was accurate but dense and figurative, which
made it slow to skim.
Applies the Google developer documentation style guide: short sentences, active
voice, present tense, American spelling, and no metaphors, idioms, or
rhetorical asides. Replaces em-dash chains with separate sentences.
Prompt caching bills on a shared prefix: the endpoint reuses the request up
to the first byte that differs from last time and no further. The live
world-state block sat third from the top of the system message, so every turn
re-priced the instructions, the plot essentials and the whole story history
underneath it. The retrieved memories and the rewritten summary did it again.
Everything fixed is emitted first now, and everything that moves goes after
the history, ordered least-volatile first — which is also where recency serves
it best, the reasoning that already put the emit reminder last. The three tail
sections that are last for their own reasons stay last. The moved sections are
still charged to the token budget; only their position changed.
Two smaller halves of the same problem. OpenRouter serves a model from
whichever upstream is free and each upstream holds its own cache, so a
deepseek model now names deepseek as its preferred upstream — a preference,
not a restriction, so a turn still runs if that upstream is down. And the
endpoint's usage block is read back off the response and kept per attempt, so
the hit rate shows up in Insights and the debug log instead of being assumed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DfMCsN1KBLsTqMkj5hSgrY
Nine findings from a review of the phase-14 stack. The one about a retry
withdrawing a memory is not a bug — a memory anchored to a node describes that
node, and it goes when the node goes. The root is the exception, and it is the
only one: migration 62 parked every memory written before memories had
coordinates on depth 0, so withdrawing the opening node would retire a whole
bank nobody attached there. A memory with no source range covers no story and
now stays; a summary that genuinely ends there is still withdrawn.
The rest are repairs.
* The adventure list quoted whichever attempt was written last rather than the
one the story tells, so switching back left the index disagreeing with the
page.
* A v1 import gave a typed memory no depth, rebuilding the NULL the migration
exists to remove — invisible until the imported adventure forked.
* The action cap counted a v1 file's turns, and a turn expands into a row per
saved attempt, so a file inside the cap could write a multiple of it.
* Forking a live node on a borrowed ancestor promoted a sibling on a branch the
caller never named. It is a branch switch, and now says so.
* Switching attempts left the state, status and memory panels reading the
previous take: the story does not change length, so nothing keyed on its
length noticed. Same class as the branch-switch bug this phase already fixed.
* A retry after switching back numbered the new attempt into the middle of the
group instead of the end.
* The cursor backfill numbered every action in the table once per adventure;
correlated to the adventure being updated, it is an index lookup instead.
* Renaming a branch answered own_actions=0.
And one behaviour change recorded rather than repaired: script-visible history
and actionCount no longer count blank-text rows. That is the right shape and
there is no reading compatible with both, so plan/14 says so.
409 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g
A hand-written memory used to carry a NULL depth, described in the model as
"belongs to the adventure rather than to a path". That sounds harmless and
is not: a NULL is a coordinate no fork can cap, so a note typed on one line
followed the reader onto branches whose story it never described. It takes
the head now — the story you were reading when you wrote it — and obeys
exactly the rule a summarised memory obeys.
The unanchored escape clause in lineage.Path.clause existed for that single
case and is deleted rather than left unused. Its docstring argued that a
capped depth would drop a typed memory the moment its branch stopped being
the newest entry; anchoring answers the same worry better, because the
memory is not exempt from the path, it is on one.
The drawer now shows the path being read and nothing else, filtered by the
clause retrieval itself uses, so the bank you can see is the bank the model
can see. Nothing is stranded: a memory lives on a branch, switching to that
branch shows it, and deleting the branch deletes it. Pinning decides order,
the path decides existence.
Migration 62 lands existing NULL-depth memories at depth 0 of their branch
rather than at the tip. 0 is at or before every fork point, so every memory
stays visible from exactly the paths it is visible from today — nobody's
bank loses a row on deploy. The tip is the tidier-sounding choice and would
have emptied them out of every branch forked earlier than they were typed.
This supersedes the on_path flag and the "another branch" badge from
earlier today; anchoring makes them redundant, and they are removed.
Four tests changed because they asserted the old contract, not because
they broke. The one worth reading is the pair replacing
test_a_hand_written_memory_is_not_lost_at_the_first_fork: typed on shared
trunk it still survives a fork, and typed on ground the fork never
travelled it no longer follows you.
402 tests. Verified on tools/branch_fixture.py: each branch's drawer holds
its own memory and not the other's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g
Retrieval has been path-scoped since SP3: a memory on a branch this story
never travelled is never sent to the AI. The drawer listed every memory
alike, so on a fork you read "Fell down the cellar stairs; badly hurt" and
reasonably concluded the model knew it. It does not. That is worse than
either hiding the row or retrieving it — it is the screen claiming
something the engine contradicts.
Hiding them is not the answer either; that was the original comment's
point, and it stands. A memory nobody can list is a memory nobody can
delete, in a phase whose rule is that nothing is removed automatically.
So the whole bank still lists, and the rows off the current path are set
back, dashed, and labelled "another branch". MemoryOut.on_path carries it,
computed from the predicate retrieval itself uses rather than a second
spelling of the same idea — two spellings drift, and the failure mode here
is a badge that says the opposite of what the model gets. Pinning does not
override it: the path clause runs before pinning is considered.
The relationship is asymmetric and there is now a test that says so. A fork
borrows its ancestors, so a memory written on the parent is on the fork's
path too; the reverse never is. Worth pinning before somebody makes it
symmetric on the grounds that it looks wrong.
399 tests, three new, egress ceilings intact — the flag costs one id-only
query. Verified in a browser on tools/branch_fixture.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g
SP7 needs three branch operations and SP5 built one. Switching exists;
naming and deleting had no column and no route between them.
A name is stored because a player chose it. An unnamed branch keeps NULL
rather than a generated "branch 4" — a generated label is derived, and it
would go stale the moment a branch before it is deleted and the ordinals
shift underneath. The client draws those from the fork depth, which
nothing can shift. The v2 bundle carries the name for the same reason it
carries the fork points and leaves `lineage` out: it is a decision, not
something computed from one.
Delete is what stands between a tree and unbounded growth, since nothing
prunes one on its own. It refuses two branches: the root, which holds the
turns every other branch borrows, and the one being read — including any
branch the head was forked from, which is the same mistake in disguise and
the one that would cascade the head away and leave head_branch_id pointing
at nothing. The nodes, memories and descendants go through the foreign
keys that already cascade.
A cursor standing on a deleted branch is cleared. On Postgres a stale
branch id would simply never resolve; SQLite hands the freed id to the
next fork, and then the anchor resolves onto a branch it has never seen
and calls a stretch of story already summarized.
396 tests, 15 new.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g
