diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index b955a4b..91e0a9b 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -26,7 +26,7 @@ from collections import OrderedDict from sqlalchemy import func, select, update from sqlalchemy.orm import Session, object_session -from . import models, vectors +from . import models, tree, vectors from .context import history, story_actions, truncate_to_last_tokens from .database import SessionLocal from .providers import OpenAICompatibleProvider, ProviderError @@ -427,14 +427,16 @@ async def _create_due_memories( return # logged in the debug page; cursor unchanged → retried next turn if not text: return - db.add( - models.Memory( - adventure_id=adventure.id, - text=text, - source_start=block[0].index, - source_end=block[-1].index, - ) + memory = models.Memory( + adventure_id=adventure.id, + text=text, + source_start=block[0].index, + source_end=block[-1].index, ) + # Phase 14: hang it off the node it summarised, so a fork inherits the + # memories of the path it forked from and nothing else. + tree.place_memory(db, adventure, memory) + db.add(memory) adventure.memory_cursor = cursor + MEMORY_INTERVAL db.commit() diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 2e19be1..787c321 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -18,6 +18,7 @@ migrations added from Phase 9 on must run on both dialects. """ import json +import re from sqlalchemy import inspect, text from sqlalchemy.engine import Engine @@ -178,6 +179,41 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ "default": "ALTER TABLE actions ADD COLUMN context_snapshot_z BYTEA"}), (44, "ALTER TABLE actions DROP COLUMN context_snapshot"), (45, "ALTER TABLE actions RENAME COLUMN context_snapshot_z TO context_snapshot"), + # Phase 14, SP1: the story becomes a tree. Every action gains the branch it + # was played on and its depth along that branch; adventures gain a head + # pointer; memories attach to the node that produced them. The `branches` + # table itself comes from create_all, like `memories` did. + # + # Nothing reads these yet — SP2 moves the reads onto them. This subphase + # exists so that by the time anything does, every row already has them, + # including the rows written between the two deploys (`app/tree.py` stamps + # those). Legacy `index`, `variants`, `variant_index` and `variant_count` + # stay in place, unread, until the tree is proven live. + # + # **This rewrites every row of `actions`, twice** — once per ADD COLUMN + # backfill pass on Postgres — so the deploy that ships it must be followed + # by, once: + # + # VACUUM FULL actions; + # + # on the direct endpoint, not -pooler. That is the 144 MB lesson from + # 2026-08-17: a rewrite roughly doubles the table and only a VACUUM FULL + # hands the space back. Skipping it is safe and simply leaves the table fat. + (46, "ALTER TABLE actions ADD COLUMN branch_id INTEGER " + "REFERENCES branches(id) ON DELETE CASCADE"), + (47, "ALTER TABLE actions ADD COLUMN depth INTEGER"), + # head_branch_id carries no REFERENCES: branches.adventure_id already points + # the other way, and two constraints would make the pair a cycle create_all + # cannot order. See the column comment in models.py. + (48, "ALTER TABLE adventures ADD COLUMN head_branch_id INTEGER"), + (49, "ALTER TABLE adventures ADD COLUMN head_depth INTEGER NOT NULL DEFAULT -1"), + (50, "ALTER TABLE memories ADD COLUMN branch_id INTEGER " + "REFERENCES branches(id) ON DELETE CASCADE"), + (51, "ALTER TABLE memories ADD COLUMN depth INTEGER"), + # The index every branch clause wants, and the data pass that fills the six + # columns above (_backfill_tree, hung off this version because it needs all + # of them to exist). + (52, "CREATE INDEX IF NOT EXISTS ix_actions_branch_depth ON actions (branch_id, depth)"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) @@ -187,6 +223,11 @@ WORLD_DELTA_VERSION = 36 VARIANT_COUNT_VERSION = 37 EMBEDDING_BLOB_VERSION = 38 SNAPSHOT_COMPRESS_VERSION = 43 +TREE_BACKFILL_VERSION = 52 + +# An adventure with no actions has no tip. -1 keeps "the next node goes at +# head_depth + 1" true without a special case (mirrors tree.NO_DEPTH). +NO_DEPTH = -1 # Snapshots converted per round trip. Deliberately far smaller than # BACKFILL_BATCH: a vector is 6 KB and a snapshot is 232 KB, so 200 of these @@ -255,6 +296,32 @@ def _for_dialect(sql: str | dict[str, str], dialect: str) -> str: return sql if isinstance(sql, str) else sql.get(dialect, sql["default"]) +# Matches the ADD COLUMN migrations in this file — all hand-written above, so +# this parses SQL we control and nothing else. +_ADD_COLUMN = re.compile(r"^\s*ALTER\s+TABLE\s+(\w+)\s+ADD\s+COLUMN\s+\"?(\w+)\"?", re.I) + + +def _column_already_there(conn, sql: str) -> bool: + """True when `sql` adds a column the table already has. + + This is the `IF NOT EXISTS` the docstring asks for, spelled in Python + because SQLite has no syntax for it on ADD COLUMN. Without it, any database + carrying a *newer* column than its stamp claims dies on a duplicate column + with the backfill never running — and that database is not hypothetical: + `create_all` always builds the current schema, so it is what every test + replaying a migration starts from, and SQLite cannot drop the columns back + off again once a foreign key names them. + """ + match = _ADD_COLUMN.match(sql) + if match is None: + return False + table, column = match.group(1), match.group(2) + inspector = inspect(conn) + if table not in inspector.get_table_names(): + return False + return column in {col["name"] for col in inspector.get_columns(table)} + + def _backfill_embedding_blob(conn) -> None: """Repack memories.embedding (JSON list) into memories.embedding_blob. @@ -345,6 +412,90 @@ def _backfill_context_snapshot(conn) -> None: last_id = rows[-1][0] +# "The root branch of the adventure this row belongs to." MIN(id) rather than a +# LIMIT so it is a plain scalar subquery on both dialects, and deterministic if a +# database ever ends up with two roots for one adventure. +def _root_branch_of(column: str) -> str: + return ( + "(SELECT MIN(b.id) FROM branches b " + f"WHERE b.adventure_id = {column} AND b.parent_branch_id IS NULL)" + ) + + +def _backfill_tree(conn) -> None: + """Re-read every existing adventure's linear story as a tree with one branch. + + One root branch per adventure, `depth` = the old `index`, the head pointing + at the tip, and every memory hung off the node it summarised. Nothing is + copied, nothing is deleted, and no ordering changes — `index` and `depth` + hold the same numbers when this finishes, which is what makes "current + adventures are unaffected" a testable claim rather than a hope. + + Server-side: `actions` is the table that fills the disk, and pulling it into + Python to write two integers a row would be the same mistake this project + has now made twice. Each statement is guarded on its own target being unset, + so a run that dies halfway resumes rather than double-applying. + """ + sqlite = conn.dialect.name == "sqlite" + + # 1. A root branch per adventure. Its lineage names the row's own id, which + # does not exist until the row does, so it starts as the empty list — + # `branches` comes from create_all, where lineage is NOT NULL, so an + # empty array is what "not filled in yet" has to look like. + conn.execute(text(""" + INSERT INTO branches (adventure_id, parent_branch_id, fork_depth, lineage, created_at) + SELECT a.id, NULL, NULL, '[]', CURRENT_TIMESTAMP + FROM adventures a + WHERE NOT EXISTS (SELECT 1 FROM branches b WHERE b.adventure_id = a.id) + """)) + + # 2. lineage = [[own_id, null]] — one entry, uncapped: the root branch is + # the whole story. Built by the database's own JSON functions because + # binding a JSON string as a parameter has no spelling that means the + # same thing to SQLite (TEXT) and to Postgres (json). The guard is a + # length, not `= '[]'`: Postgres `json` has no equality operator. + conn.execute(text( + "UPDATE branches SET lineage = json_array(json_array(id, null)) " + "WHERE json_array_length(lineage) = 0" + if sqlite else + "UPDATE branches SET lineage = " + "jsonb_build_array(jsonb_build_array(id, null))::json " + "WHERE json_array_length(lineage) = 0" + )) + + # 3. Every action onto that branch, at the depth its index already implies. + # Deleting a middle action left gaps in `index`, and those gaps carry + # over deliberately: depth has to keep the order the story is read in, + # and renumbering here would move every cursor that points past the gap. + conn.execute(text(f""" + UPDATE actions + SET branch_id = {_root_branch_of('actions.adventure_id')}, + depth = "index" + WHERE branch_id IS NULL + """)) + + # 4. The head: the root branch, and the depth of its newest node. + conn.execute(text(f""" + UPDATE adventures + SET head_branch_id = {_root_branch_of('adventures.id')}, + head_depth = COALESCE( + (SELECT MAX(a."index") FROM actions a WHERE a.adventure_id = adventures.id), + {NO_DEPTH} + ) + WHERE head_branch_id IS NULL + """)) + + # 5. Memories onto the node that produced them: `source_end` is the index of + # the last action a memory summarised, so it is that node's depth. A + # hand-written memory has no node and keeps depth NULL. + conn.execute(text(f""" + UPDATE memories + SET branch_id = {_root_branch_of('memories.adventure_id')}, + depth = source_end + WHERE branch_id IS NULL + """)) + + def _get_version(conn) -> int: if conn.dialect.name == "sqlite": return conn.execute(text("PRAGMA user_version")).scalar() or 1 @@ -382,7 +533,11 @@ def bootstrap(engine: Engine) -> None: current = _get_version(conn) for version, sql in MIGRATIONS: if version > current: - conn.execute(text(_for_dialect(sql, conn.dialect.name))) + statement = _for_dialect(sql, conn.dialect.name) + # The DDL is skippable when it has already happened; the data + # pass below it is not, and still runs. + if not _column_already_there(conn, statement): + conn.execute(text(statement)) if version == WORLD_DELTA_VERSION: _backfill_world_delta(conn) if version == VARIANT_COUNT_VERSION: @@ -394,6 +549,8 @@ def bootstrap(engine: Engine) -> None: # DROP rolls back with it and the prompts are still there. if version == SNAPSHOT_COMPRESS_VERSION: _backfill_context_snapshot(conn) + if version == TREE_BACKFILL_VERSION: + _backfill_tree(conn) current = version _set_version(conn, current) _encrypt_plaintext_api_keys(conn) diff --git a/backend/app/models.py b/backend/app/models.py index 9938980..5c5df1d 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -1,8 +1,8 @@ from datetime import datetime, timezone from sqlalchemy import ( - JSON, Boolean, Column, DateTime, Float, ForeignKey, Integer, LargeBinary, String, - Table, Text, + JSON, Boolean, Column, DateTime, Float, ForeignKey, Index, Integer, LargeBinary, + String, Table, Text, ) from sqlalchemy.orm import Mapped, mapped_column, relationship @@ -116,6 +116,17 @@ class Adventure(Base): # How many actions have already been folded into memories / the story summary. memory_cursor: Mapped[int] = mapped_column(Integer, default=0) summary_cursor: Mapped[int] = mapped_column(Integer, default=0) + # Phase 14: where the story is being played — which branch, and the depth of + # its newest node. Deliberately NOT a ForeignKey: branches.adventure_id + # already points this way, and a second constraint back would make the two + # tables a cycle that create_all cannot order (the fix for that is + # use_alter, which SQLite has no ALTER for). It is a cache of a pointer, and + # `tree.head_branch` treats a head naming a branch that no longer exists as + # a bug to recover from rather than a state to honour. + head_branch_id: Mapped[int | None] = mapped_column(Integer, nullable=True) + # The depth of the tip, so the next node is always head_depth + 1. + # NO_DEPTH (-1) for an adventure with no actions yet. + head_depth: Mapped[int] = mapped_column(Integer, default=-1) created_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow) updated_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow, onupdate=utcnow) @@ -140,6 +151,48 @@ class Adventure(Base): ) +class Branch(Base): + """Phase 14 — one path through an adventure's story tree. + + A branch does not own a copy of the story: it holds the nodes played on it + and *borrows* everything before its fork point from its ancestors. Reading + branch C means reading C's nodes, plus B's up to where C left it, plus A's + up to where B left it — which is what `lineage` spells out, so a read is an + OR-clause per entry instead of a walk up parent pointers. + + Until forking ships there is exactly one root branch per adventure and + every node hangs off it. That is not a half-migrated state: a linear story + *is* a tree with one branch, which is why writing these columns changes + nothing anyone can observe. + + No ORM relationships on purpose. `actions.branch_id` and `memories + .branch_id` carry ON DELETE CASCADE, so the database removes a deleted + branch's nodes; a relationship would have SQLAlchemy load them all to do + the same thing, and loading every action of a branch is the exact cost the + windowed reads exist to avoid. + """ + + __tablename__ = "branches" + + id: Mapped[int] = mapped_column(primary_key=True) + adventure_id: Mapped[int] = mapped_column(ForeignKey("adventures.id", ondelete="CASCADE")) + # NULL on a root branch. + parent_branch_id: Mapped[int | None] = mapped_column( + ForeignKey("branches.id", ondelete="CASCADE"), nullable=True + ) + # The depth this branch left its parent at, stored when the fork happens and + # never inferred afterwards. Inferring it from where two branches' nodes + # first differ would be a guess about how the story was played — and a wrong + # one as soon as an attempt happens to repeat its parent's text. + fork_depth: Mapped[int | None] = mapped_column(Integer, nullable=True) + # The ancestry, newest first: [[branch_id, max_depth], ...] where max_depth + # is NULL for "to the tip" and otherwise the fork_depth of the branch + # beneath it, inclusive. Computed once at fork from the parent's lineage + # plus one entry, so no read ever reconstructs it. + lineage: Mapped[list] = mapped_column(JSON, default=list) + created_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow) + + class Memory(Base): """Phase 6: an auto-summarized (or hand-written) fact about the adventure. @@ -169,6 +222,18 @@ class Memory(Base): # Action index range this memory summarizes (null for manual memories). source_start: Mapped[int | None] = mapped_column(Integer, nullable=True) source_end: Mapped[int | None] = mapped_column(Integer, nullable=True) + # Phase 14: the node that produced this memory — the last action it + # summarises. Anything derived attaches to the node it came from, which is + # what makes a fork free: a shared ancestor's memories are shared + # automatically, and a memory covering a stretch of branch B is invisible + # from any path that does not go through B. + # + # `depth` is NULL for a hand-written memory, which no node produced; that + # reads as "belongs to the adventure, not to a path". + branch_id: Mapped[int | None] = mapped_column( + ForeignKey("branches.id", ondelete="CASCADE"), nullable=True + ) + depth: Mapped[int | None] = mapped_column(Integer, nullable=True) # Whether embedding_blob is set. Maintained on write by memorybank # .set_vector, for the same reason actions.variant_count exists beside # actions.variants: every reader wants the one-bit answer and none of them @@ -212,10 +277,27 @@ class StoryCard(Base): class Action(Base): __tablename__ = "actions" + # Phase 14: every read of a story is "this branch up to this depth, or that + # branch up to that depth, ...", so (branch_id, depth) is the shape every + # one of those clauses wants an index on. + __table_args__ = (Index("ix_actions_branch_depth", "branch_id", "depth"),) id: Mapped[int] = mapped_column(primary_key=True) adventure_id: Mapped[int] = mapped_column(ForeignKey("adventures.id", ondelete="CASCADE")) index: Mapped[int] = mapped_column(Integer) + # Phase 14: the node's place in the tree. `depth` is a position along *a* + # path, not a global turn number — A4 and B4 are alternatives, not + # duplicates — and it replaces `index` as the ordering key. + # + # Nullable because ALTER TABLE cannot add a NOT NULL column with no + # default and there is no sensible default for "which branch": the + # migration fills them, `tree.place_action` fills them for new nodes, and + # from SP2 on a NULL branch_id is a row no read can see. Legacy `index` + # stays beside them, unread, until the tree is proven live (SP8 drops it). + branch_id: Mapped[int | None] = mapped_column( + ForeignKey("branches.id", ondelete="CASCADE"), nullable=True + ) + depth: Mapped[int | None] = mapped_column(Integer, nullable=True) type: Mapped[str] = mapped_column(String(20)) # start|do|say|story|continue|ai text: Mapped[str] = mapped_column(Text, default="") # Reasoning-model "thinking" that preceded the text (AI actions only). diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index 336d9cf..30a7274 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -9,7 +9,7 @@ from sqlalchemy import func from sqlalchemy.orm import Session, load_only, undefer from sqlalchemy.orm.attributes import set_committed_value -from .. import auth, images, limits, memorybank, models, schemas, worldstate +from .. import auth, images, limits, memorybank, models, schemas, tree, worldstate from ..context import build_context from ..context import history as context_history from ..database import get_db @@ -337,6 +337,10 @@ def create_adventure( ) db.add(adventure) db.flush() + # Every adventure has a story tree from the moment it exists, even before + # anything is played onto it — an adventure with a NULL head is a state the + # tree would otherwise have to tolerate everywhere for no gain. + tree.head_branch(db, adventure) if scenario: for ref, spec in scenario_card_specs(scenario, values).items(): @@ -356,14 +360,14 @@ def create_adventure( ) ) if scenario.prompt.strip(): - db.add( - models.Action( - adventure_id=adventure.id, - index=0, - type="start", - text=fill_placeholders(scenario.prompt, values), - ) + opening = models.Action( + adventure_id=adventure.id, + index=0, + type="start", + text=fill_placeholders(scenario.prompt, values), ) + tree.place_action(db, adventure, opening) + db.add(opening) db.commit() db.refresh(adventure) @@ -823,6 +827,7 @@ async def _generate_turn( state_before=state_before, world_state_before=world_state_before, ) + tree.place_action(db, adventure, ai_action) db.add(ai_action) adventure.updated_at = models.utcnow() if cfg.using_demo: @@ -877,6 +882,7 @@ async def run_player_turn( state_before=state_before, world_state_before=world_state_before, ) + tree.place_action(db, adventure, player_action) db.add(player_action) db.commit() db.refresh(player_action) @@ -1079,6 +1085,8 @@ def undo_turn( db.flush() # apply deletes so pruning sees the shrunken action list db.expire(adventure, ["actions"]) memorybank.prune_dangling_memories(adventure, db) + # The tip moved back with them. + tree.refresh_head(db, adventure) db.commit() db.refresh(adventure) # The newest window, not the whole story: the client replaces its @@ -1202,10 +1210,13 @@ def import_adventure( ) db.add(adventure) db.flush() + # A v1 bundle is a linear story, which is a tree with one branch. SP6's v2 + # format carries the branches themselves. + tree.head_branch(db, adventure) for m in bundle.get("memories") or []: if isinstance(m, dict) and str(m.get("text") or "").strip(): - db.add(models.Memory( + memory = models.Memory( adventure_id=adventure.id, text=str(m["text"]), pinned=bool(m.get("pinned", False)), @@ -1213,7 +1224,9 @@ def import_adventure( source_start=m.get("sourceStart"), source_end=m.get("sourceEnd"), use_count=int(m.get("useCount", 0)), - )) + ) + tree.place_memory(db, adventure, memory) + db.add(memory) for card in bundle.get("storyCards") or []: if isinstance(card, dict): @@ -1248,7 +1261,7 @@ def import_adventure( for v in (a.get("variants") or []) if isinstance(v, dict) ] - db.add(models.Action( + action = models.Action( adventure_id=adventure.id, index=int(a.get("index", i)), type=str(a.get("type") or "story")[:20], # VARCHAR(20) @@ -1259,7 +1272,9 @@ def import_adventure( # Clamped: a bundle could name an index its variant list # doesn't have, which would make the pager point at nothing. variant_index=min(max(int(a.get("variantIndex", 0)), 0), max(len(variants) - 1, 0)), - )) + ) + tree.place_action(db, adventure, action) + db.add(action) db.commit() db.refresh(adventure) @@ -1629,6 +1644,8 @@ def create_memory( if not payload.text.strip(): raise HTTPException(400, "Memory text cannot be empty") memory = models.Memory(adventure_id=adventure.id, text=payload.text.strip()) + # No node produced this one, so it gets a branch but no depth. + tree.place_memory(db, adventure, memory) db.add(memory) db.commit() db.refresh(memory) @@ -1742,4 +1759,7 @@ def delete_action( db.flush() # apply the delete so pruning sees the shrunken action list db.expire(adventure, ["actions"]) memorybank.prune_dangling_memories(adventure, db) + # Deleting the newest action moves the tip; deleting a middle one leaves a + # gap in the depths, deliberately — see _backfill_tree. + tree.refresh_head(db, adventure) db.commit() diff --git a/backend/app/tree.py b/backend/app/tree.py new file mode 100644 index 0000000..db45c94 --- /dev/null +++ b/backend/app/tree.py @@ -0,0 +1,120 @@ +"""Phase 14 — putting nodes on the story tree. + +The write half of the tree. Which branch a new node hangs off, what depth it +gets, and where an adventure's head points all live here, because every one of +them is the kind of thing that is silently wrong when it is spread across four +call sites: a node written without a branch is a node no read can see, and it +fails by disappearing rather than by raising. + +The read half — the lineage clause that turns a branch into "this story" — +lands beside it in SP2 (`context/lineage.py`). Nothing here is read yet. + +Until forking ships there is exactly one branch per adventure and `depth` is +the number `index` already held, so everything in this module is bookkeeping +that changes nothing observable. That is the point: by the time a read depends +on these columns, every row has them — including the rows written between the +two deploys, which no migration will ever visit. +""" + +from sqlalchemy import func +from sqlalchemy.orm import Session + +from . import models + +# The head depth of an adventure with no actions. Keeps "the next node goes at +# head_depth + 1" true with no special case, and mirrors migrations.NO_DEPTH. +NO_DEPTH = -1 + + +def root_branch(db: Session, adventure: models.Adventure) -> models.Branch: + """The adventure's root branch, created on first use. + + Get-or-create rather than created-with-the-adventure, because the adventures + that need one most are the ones that already exist: a bundle being imported, + a fixture built straight through the ORM, or a database whose migration ran + before this code shipped. + """ + branch = ( + db.query(models.Branch) + .filter( + models.Branch.adventure_id == adventure.id, + models.Branch.parent_branch_id.is_(None), + ) + .order_by(models.Branch.id) + .first() + ) + if branch is not None: + return branch + branch = models.Branch(adventure_id=adventure.id, lineage=[]) + db.add(branch) + # The lineage names the branch's own id, so the row has to exist first. + db.flush() + branch.lineage = [[branch.id, None]] + return branch + + +def head_branch(db: Session, adventure: models.Adventure) -> models.Branch: + """The branch new nodes are played onto.""" + if adventure.head_branch_id is not None: + branch = db.get(models.Branch, adventure.head_branch_id) + if branch is not None: + return branch + # A head naming a branch that is gone is a bug somewhere else. Recover + # onto the root instead of refusing to play — the alternative is an + # adventure nobody can add to. + branch = root_branch(db, adventure) + adventure.head_branch_id = branch.id + return branch + + +def place_action( + db: Session, adventure: models.Adventure, action: models.Action +) -> models.Branch: + """Put `action` on the head branch and move the head to it. + + `depth` follows `index` while the two coexist. They have to agree: a read + ordering by depth and a cursor counting in index space are describing the + same story, and SP2 swaps one for the other under everything at once. + """ + branch = head_branch(db, adventure) + action.branch_id = branch.id + if action.depth is None: + action.depth = action.index + adventure.head_branch_id = branch.id + if action.depth > adventure.head_depth: + adventure.head_depth = action.depth + return branch + + +def place_memory( + db: Session, adventure: models.Adventure, memory: models.Memory +) -> models.Branch: + """Attach a memory to the node that produced it. + + `source_end` is the index of the last action the memory summarises, which is + that node's depth. A hand-written memory summarises nothing, so its depth + stays NULL and it belongs to the adventure rather than to a path. + """ + branch = head_branch(db, adventure) + memory.branch_id = branch.id + if memory.depth is None and memory.source_end is not None: + memory.depth = memory.source_end + return branch + + +def refresh_head(db: Session, adventure: models.Adventure) -> None: + """Re-derive the head depth after nodes were removed (undo, delete). + + A branch with nothing on it sits at its fork point, because that is the last + node its story contains — borrowed from the parent, but the tip all the + same. A root branch with nothing on it has no story at all. + """ + branch = head_branch(db, adventure) + tip = ( + db.query(func.max(models.Action.depth)) + .filter(models.Action.branch_id == branch.id) + .scalar() + ) + if tip is None: + tip = branch.fork_depth if branch.fork_depth is not None else NO_DEPTH + adventure.head_depth = tip diff --git a/backend/seed_demo.py b/backend/seed_demo.py index a18b3c0..cead708 100644 --- a/backend/seed_demo.py +++ b/backend/seed_demo.py @@ -9,7 +9,7 @@ adventure and script-library copies belong to the local user (only relevant on single-user installs). """ -from app import auth, models, migrations +from app import auth, models, migrations, tree from app.database import SessionLocal, engine # create_all + user_version stamp; plain create_all would leave a fresh DB at @@ -250,7 +250,13 @@ try: db.add(models.StoryCard(adventure_id=adventure.id, **card)) for position, s in enumerate(SCRIPTS): db.add(models.AdventureScript(adventure_id=adventure.id, position=position, **s)) - db.add(models.Action(adventure_id=adventure.id, index=0, type="start", text=scenario.prompt)) + opening = models.Action( + adventure_id=adventure.id, index=0, type="start", text=scenario.prompt + ) + # Through the same door create_adventure uses, so the seeded adventure has a + # story tree like every other one. + tree.place_action(db, adventure, opening) + db.add(opening) db.commit() print(f"Scenario id={scenario.id}: {scenario.title}") diff --git a/backend/tests/schema_rewind.py b/backend/tests/schema_rewind.py new file mode 100644 index 0000000..6cce7ea --- /dev/null +++ b/backend/tests/schema_rewind.py @@ -0,0 +1,47 @@ +"""Make a database look like an older schema version, so a migration can run. + +`create_all` always builds the *current* schema. A test that wants to watch a +migration happen therefore has to take the newer columns back off before it +stamps an older version — otherwise the migration meets a table that already +has its column and dies on a duplicate. + +Rewinding the stamp alone was enough for a while, which is why two test files +did exactly that. It stopped being enough the moment another `ADD COLUMN` +landed after theirs: the replay then runs migrations they never meant to +exercise, against columns `create_all` had already made. This module is that +rewind done properly, in one place, so appending a migration means adding its +inverse here rather than discovering three unrelated test failures. + +SQLite only — every test that replays migrations runs on a temp file, and +`PRAGMA user_version` is where the stamp lives there. Migrations that change a +column's *type* (43–45, JSON to compressed bytes) have no clean inverse and are +not listed: they get replayed as-is, which is what the tests using them already +relied on. +""" + +from sqlalchemy import text +from sqlalchemy.engine import Engine + +# (version that added it, statements that take it back off), newest first. +# +# Phase 14's `branch_id` columns are deliberately absent: SQLite refuses to drop +# a column a foreign key names ("unknown column in foreign key definition"), so +# a current-schema database cannot be rewound past them at all. That is what +# `migrations._column_already_there` is for — the replay skips DDL that has +# already happened, so the tree migrations run their backfill against a schema +# that already has the columns, which is exactly the situation here. +_UNDO: list[tuple[int, tuple[str, ...]]] = [ + # Packed float32 vectors and the flag beside them. + (39, ("ALTER TABLE memories DROP COLUMN embedded",)), + (38, ("ALTER TABLE memories DROP COLUMN embedding_blob",)), +] + + +def rewind_to(engine: Engine, version: int) -> None: + """Drop everything added after `version`, then stamp the database at it.""" + with engine.begin() as conn: + for added_at, statements in _UNDO: + if added_at > version: + for sql in statements: + conn.execute(text(sql)) + conn.execute(text(f"PRAGMA user_version = {version}")) diff --git a/backend/tests/test_embedding_blob.py b/backend/tests/test_embedding_blob.py index 47d8914..1dc9959 100644 --- a/backend/tests/test_embedding_blob.py +++ b/backend/tests/test_embedding_blob.py @@ -25,6 +25,7 @@ from sqlalchemy import text from app import memorybank, migrations, models, vectors from app.database import Base, SessionLocal, engine +from tests import schema_rewind def float32(value: float) -> float: @@ -270,10 +271,7 @@ def test_bootstrap_adds_the_columns_and_backfills_them(db, adventure): unembedded_id = unembedded.id db.close() - with engine.begin() as conn: - conn.execute(text("ALTER TABLE memories DROP COLUMN embedding_blob")) - conn.execute(text("ALTER TABLE memories DROP COLUMN embedded")) - conn.execute(text(f"PRAGMA user_version = {migrations.EMBEDDING_BLOB_VERSION - 1}")) + schema_rewind.rewind_to(engine, migrations.EMBEDDING_BLOB_VERSION - 1) migrations.bootstrap(engine) diff --git a/backend/tests/test_snapshot_compression.py b/backend/tests/test_snapshot_compression.py index 0cde792..4eab3ec 100644 --- a/backend/tests/test_snapshot_compression.py +++ b/backend/tests/test_snapshot_compression.py @@ -33,6 +33,7 @@ from sqlalchemy import text from app import compression, migrations, models from app.database import Base, SessionLocal, engine +from tests import schema_rewind from tools.fakeprose import prose @@ -196,8 +197,10 @@ def seed_pre_43(db, adventure, count: int = 4) -> dict[int, dict]: expected = {action_id: snapshot(action_id) for action_id in ids} as_json_column(db, expected) - db.execute(text(f"PRAGMA user_version = {migrations.SNAPSHOT_COMPRESS_VERSION - 1}")) db.commit() + # Take the later migrations' columns back off too, not just the stamp: + # replaying 43-45 also replays everything appended after them. + schema_rewind.rewind_to(engine, migrations.SNAPSHOT_COMPRESS_VERSION - 1) return expected diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py new file mode 100644 index 0000000..bf8bd77 --- /dev/null +++ b/backend/tests/test_tree_migration.py @@ -0,0 +1,457 @@ +"""Phase 14 SP1 — every existing adventure becomes a tree with one branch. + +The migration this file watches is the one that cannot be re-run: it reads +`index` and writes `depth`, and from SP2 on the reads follow `depth`. If it +mis-maps a row, that row does not error — it *disappears from the story*, which +is why the assertions here are about every row rather than about a sample. + +The fixture is a genuine **schema 45** database, not a current one with an old +stamp. `create_all` always builds the current schema, so the three tables the +tree touches are dropped and rebuilt from frozen pre-tree DDL below; the +migration then runs its real ALTERs against them, including the one that adds a +foreign key. A pre-migration database built any other way (stamp rewound, +columns left in place) would quietly skip the DDL and test half the change. + + python -m pytest tests/test_tree_migration.py -v +""" +import json +import os +import tempfile + +_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) +_tmp.close() +os.environ["AIDND_DB_PATH"] = _tmp.name +os.environ.pop("AIDND_DATABASE_URL", None) +os.environ.pop("DATABASE_URL", None) + +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient +from sqlalchemy import text + +from app import auth, limits, migrations, models, tree +from app.database import Base, SessionLocal, engine, get_db +from app.main import app + +# The three tables as they stood at schema 45, frozen. This is a snapshot of a +# past schema and must NOT be updated to track models.py — the whole point is +# that it lacks what SP1 adds. SQLite spelling only; the migration's Postgres +# half is exercised against a real server at deploy time (see plan/14). +PRE_TREE_DDL = ( + """ + CREATE TABLE adventures ( + id INTEGER NOT NULL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + scenario_id INTEGER, + title VARCHAR(200) NOT NULL DEFAULT 'Untitled Adventure', + memory TEXT NOT NULL DEFAULT '', + authors_note TEXT NOT NULL DEFAULT '', + ai_instructions TEXT NOT NULL DEFAULT '', + story_summary TEXT NOT NULL DEFAULT '', + script_state JSON NOT NULL DEFAULT '{}', + world_state JSON NOT NULL DEFAULT '{}', + placeholders JSON, + auto_summarize BOOLEAN NOT NULL DEFAULT 0, + memory_bank_enabled BOOLEAN NOT NULL DEFAULT 0, + memory_cursor INTEGER NOT NULL DEFAULT 0, + summary_cursor INTEGER NOT NULL DEFAULT 0, + created_at DATETIME, + updated_at DATETIME + ) + """, + """ + CREATE TABLE actions ( + id INTEGER NOT NULL PRIMARY KEY, + adventure_id INTEGER NOT NULL REFERENCES adventures(id) ON DELETE CASCADE, + "index" INTEGER NOT NULL, + type VARCHAR(20) NOT NULL, + text TEXT NOT NULL DEFAULT '', + reasoning TEXT, + context_snapshot BLOB, + world_delta JSON, + state_before JSON, + world_state_before JSON, + variants JSON, + variant_count INTEGER NOT NULL DEFAULT 0, + variant_index INTEGER NOT NULL DEFAULT 0, + created_at DATETIME + ) + """, + """ + CREATE TABLE memories ( + id INTEGER NOT NULL PRIMARY KEY, + adventure_id INTEGER NOT NULL REFERENCES adventures(id) ON DELETE CASCADE, + text TEXT NOT NULL DEFAULT '', + embedding_blob BLOB, + source_start INTEGER, + source_end INTEGER, + embedded BOOLEAN NOT NULL DEFAULT 0, + pinned BOOLEAN NOT NULL DEFAULT 0, + forgotten BOOLEAN NOT NULL DEFAULT 0, + use_count INTEGER NOT NULL DEFAULT 0, + last_used_at DATETIME, + created_at DATETIME + ) + """, +) + +# The story of adventure "Gapped": index 3 is missing, because deleting a middle +# action never renumbered the ones after it. The gap has to survive as a gap. +GAPPED_INDEXES = (0, 1, 2, 4) +STRAIGHT_INDEXES = (0, 1) + + +@pytest.fixture() +def pre_tree(): + """A schema-45 database with three adventures in it, returned as the ids + (gapped, straight, empty) their stories were written under.""" + # Every test in this module shares one temp file, and a setup that fails + # before its yield never reaches a teardown — so start from empty rather + # than from whatever the last one left. + Base.metadata.drop_all(bind=engine) + Base.metadata.create_all(bind=engine) + with engine.begin() as conn: + # `branches` and the six new columns never existed at 45. Dropping the + # tables is the only way to lose the columns: SQLite refuses to drop a + # column a foreign key names, which is exactly the case for branch_id. + for table in ("actions", "memories", "branches", "adventures"): + conn.execute(text(f"DROP TABLE IF EXISTS {table}")) + for ddl in PRE_TREE_DDL: + conn.execute(text(ddl)) + conn.execute(text( + "INSERT INTO users (id, email, is_guest, created_at, demo_turns_used, " + "demo_turns_date) VALUES (1, 'v45@example.com', 0, CURRENT_TIMESTAMP, 0, '')" + )) + + ids = {} + for name in ("Gapped", "Straight", "Empty"): + conn.execute(text( + "INSERT INTO adventures (user_id, title) VALUES (1, :title)" + ), {"title": name}) + ids[name] = conn.execute(text( + "SELECT id FROM adventures WHERE title = :title" + ), {"title": name}).scalar() + + for adventure_id, indexes in ( + (ids["Gapped"], GAPPED_INDEXES), + (ids["Straight"], STRAIGHT_INDEXES), + ): + for index in indexes: + conn.execute(text( + 'INSERT INTO actions (adventure_id, "index", type, text) ' + "VALUES (:a, :i, :t, :x)" + ), {"a": adventure_id, "i": index, + "t": "start" if index == 0 else "do", + "x": f"Turn {index}."}) + + # One memory that summarised a block of story, and one written by hand, + # which summarised nothing and so belongs to no node. + conn.execute(text( + "INSERT INTO memories (adventure_id, text, source_start, source_end) " + "VALUES (:a, 'The gate opened.', 0, 1)" + ), {"a": ids["Gapped"]}) + conn.execute(text( + "INSERT INTO memories (adventure_id, text) VALUES (:a, 'Hand-written.')" + ), {"a": ids["Gapped"]}) + + conn.execute(text("PRAGMA user_version = 45")) + + try: + yield ids + finally: + Base.metadata.drop_all(bind=engine) + + +def rows(sql: str, **params) -> list[tuple]: + with engine.begin() as conn: + return conn.execute(text(sql), params).all() + + +def scalar(sql: str, **params): + with engine.begin() as conn: + return conn.execute(text(sql), params).scalar() + + +# ------------------------------------------------------- the migration itself + +def test_the_stamp_reaches_the_current_version(pre_tree): + migrations.bootstrap(engine) + assert scalar("PRAGMA user_version") == migrations.LATEST_VERSION + + +def test_every_action_lands_on_its_adventure_root_branch(pre_tree): + before = scalar("SELECT count(*) FROM actions") + + migrations.bootstrap(engine) + + assert scalar("SELECT count(*) FROM actions") == before, "the migration lost a row" + assert scalar("SELECT count(*) FROM actions WHERE branch_id IS NULL") == 0 + assert scalar("SELECT count(*) FROM actions WHERE depth IS NULL") == 0 + # Each action's branch belongs to that action's own adventure. A branch + # clause that forgot its adventure would still look right on a database + # holding one, which is why the fixture holds three. + mismatched = scalar(""" + SELECT count(*) FROM actions a JOIN branches b ON b.id = a.branch_id + WHERE b.adventure_id != a.adventure_id + """) + assert mismatched == 0 + + +def test_depth_is_the_old_index_gaps_included(pre_tree): + migrations.bootstrap(engine) + + assert rows('SELECT "index", depth FROM actions WHERE depth != "index"') == [] + depths = [ + row[0] for row in rows( + "SELECT depth FROM actions WHERE adventure_id = :a ORDER BY depth", + a=pre_tree["Gapped"], + ) + ] + # 3 is still missing. Renumbering here would silently move every cursor + # pointing past the gap, and the reads only need the order, not density. + assert depths == list(GAPPED_INDEXES) + + +def test_one_root_branch_per_adventure_with_its_own_lineage(pre_tree): + migrations.bootstrap(engine) + + branches = rows( + "SELECT id, adventure_id, parent_branch_id, fork_depth, lineage FROM branches" + ) + assert len(branches) == 3, "one branch per adventure, including the empty one" + for branch_id, _adventure_id, parent, fork_depth, lineage in branches: + assert parent is None, "a migrated branch is a root; nothing forked yet" + assert fork_depth is None + # The whole story, uncapped: one entry, itself, no ceiling. + assert json.loads(lineage) == [[branch_id, None]] + + +def test_the_head_points_at_the_tip_of_the_root_branch(pre_tree): + migrations.bootstrap(engine) + + heads = dict(rows("SELECT title, head_depth FROM adventures")) + assert heads["Gapped"] == max(GAPPED_INDEXES) + assert heads["Straight"] == max(STRAIGHT_INDEXES) + # No actions, no tip. -1 keeps "the next node goes at head_depth + 1" true + # without a special case anywhere else. + assert heads["Empty"] == tree.NO_DEPTH + assert scalar("SELECT count(*) FROM adventures WHERE head_branch_id IS NULL") == 0 + dangling = scalar(""" + SELECT count(*) FROM adventures a + WHERE NOT EXISTS ( + SELECT 1 FROM branches b + WHERE b.id = a.head_branch_id AND b.adventure_id = a.id + ) + """) + assert dangling == 0, "a head pointing outside its own adventure" + + +def test_memories_attach_to_the_node_they_summarised(pre_tree): + migrations.bootstrap(engine) + + summarised = rows( + "SELECT source_end, depth, branch_id FROM memories WHERE source_end IS NOT NULL" + ) + assert summarised, "the fixture is supposed to have one" + for source_end, depth, branch_id in summarised: + assert depth == source_end, "the memory hangs off the last action it covered" + assert branch_id is not None + + # A hand-written memory has no node: it gets a branch, but no depth, which + # SP3 reads as belonging to the adventure rather than to a path. + manual = rows("SELECT depth, branch_id FROM memories WHERE source_end IS NULL") + assert manual and all(depth is None and branch is not None for depth, branch in manual) + + +def test_the_branch_clause_index_exists(pre_tree): + """SP2's reads are only cheap if this exists — and `create_all` does not add + an index to a table it did not create, which is what migration 52 is for.""" + migrations.bootstrap(engine) + + assert scalar( + "SELECT count(*) FROM sqlite_master " + "WHERE type = 'index' AND name = 'ix_actions_branch_depth'" + ) == 1 + + +def test_running_it_again_changes_nothing(pre_tree): + migrations.bootstrap(engine) + snapshot = ( + rows("SELECT id, branch_id, depth FROM actions ORDER BY id"), + rows("SELECT id, adventure_id, lineage FROM branches ORDER BY id"), + rows("SELECT id, head_branch_id, head_depth FROM adventures ORDER BY id"), + rows("SELECT id, branch_id, depth FROM memories ORDER BY id"), + ) + + # Twice through the deploy path, then the data pass on its own — the stamp + # stops the first, the NULL guards stop the second, and a migration that + # only survives because of the stamp is one bad rescue away from doubling + # every branch. + migrations.bootstrap(engine) + with engine.begin() as conn: + migrations._backfill_tree(conn) + + assert ( + rows("SELECT id, branch_id, depth FROM actions ORDER BY id"), + rows("SELECT id, adventure_id, lineage FROM branches ORDER BY id"), + rows("SELECT id, head_branch_id, head_depth FROM adventures ORDER BY id"), + rows("SELECT id, branch_id, depth FROM memories ORDER BY id"), + ) == snapshot + + +# -------------------------------------------------- rows written *after* it + +@pytest.fixture() +def client(monkeypatch): + """The app on a migrated database, so new rows go through the real writers. + + Everything the migration fixes is only half the job: no migration will ever + visit a row written after it ran, and a row without a branch is a row no + read can see. + """ + Base.metadata.drop_all(bind=engine) + Base.metadata.create_all(bind=engine) + setup = SessionLocal() + user = models.User(is_guest=False, email="writer@example.com") + setup.add(user) + setup.flush() + scenario = models.Scenario(user_id=user.id, title="S", prompt="You enter a cave.") + setup.add(scenario) + setup.commit() + user_id, scenario_id = user.id, scenario.id + setup.close() + + monkeypatch.setattr(limits, "rate_limit", lambda *a, **k: None) + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + + def _current_user(db=Depends(get_db)): + return db.get(models.User, user_id) + + app.dependency_overrides[auth.get_current_user] = _current_user + c = TestClient(app) + c.scenario_id = scenario_id + try: + yield c + finally: + app.dependency_overrides.clear() + Base.metadata.drop_all(bind=engine) + + +def test_a_new_adventure_gets_a_branch_and_its_opening_sits_on_it(client): + response = client.post("/api/adventures", json={"scenario_id": client.scenario_id}) + assert response.status_code == 201 + adventure_id = response.json()["id"] + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adventure_id) + branch = db.query(models.Branch).filter_by(adventure_id=adventure_id).one() + assert adventure.head_branch_id == branch.id + assert branch.lineage == [[branch.id, None]] + opening = db.query(models.Action).filter_by(adventure_id=adventure_id).one() + assert (opening.branch_id, opening.depth) == (branch.id, 0) + assert adventure.head_depth == 0 + finally: + db.close() + + +def test_a_blank_adventure_has_a_branch_before_anything_is_played(client): + adventure_id = client.post("/api/adventures", json={}).json()["id"] + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adventure_id) + assert adventure.head_branch_id is not None + assert adventure.head_depth == tree.NO_DEPTH + finally: + db.close() + + +def test_a_hand_written_memory_gets_a_branch_but_no_depth(client): + adventure_id = client.post("/api/adventures", json={}).json()["id"] + + created = client.post( + f"/api/adventures/{adventure_id}/memories", json={"text": "Remember the gate."} + ) + assert created.status_code == 201 + + db = SessionLocal() + try: + memory = db.query(models.Memory).filter_by(adventure_id=adventure_id).one() + assert memory.branch_id is not None + assert memory.depth is None + finally: + db.close() + + +def test_deleting_a_branch_takes_its_nodes_with_it(client): + """`ON DELETE CASCADE` on both `branch_id` columns, so the database removes a + branch's nodes rather than any code remembering to. SP7 ships delete-a-branch + on top of exactly this, and nothing else has to load a branch to do it.""" + adventure_id = client.post( + "/api/adventures", json={"scenario_id": client.scenario_id} + ).json()["id"] + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adventure_id) + memory = models.Memory( + adventure_id=adventure_id, text="m", source_start=0, source_end=0 + ) + tree.place_memory(db, adventure, memory) + db.add(memory) + db.commit() + branch_id = adventure.head_branch_id + + db.execute( + models.Branch.__table__.delete().where(models.Branch.id == branch_id) + ) + db.commit() + assert db.query(models.Action).filter_by(adventure_id=adventure_id).count() == 0 + assert db.query(models.Memory).filter_by(adventure_id=adventure_id).count() == 0 + finally: + db.close() + + +def test_deleting_an_adventure_takes_its_branch_with_it(client): + adventure_id = client.post( + "/api/adventures", json={"scenario_id": client.scenario_id} + ).json()["id"] + + assert client.delete(f"/api/adventures/{adventure_id}").status_code == 204 + + db = SessionLocal() + try: + assert db.query(models.Branch).filter_by(adventure_id=adventure_id).count() == 0 + finally: + db.close() + + +def test_deleting_the_newest_action_moves_the_head_back(client): + """The head is a cache, and a cache that only ever moves forward is wrong + the first time someone undoes a turn.""" + adventure_id = client.post( + "/api/adventures", json={"scenario_id": client.scenario_id} + ).json()["id"] + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adventure_id) + extra = models.Action(adventure_id=adventure_id, index=1, type="do", text="Look.") + tree.place_action(db, adventure, extra) + db.add(extra) + db.commit() + assert adventure.head_depth == 1 + action_id = extra.id + finally: + db.close() + + assert client.delete( + f"/api/adventures/{adventure_id}/actions/{action_id}" + ).status_code == 204 + + db = SessionLocal() + try: + assert db.get(models.Adventure, adventure_id).head_depth == 0 + finally: + db.close() diff --git a/backend/tools/stress_session.py b/backend/tools/stress_session.py index b50334a..2d64447 100644 --- a/backend/tools/stress_session.py +++ b/backend/tools/stress_session.py @@ -137,7 +137,7 @@ from fastapi import Depends from fastapi.testclient import TestClient from sqlalchemy import text -from app import auth, limits, memorybank, models, security, seed, worldstate +from app import auth, limits, memorybank, models, security, seed, tree, worldstate from app.database import Base, SessionLocal, engine, get_db from app.main import app from app.providers import PromptParts @@ -379,12 +379,14 @@ def add_second_adventure(db, rng: random.Random, user) -> int: db.add(other) db.flush() for i in range(6): - db.add(models.Action( + action = models.Action( adventure_id=other.id, index=i, type="ai" if i % 2 else "do", text=f"[second adventure] turn {i}. {prose(rng, 200)}", state_before=rich_script_state(i), - )) + ) + tree.place_action(db, other, action) + db.add(action) memory = models.Memory( adventure_id=other.id, text="This memory belongs to the second adventure and must never be " @@ -392,6 +394,7 @@ def add_second_adventure(db, rng: random.Random, user) -> int: source_start=0, source_end=5, ) memorybank.set_vector(memory, [rng.uniform(-1.0, 1.0) for _ in range(EMBEDDING_DIMS)]) + tree.place_memory(db, other, memory) db.add(memory) return other.id @@ -483,7 +486,7 @@ def build_fixture(args, rng: random.Random) -> tuple[int, int]: if retried else [] ), 0 body = NARRATION if is_ai else PLAYER_INPUT - db.add(models.Action( + action = models.Action( adventure_id=adventure.id, index=i, type="ai" if is_ai else "do", @@ -503,7 +506,13 @@ def build_fixture(args, rng: random.Random) -> tuple[int, int]: world_state_before=( rich_world_state(schema, i) if args.rich else None ), - )) + ) + # A fresh database is built by create_all and stamped LATEST, so no + # migration ever runs against it and the tree backfill never sees + # it. The fixture has to stamp its own nodes, or it would be the one + # database in the project whose actions have no branch. + tree.place_action(db, adventure, action) + db.add(action) for i in range(args.memories): memory = models.Memory( @@ -522,6 +531,7 @@ def build_fixture(args, rng: random.Random) -> tuple[int, int]: memorybank.set_vector( memory, [rng.uniform(-1.0, 1.0) for _ in range(EMBEDDING_DIMS)] ) + tree.place_memory(db, adventure, memory) db.add(memory) if args.rich: diff --git a/plan/14-phase-story-tree.md b/plan/14-phase-story-tree.md index 2692f0c..c2a5193 100644 --- a/plan/14-phase-story-tree.md +++ b/plan/14-phase-story-tree.md @@ -231,6 +231,50 @@ mapped. Bootstrap run twice is a no-op. `tests/test_egress.py` ceilings unmoved integers a row). **Post-deploy `VACUUM FULL actions;` is mandatory** — this rewrites every row, which is the 144 MB lesson at the top of `STATUS.md`. +**Done, 2026-08-17** (branch `sp1-tree-schema`). **297 tests green**, the 283 from SP0 +plus 14 in `test_tree_migration.py`. The baseline contract passes **unmodified**, which +was the pass condition. Five things the plan did not anticipate, all of them found by +building it: + +- **The writes could not wait for SP2.** The file table above lists only `models.py` and + `migrations.py`, but a migration never visits a row written *after* it runs — so + shipping the columns without a writer would leave every turn played between the two + deploys with no branch, and from SP2 on a row with no branch is a row no read can see. + `app/tree.py` is that writer: `root_branch` / `head_branch` (get-or-create), + `place_action`, `place_memory`, `refresh_head`. One module for the same reason SP2 gets + one — a node written without a branch fails by *disappearing*, not by raising. Wired + into create/turn/import/undo/delete/memory, plus `seed_demo.py` and + `tools.stress_session` (a fixture built by `create_all` is stamped LATEST, so no + migration ever runs against it). +- **`adventures.head_branch_id` cannot be a foreign key.** `branches.adventure_id` + already points the other way, and the pair is then a cycle `create_all` refuses to + order; the fix for that is `use_alter`, which SQLite has no ALTER for. It is a plain + integer, documented as a cache, and `head_branch` recovers onto the root if it ever + names a branch that is gone. +- **`lineage` is NOT NULL, because `branches` comes from `create_all`.** The backfill + cannot insert a row and fill the lineage afterwards via a NULL marker, so it inserts + `'[]'` and guards step two on `json_array_length(lineage) = 0` — not `= '[]'`, because + Postgres `json` has no equality operator. +- **SQLite cannot drop a column a foreign key names.** So a current-schema database + cannot be rewound past `branch_id` at all, which broke the two existing tests that + simulate an old database by rewinding only the *stamp*. Fixed properly: + `migrations._column_already_there` makes every `ADD COLUMN` idempotent (the + `IF NOT EXISTS` the module docstring asks for and SQLite has no syntax for), and + `tests/schema_rewind.py` holds the inverse of the migrations that *can* be undone. + The SP1 fixture therefore builds a **genuine schema 45** by dropping the three tables + and recreating them from frozen pre-tree DDL, so the real ALTERs run — including the + one that adds a foreign key. +- **Deleting a branch takes its nodes with it**, and deleting an adventure takes its + branch — both verified, both at the database level via `ON DELETE CASCADE` on the two + `branch_id` columns. SP7's delete-a-branch needs no code of its own for the nodes. + +Measured: `branches` costs **0.1 kB of a 733.5 kB turn (0 %)**, and the page load +(62.6 kB) and index (1.8 kB) shapes are byte-identical to the figures in `STATUS.md`. +One cost that is *not* free: the new table and its index add ~47 ms to every +`create_all`/`drop_all` cycle on SQLite, and the suite does one per test — 20 s → 38 s. +Test-only (DDL fsync), so no model change; if it ever matters, the fix is the test +harness, not the schema. + ### SP2 — The branch clause *(reads move to lineage; still one branch)* One module owns the clause; every action read goes through it. A forgotten clause shows diff --git a/plan/STATUS.md b/plan/STATUS.md index 7b13c37..1a0d1e1 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -78,8 +78,16 @@ needed; nothing requires reading a row of anyone's story. ## Pick up here -**`plan/14-phase-story-tree.md` — the tree itself.** `plan/13` is finished. Its design -is settled in `plan/14` and nothing about it has been built. +**`plan/14-phase-story-tree.md`, SP2 — the branch clause.** SP0 (the regression contract +and the `--rich` fixture) and SP1 (schema, migration, and the writer that keeps new rows +on the tree) are done and green; nothing is deployed yet. SP2 is where the reads move +onto `(branch_id, depth)`, and where the highest-risk line in the whole phase lives: +`history._from_memory()` slices `adventure.actions`, which under a tree is *every +branch's* actions rather than the path. Make that shortcut branch-aware or delete it. + +**The schema is live in code but not on production.** When SP1 ships, the deploy needs +one `VACUUM FULL actions;` on the direct (non-`-pooler`) endpoint afterwards — it rewrites +every row. See the 144 MB lesson at the top of this file. Two things from the egress work are worth carrying into it: @@ -101,6 +109,40 @@ drive it before rewriting it. --- +## What happened on 2026-08-17, part four — the tree, SP0 and SP1 + +No behaviour change, and none intended: a linear story is a tree with one branch, so +every adventure reads exactly as it did. **297 tests green** (259 before the phase +started, 283 with SP0's contract, 297 with SP1's migration tests). + +**SP0** built `tests/test_story_tree_baseline.py` — 24 tests driving the product over +HTTP, asserting only on API responses — and `tools.stress_session --rich`, a correctness +fixture beside the scale one. Both on branch `phase-14-story-tree`. + +**SP1** put the tree in the schema, on branch `sp1-tree-schema`: a `branches` table, +`branch_id`/`depth` on actions and memories, `head_branch_id`/`head_depth` on adventures, +migrations 46–52 with a server-side backfill, and `app/tree.py` for the write side. +Nothing reads any of it yet. Four things worth not rediscovering: + +- **A schema needs its writer in the same subphase.** No migration will ever visit a row + written after it ran, so columns backfilled today and populated-on-write next week leave + a hole exactly the width of one deploy. `app/tree.py` stamps every new node, including + the ones `seed_demo.py` and the stress fixture write — a fixture built by `create_all` + is stamped LATEST and no migration ever touches it. +- **SQLite will not drop a column a foreign key names.** Two tests simulated an old + database by rewinding the *stamp* while `create_all` left the new columns in place; that + works until the next `ADD COLUMN` lands, and then it fails on a duplicate column. Every + `ADD COLUMN` migration is now idempotent (`migrations._column_already_there`), which is + the `IF NOT EXISTS` SQLite has no syntax for. A true pre-migration fixture has to drop + and rebuild the tables from frozen DDL, which is what `test_tree_migration.py` does. +- **Two mutually-referencing tables cannot both carry the foreign key.** `create_all` + refuses to order the cycle, and its escape hatch (`use_alter`) needs an ALTER SQLite + does not have. `adventures.head_branch_id` is a plain integer and a documented cache. +- **The suite went 20 s → 38 s, and it is not the app.** One new table plus one index adds + ~47 ms to a `create_all`/`drop_all` pair on SQLite (DDL fsync), and nearly every test + does one. Measured, not guessed. Egress is unmoved: `branches` is 0.1 kB of a 733.5 kB + turn, and the page-load and index shapes are byte-identical to the numbers above. + ## What happened on 2026-08-17, part three No behaviour change. A way to get a long adventure in front of a browser, because the @@ -330,7 +372,7 @@ the SQLite dev parity this codebase protects on purpose). ``` cd backend -.venv/Scripts/python.exe -m pytest tests/ # 259 tests +.venv/Scripts/python.exe -m pytest tests/ # 297 tests (~38s) .venv/Scripts/python.exe -m tools.stress_session # egress report (SQLite) # Same harness against a real Postgres. The target must be a THROWAWAY database