diff --git a/backend/app/attempts.py b/backend/app/attempts.py new file mode 100644 index 0000000..a17a967 --- /dev/null +++ b/backend/app/attempts.py @@ -0,0 +1,220 @@ +"""Phase 14, SP4 — the attempts at one turn. + +A retry used to rewrite the AI action in place and push the discarded take into +a JSON list on the same row. That is where seven separate bugs came from: the +row's `text` mirrored one entry of a repeating group, `variant_count` mirrored +its length, and every reader that touched the story during a retry had to be +told to pretend the row was not there. + +Now an attempt is a **node**. Retry writes a sibling at the same +`(branch_id, depth)` and marks it live; the previous one stays exactly as it +was written, at the same coordinate, `live = False`. Nothing is mirrored, so +nothing can drift. + +Two invariants hold the arrangement together, and this module is the only place +that maintains either: + +* **Exactly one sibling in a group is live.** `lineage.Path.clause` selects on + it, so the losing attempts are invisible to every read of the story without + any of those reads knowing that attempts exist. +* **The assembled prompt is stored once per turn, on the live sibling.** + A `context_snapshot` is ~163 kB of prompt that every attempt at a turn + shares, plus a few hundred bytes that differ (`ATTEMPT_KEYS`). Giving each + sibling its own copy would have made retry a permanent multiplier on the + biggest column in the database — the thing the JSON list was invented to + avoid. So the prompt moves with the live flag, and a superseded sibling keeps + only its own slices. + +Ordering inside a group is `variant_index`, an explicit ordinal, not +`created_at`. Two attempts made in the same second must still page in the order +they were made, and the migration that split the old JSON lists had to be able +to state the order rather than reconstruct it. +""" + +import copy + +from sqlalchemy.orm import Session, undefer + +from . import models +from .context import lineage + +# The slices of a context snapshot that belong to one attempt rather than to +# the turn: the world-state delta it proposed and what the referee did with it, +# the script report, and the model's literal reply. Everything else in a +# snapshot is the prompt, which is assembled once per turn. +ATTEMPT_KEYS = ("world_state", "script", "raw_output") + + +# ------------------------------------------------------------------ reading + +def group(db: Session, action: models.Action) -> list[models.Action]: + """Every attempt at `action`'s turn, oldest first. + + A node with no branch is a pre-tree row that no path contains; it is its + own only attempt, and saying so here saves every caller a special case. + """ + if action.branch_id is None or action.depth is None: + return [action] + return ( + db.query(models.Action) + .filter( + models.Action.adventure_id == action.adventure_id, + models.Action.branch_id == action.branch_id, + models.Action.depth == action.depth, + ) + .order_by(models.Action.variant_index, models.Action.id) + .all() + ) + + +def live_in(rows: list[models.Action]) -> models.Action | None: + for row in rows: + if row.live: + return row + return None + + +def preceding( + db: Session, adventure: models.Adventure, node: models.Action +) -> models.Action | None: + """The node the story tells immediately before `node`. + + "Before this turn" as a fact about the path rather than as a snapshot taken + from inside the turn — which is what makes the after-snapshots enough on + their own. Undefers both of them because the only reason to ask for this + row is to put back what it left behind. + """ + if node.depth is None: + return None + return ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adventure.id, + lineage.path_of(db, adventure).clause(models.Action), + models.Action.depth < node.depth, + ) + .options( + undefer(models.Action.state_after), + undefer(models.Action.world_state_after), + ) + .order_by(models.Action.depth.desc(), models.Action.id.desc()) + .first() + ) + + +# ------------------------------------------------------------------ writing + +def restore_state(adventure: models.Adventure, node: models.Action | None) -> None: + """Put back the script scoreboard and world state `node` left behind. + + A NULL snapshot means "leave the live state alone", never "reset it": rows + written before SP4 that the migration could not derive an outcome for carry + NULLs, and clobbering a running adventure's scoreboard with an empty dict + would be a far worse answer than doing nothing. + """ + if node is None: + return + if isinstance(node.state_after, dict): + adventure.script_state = copy.deepcopy(node.state_after) + if isinstance(node.world_state_after, dict): + adventure.world_state = copy.deepcopy(node.world_state_after) + + +def snapshot_outcome(adventure: models.Adventure, node: models.Action) -> None: + """Record on `node` what the adventure looks like now that it has played.""" + state = adventure.script_state if isinstance(adventure.script_state, dict) else {} + world = adventure.world_state if isinstance(adventure.world_state, dict) else {} + node.state_after = copy.deepcopy(state) + node.world_state_after = copy.deepcopy(world) + + +def roll_back_before( + db: Session, adventure: models.Adventure, node: models.Action +) -> None: + """Rewind the shared state to before `node` was played.""" + restore_state(adventure, preceding(db, adventure, node)) + + +def add_attempt( + db: Session, + adventure: models.Adventure, + previous: models.Action, + replacement: models.Action, +) -> None: + """Put `replacement` beside `previous` as the newer attempt at that turn. + + Placed by hand rather than through `tree.place_action`, which would read the + depth off the legacy `index` and move the head: a sibling is not a new turn, + it is another take on the one the head is already standing on. + """ + replacement.branch_id = previous.branch_id + replacement.depth = previous.depth + replacement.live = True + replacement.variant_index = previous.variant_index + 1 + previous.live = False + # The replacement was assembled with a fresh snapshot, so the prompt for + # this turn is now the one it carries; the superseded attempt keeps only + # what was its own. + keep_own_slices(previous) + + +def make_live( + db: Session, adventure: models.Adventure, node: models.Action +) -> list[models.Action]: + """Make `node` the attempt the story tells, and put its outcome back. + + Returns the group, renumbered, so a caller that wants to report on it does + not read it twice. + """ + rows = group(db, node) + previous = live_in(rows) + if previous is not None and previous is not node: + hand_over_the_prompt(previous, node) + for row in rows: + row.live = row is node + restore_state(adventure, node) + renumber(rows) + return rows + + +def renumber(rows: list[models.Action]) -> None: + """Refresh the group-shape cache the page response reads. + + `variant_count` is 0 rather than 1 for a turn nobody retried, because the + pager's question is "is there anything to page through?" and the answer for + a single attempt is no. + """ + count = len(rows) if len(rows) > 1 else 0 + for i, row in enumerate(rows): + row.variant_index = i + row.variant_count = count + + +# ------------------------------------------------- the prompt, stored once + +def keep_own_slices(node: models.Action) -> None: + """Strip `node`'s snapshot back to what is only its own.""" + snapshot = node.context_snapshot + if not isinstance(snapshot, dict): + return + node.context_snapshot = { + key: snapshot[key] for key in ATTEMPT_KEYS if key in snapshot + } or None + + +def hand_over_the_prompt(giver: models.Action, taker: models.Action) -> None: + """Move the turn's assembled prompt from one attempt to another. + + Called when the live flag moves, so the row in the story is always the row + the Insights viewer can explain. Nothing is copied — the prompt exists once + before and once after, on whichever sibling is being read. + """ + held = giver.context_snapshot if isinstance(giver.context_snapshot, dict) else {} + shared = {k: v for k, v in held.items() if k not in ATTEMPT_KEYS} + if not shared: + return + keep_own_slices(giver) + own = taker.context_snapshot if isinstance(taker.context_snapshot, dict) else {} + taker.context_snapshot = shared | { + k: v for k, v in own.items() if k in ATTEMPT_KEYS + } diff --git a/backend/app/context/history.py b/backend/app/context/history.py index 4b4dfe7..2ff37f2 100644 --- a/backend/app/context/history.py +++ b/backend/app/context/history.py @@ -28,12 +28,12 @@ Three rules hold everything together: SQL does. This is the line that would silently assemble a prompt out of two different stories, which is why `lineage.Path` owns both halves of it. -Ordering is by `depth` now, not `index`. The two hold the same numbers until -retry stops mutating rows (SP4), but only one of them is a position along a -path. +Ordering is by `depth` now, not `index`. Since SP4 the two can hold the same +number on *different rows* — attempts at one turn share both — so only `depth` +plus the branch clause's `live` test says which of them the story is. SP3 added the reads that count *from a node* rather than from the start — -`count_after`, `after`, `newest_settled`. The memory bank used to ask for +`count_after`, `after`, `newest`. The memory bank used to ask for "positions 12 to 18 of the story", which is a question whose answer moves when an action is deleted from in front of it. It now asks for "the six actions after depth 41", which is the same question a fork has to answer anyway. @@ -202,8 +202,9 @@ def story_actions( or `count` anywhere the caller only needs part of it. `exclude_action_id` drops one action from the story — used by retry, where - the row being regenerated is still attached to the adventure (it holds the - variant history) but must not appear in the context assembled to replace it. + the attempt being replaced is still the live node of its turn (it stays + live until a replacement exists) but must not appear in the context + assembled to replace it. """ in_memory = _from_memory(adventure, exclude_action_id) if in_memory is not None: @@ -387,15 +388,20 @@ def after( ) -def newest_settled(adventure: models.Adventure) -> models.Action | None: - """The newest story action that is not the newest one — see - `memorybank.settled_story_actions` for why one is always held back. +def newest(adventure: models.Adventure) -> models.Action | None: + """The newest story action, or None on an empty story. - Two rows, not a count and an offset: this is the node an anchor moves to - when derived work catches up with the settled end of the story. + A row, not a count and an offset: this is the node an anchor moves to when + derived work catches up with the end of the story. + + It used to be the *second* newest — the memory bank held one action back + because retry rewrote a row, so a memory covering the newest action could + end up describing narration the player had retried away. Since SP4 a retry + writes a sibling instead, and the coordinate's derived work is withdrawn + when the story at it changes, so there is nothing left to hold back. """ - rows = tail(adventure, 2) - return rows[0] if len(rows) == 2 else None + rows = tail(adventure, 1) + return rows[0] if rows else None def max_action_index(adventure: models.Adventure) -> int: diff --git a/backend/app/context/lineage.py b/backend/app/context/lineage.py index 90bc0dc..0d6e609 100644 --- a/backend/app/context/lineage.py +++ b/backend/app/context/lineage.py @@ -102,6 +102,12 @@ class Path: after it was typed. An action with no depth is a pre-tree row that no read should see, so actions never pass this. + Actions also have to be *live* (SP4). A coordinate can hold several + attempts at the same turn, and the story tells one of them; the losing + siblings sit at the same branch and depth and are excluded here, once, + so that no read of the story has to know that retries exist. Only + `app/attempts.py` looks past this. + An empty path yields `false`, not "no filter": an adventure whose nodes carry no branch has no story, and the loud version of that is an empty page, not every branch at once. @@ -109,7 +115,10 @@ class Path: entries = self.entries if count is None else self.entries[:count] if not entries: return false() - return or_(*[self._entry_clause(model, b, d, unanchored) for b, d in entries]) + on_path = or_(*[self._entry_clause(model, b, d, unanchored) for b, d in entries]) + if model is models.Action: + return and_(on_path, models.Action.live.is_(True)) + return on_path @staticmethod def _entry_clause(model, branch_id: int, max_depth: int | None, unanchored=False): @@ -128,7 +137,12 @@ class Path: Used where the rows are already in memory (the scripting pipeline hands user scripts the whole history), so an already-loaded collection can be cut down to the path without a second read. + + `live` is checked first, and only on rows that have the attribute: + memories have no siblings to lose to. """ + if getattr(node, "live", True) is False: + return False for branch_id, max_depth in self.entries: if node.branch_id != branch_id: continue diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index 7454e30..efb93b3 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -4,9 +4,7 @@ After each turn, a fire-and-forget task (`run_post_turn`) runs with its own DB session: - every MEMORY_INTERVAL actions (starting at MEMORY_START), each uncovered - block of actions is summarized into a short "memory". Summarization only - ever reads *settled* actions (see settled_story_actions) — the newest action - is held back one turn because it is still retryable; + block of actions is summarized into a short "memory"; - every SUMMARY_INTERVAL actions, the Story Summary is rewritten folding in the new memories (the user-edited text is always the base, never clobbered); - new memories are embedded (OpenAI-compatible /v1/embeddings) and the bank @@ -156,48 +154,6 @@ def _vectors_for(db: Session, adventure_id: int, ids: list[int]) -> dict[int, ar return cached -def settled_count(adventure: models.Adventure) -> int: - """How many story actions are old enough to summarize: all but the newest. - - See settled_story_actions for why one action is held back. Counting rather - than listing keeps the post-turn pass off the whole story. - """ - return max(history.count(adventure) - 1, 0) - - -def settled_after(adventure: models.Adventure, depth: int) -> int: - """How many settled story actions lie past `depth`. - - "How much story this pass has not read yet". The newest action is never - settled, so it is the one subtracted — and a cursor sitting at or past the - tip (undo moved the story back behind it) comes out at zero or below and - simply does no work, which is what the position cursors needed a clamp - every post-turn pass to achieve. - """ - return history.count_after(adventure, depth) - 1 - - -def settled_story_actions(adventure: models.Adventure) -> list[models.Action]: - """Story actions old enough to summarize: everything but the newest one. - - The plain-list form of the rule. The passes below use `settled_count` and - `settled_after` instead, which express the same thing without reading the - whole story; this stays as the statement of what they must agree with. - - Only the *last* action can be retried, so once an action has another action - after it, its text is final. Summarizing right up to the newest action meant - a memory could describe an attempt the player then retried away — the - memory's cursor has already advanced, so it is never regenerated, leaving a - memory (and, downstream, a story summary) describing narration that is no - longer in the story. Holding one action back costs a turn of latency and - makes that unreachable. - - The result is always a prefix of the story, so an anchor set from it can - never sit past the settled end and no action is ever skipped. - """ - return story_actions(adventure)[:-1] - - def forget_node(db: Session, adventure: models.Adventure, action: models.Action) -> int: """Withdraw what a node produced, because the node is being removed. @@ -401,9 +357,9 @@ async def _create_due_memories( # but this loop is the only thing that moves the anchor, so both # numbers have to be current. anchor = cursors.MEMORY.depth(db, adventure) - if settled_after(adventure, anchor) < MEMORY_INTERVAL: - return # no full block of settled story past the mark - if settled_count(adventure) < MEMORY_START: + if history.count_after(adventure, anchor) < MEMORY_INTERVAL: + return # no full block of story past the mark + if history.count(adventure) < MEMORY_START: return # ...and the adventure is too short to have started at all # (that order on purpose: the common answer is "nothing due", and the # first question answers it without asking how long the story is) @@ -440,13 +396,13 @@ async def _update_story_summary( adventure: models.Adventure, settings: models.Settings, db: Session ) -> None: anchor = cursors.SUMMARY.depth(db, adventure) - uncovered = settled_after(adventure, anchor) + uncovered = history.count_after(adventure, anchor) if uncovered < SUMMARY_INTERVAL: return # Where the summary will stand once this run succeeds. Read before the AI - # call, not after: the mark is the settled end of the story as this pass - # saw it, and a turn landing meanwhile must not be quietly claimed as read. - caught_up = history.newest_settled(adventure) + # call, not after: the mark is the end of the story as this pass saw it, + # and a turn landing meanwhile must not be quietly claimed as read. + caught_up = history.newest(adventure) if caught_up is None: return diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 16bd906..c1fb0dd 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -19,6 +19,7 @@ migrations added from Phase 9 on must run on both dialects. import json import re +from datetime import datetime from sqlalchemy import inspect, text from sqlalchemy.engine import Engine @@ -226,6 +227,30 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ (54, "ALTER TABLE adventures ADD COLUMN memory_cursor_depth INTEGER NOT NULL DEFAULT -1"), (55, "ALTER TABLE adventures ADD COLUMN summary_cursor_branch_id INTEGER"), (56, "ALTER TABLE adventures ADD COLUMN summary_cursor_depth INTEGER NOT NULL DEFAULT -1"), + # Phase 14, SP4 — a retry stops rewriting a row and writes a sibling beside + # it. `live` says which sibling the story tells; `state_after` / + # `world_state_after` give each attempt its own outcome to be switched back + # to. The legacy `variants` / `variant_index` / `state_before` / + # `world_state_before` columns stay, unread, until SP8. + # + # **This rewrites every row of `actions` three times** (one ADD COLUMN + # backfill pass each on Postgres) and then inserts a row per discarded + # attempt, so the deploy that ships it must be followed by, once: + # + # VACUUM FULL actions; + # + # on the direct endpoint, not -pooler. Same 144 MB lesson as SP1's. + (57, "ALTER TABLE actions ADD COLUMN live BOOLEAN NOT NULL DEFAULT true"), + (58, "ALTER TABLE actions ADD COLUMN state_after JSON"), + (59, "ALTER TABLE actions ADD COLUMN world_state_after JSON"), + # The two data passes, hung off a version of their own so they run after + # all three columns exist: derive the after-snapshots from the before-ones + # (_backfill_state_after), then split each `variants` list into sibling + # rows (_split_variants_into_siblings). The index below is the one those + # siblings make worth having — a group lookup is (branch_id, depth) with a + # handful of rows behind it, which ix_actions_branch_depth already serves, + # so this is a no-op statement that gives the passes a version to hang on. + (60, "CREATE INDEX IF NOT EXISTS ix_actions_branch_depth ON actions (branch_id, depth)"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) @@ -237,6 +262,7 @@ EMBEDDING_BLOB_VERSION = 38 SNAPSHOT_COMPRESS_VERSION = 43 TREE_BACKFILL_VERSION = 52 CURSOR_ANCHOR_VERSION = 56 +SIBLING_SPLIT_VERSION = 60 # 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). @@ -569,6 +595,210 @@ def _backfill_cursor_anchors(conn) -> None: """)) +# The per-attempt slices of a context snapshot, frozen here as they stood at +# version 60 (`adventures.VARIANT_SNAPSHOT_KEYS`, now `attempts.ATTEMPT_KEYS`). +# Everything else in a snapshot is the assembled prompt, which every attempt at +# one turn shares — which is why a discarded attempt's row carries only these. +_ATTEMPT_KEYS = ("world_state", "script", "raw_output") + + +def _backfill_state_after(conn) -> None: + """Turn each action's "before" snapshots into the "after" ones SP4 reads. + + The state a node left behind is the state the node in front of it started + from, so `state_after` of action *n* is `state_before` of action *n + 1* — + exactly, not approximately: the hooks that ran between them are this turn's, + and both snapshots were taken with them already applied. The newest action + of an adventure has nothing in front of it, and what it left behind is + simply what the adventure is carrying right now. + + Two statements per column rather than one COALESCE, because the second is + guarded on there being no later action at all — a row whose successor + predates `state_before` must stay NULL rather than inherit the tip's state. + + Ordered by `index`, which is still the story's order at version 60: SP4 is + the first migration where `depth` and `index` can disagree, and it has not + run yet when this does. + """ + for column, live in ( + ("state_after", "script_state"), + ("world_state_after", "world_state"), + ): + before = column.replace("_after", "_before") + conn.execute(text(f""" + UPDATE actions SET {column} = ( + SELECT n.{before} FROM actions n + WHERE n.adventure_id = actions.adventure_id + AND n."index" > actions."index" + ORDER BY n."index", n.id LIMIT 1 + ) + WHERE actions.{column} IS NULL + """)) + conn.execute(text(f""" + UPDATE actions SET {column} = ( + SELECT adv.{live} FROM adventures adv + WHERE adv.id = actions.adventure_id + ) + WHERE actions.{column} IS NULL + AND NOT EXISTS ( + SELECT 1 FROM actions n + WHERE n.adventure_id = actions.adventure_id + AND n."index" > actions."index" + ) + """)) + + +def _split_variants_into_siblings(conn) -> None: + """Give every discarded retry attempt the row it should always have had. + + `actions.variants` is a JSON repeating group: attempts at one turn, oldest + first, with `variant_index` naming the one the row's own `text` mirrors. + SP4 makes each attempt a node — same branch, same depth, `live` on exactly + one of them — so this reads the list one last time and writes it out as + siblings. + + The row that already exists keeps its attempt (the one it mirrors) and its + context snapshot, which is the assembled prompt for the whole turn and is + stored once per turn, never once per attempt. A sibling's snapshot carries + only the slices that actually differ between attempts, which is precisely + what the JSON entry held — so this changes how the bytes are arranged and + not how many there are. + + In Python rather than SQL because the shape of the work is "iterate a JSON + array and insert a row per element", which the two dialects spell + differently and neither spells well. It is bounded by the number of turns + anyone has ever retried, not by the size of the table, and it reads no + `context_snapshot` at all. + + Resumable: a group that already has as many rows as its `variant_count` + claims has been split, and is skipped. + """ + actions = Base.metadata.tables["actions"] + last_id = 0 + while True: + rows = conn.execute( + text(""" + SELECT id, adventure_id, branch_id, depth, "index", type, + created_at, variants, variant_index + FROM actions + WHERE variants IS NOT NULL AND id > :last + ORDER BY id LIMIT :batch + """), + {"last": last_id, "batch": BACKFILL_BATCH}, + ).mappings().all() + if not rows: + return + last_id = rows[-1]["id"] + for row in rows: + entries = row["variants"] + if isinstance(entries, str): + entries = json.loads(entries) + if not isinstance(entries, list) or len(entries) < 2: + continue + _split_one_action(conn, actions, row, entries) + + +def _split_one_action(conn, actions, row, entries: list) -> None: + live_index = row["variant_index"] or 0 + live_index = min(max(live_index, 0), len(entries) - 1) + already = conn.execute( + text(""" + SELECT COUNT(*) FROM actions + WHERE adventure_id = :adv AND branch_id = :branch AND depth = :depth + """), + {"adv": row["adventure_id"], "branch": row["branch_id"], "depth": row["depth"]}, + ).scalar() + if already and already >= len(entries): + return # a previous run of this pass already split it + # The live attempt's own recorded outcome beats the one _backfill_state_after + # derived from the turn in front of it — they agree, but only one of them is + # a fact about this attempt. Left alone where the entry has none to give + # (an adventure with no RPG layer stores no world state per attempt), so a + # derived value is never overwritten with a NULL. + kept = {"live": True, "variant_count": len(entries), "variant_index": live_index} + live_state = _entry_script_state(entries[live_index]) + live_world = _entry_world_state(entries[live_index]) + if live_state is not None: + kept["state_after"] = live_state + if live_world is not None: + kept["world_state_after"] = live_world + # Through the Table rather than text(), here and below: these values are + # dicts headed for JSON columns, and the column type is the only thing that + # knows how to spell one on this dialect. + conn.execute(actions.update().where(actions.c.id == row["id"]).values(**kept)) + siblings = [ + { + "adventure_id": row["adventure_id"], + "index": row["index"], + "branch_id": row["branch_id"], + "depth": row["depth"], + "live": False, + "type": row["type"], + "text": str(entry.get("text") or ""), + "reasoning": entry.get("reasoning"), + "context_snapshot": _attempt_snapshot(entry) or None, + "world_delta": _entry_world_delta(entry), + "state_before": None, + "world_state_before": None, + "state_after": _entry_script_state(entry), + "world_state_after": _entry_world_state(entry), + "variants": None, + "variant_count": len(entries), + "variant_index": i, + "created_at": _entry_created_at(entry, row["created_at"]), + } + for i, entry in enumerate(entries) + if i != live_index and isinstance(entry, dict) + ] + if siblings: + conn.execute(actions.insert(), siblings) + + +def _entry_script_state(entry) -> dict | None: + state = (entry or {}).get("script_state") + return state if isinstance(state, dict) else None + + +def _entry_world_state(entry) -> dict | None: + state = ((entry or {}).get("world_state") or {}).get("state") + return state if isinstance(state, dict) else None + + +def _entry_world_delta(entry) -> dict | None: + ws = (entry or {}).get("world_state") + if not isinstance(ws, dict): + return None + return { + "delta": ws.get("delta") or {}, + "applied": (ws.get("report") or {}).get("applied") or [], + } + + +def _attempt_snapshot(entry) -> dict: + return {k: entry[k] for k in _ATTEMPT_KEYS if isinstance(entry, dict) and k in entry} + + +def _entry_created_at(entry, fallback): + """The attempt's own timestamp, falling back to the row's. + + Only ever cosmetic — sibling order is `variant_index`, which this pass + writes explicitly, precisely so that nothing depends on two attempts made + in the same second sorting the way they were made. + """ + raw = (entry or {}).get("created_at") + if isinstance(raw, str) and raw: + try: + return datetime.fromisoformat(raw) + except ValueError: + pass + if isinstance(fallback, str): + try: + return datetime.fromisoformat(fallback) + except ValueError: + return None + return fallback + + def _get_version(conn) -> int: if conn.dialect.name == "sqlite": return conn.execute(text("PRAGMA user_version")).scalar() or 1 @@ -626,6 +856,13 @@ def bootstrap(engine: Engine) -> None: _backfill_tree(conn) if version == CURSOR_ANCHOR_VERSION: _backfill_cursor_anchors(conn) + # Order matters: the split reads what the first pass wrote for + # the rows it does not touch, and overwrites it for the ones it + # does — an attempt's own outcome beats one derived from the + # turn after it. + if version == SIBLING_SPLIT_VERSION: + _backfill_state_after(conn) + _split_variants_into_siblings(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 2d2dd16..1c32917 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -324,6 +324,17 @@ class Action(Base): ForeignKey("branches.id", ondelete="CASCADE"), nullable=True ) depth: Mapped[int | None] = mapped_column(Integer, nullable=True) + # Phase 14, SP4: whether this node is the one the story tells at its + # coordinate. Retry no longer rewrites a row — it writes a *sibling* at the + # same (branch, depth), so a coordinate can hold several attempts and + # exactly one of them is on the path. `lineage.Path.clause` is the only + # place that reads this, for the same reason it is the only place that + # knows about branches: an attempt leaking into a read is a story quietly + # telling itself twice. + # + # A node with no siblings is live, which is why the default is True and why + # every pre-SP4 row is correct without being visited. + live: Mapped[bool] = mapped_column(Boolean, default=True, nullable=False) 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). @@ -348,30 +359,58 @@ class Action(Base): # and for re-attaching the emit block when replaying history to the model. # Mirrors the active variant, same as text/reasoning/context_snapshot. world_delta: Mapped[dict | None] = mapped_column(JSON, nullable=True) - # Copy of Adventure.script_state as it was immediately BEFORE this action's - # script hooks ran, so undo/retry can roll the shared scoreboard back. - # NULL for actions created before this column existed. Deferred: only ever - # read for the one action being undone or retried. + # LEGACY (SP4): Adventure.script_state / world_state as they were + # immediately BEFORE this action's script hooks ran. Unwritten since SP4 + # and read by nothing — the *after* pair below replaced them, because a + # sibling attempt needs its own outcome and a "before" picture is shared by + # every attempt at the turn. Kept for one release so a rolled-back build + # still finds a real snapshot on every row it wrote itself; SP8 drops them + # beside `index` and `variants`. state_before: Mapped[dict | None] = mapped_column( JSON, nullable=True, deferred=True ) - # Phase 12: same idea for the RPG world_state, so undo/retry rolls it back too. world_state_before: Mapped[dict | None] = mapped_column( JSON, nullable=True, deferred=True ) - # Retry history (AI actions): every attempt made for this turn, oldest - # first, INCLUDING the active one. NULL/empty means never retried — the row - # is its own only version. `variant_index` says which entry `text`, - # `reasoning` and `context_snapshot` currently mirror; retry appends and - # points here instead of deleting the row, so nothing is lost. + # Phase 14, SP4: the shared script scoreboard and the RPG world state as + # they stood once this node had been played — *its* outcome, not its + # starting position. # - # Deferred for the same reason as context_snapshot: a list response only - # ever needs the *count* (see variant_count below), but the column holds - # every discarded attempt's full narration, so loading it in bulk made each - # retry a permanent tax on every later page load of that adventure. + # Two things want this and neither can use a "before" picture. Switching + # between siblings has to put back the state the chosen attempt produced, + # and the attempts differ precisely in what they produced. And rolling back + # to before a turn is "the state the node in front of it left behind", + # which is one lookup on the path rather than a snapshot that has to be + # taken from inside the turn being rolled back. + # + # NULL on rows written before SP4 that the migration could not derive one + # for, and tolerated everywhere: a missing snapshot means "leave the live + # state alone", never "reset it". + # + # Deferred: only ever read for the one node being switched to, undone or + # retried past. + state_after: Mapped[dict | None] = mapped_column( + JSON, nullable=True, deferred=True + ) + world_state_after: Mapped[dict | None] = mapped_column( + JSON, nullable=True, deferred=True + ) + # LEGACY (SP4): retry history as a JSON repeating group. Every attempt at + # this turn is its own row now — see `live` above and `app/attempts.py` — + # so nothing reads this. Kept until SP8 for the same reason `index` is, and + # read exactly once more on the way out: migration 60 is what turns each + # entry into the sibling row it should always have been. variants: Mapped[list | None] = mapped_column(JSON, nullable=True, deferred=True) - # len(variants), maintained on write by set_variants() so the deferred - # column above never has to be fetched just to count it. 0 = never retried. + # Where the row sits in its sibling group: `variant_index` is this + # attempt's ordinal, oldest first, and `variant_count` is how many attempts + # the group holds (0, not 1, when the turn was never retried — the pager + # reads that as "nothing to page through"). + # + # A cache of two facts about the group, maintained in one place + # (`attempts.renumber`) for the same reason it used to be a cache of + # `len(variants)`: a page response wants them for every row and must not + # pay a query per turn to get them. SP7 replaces the pager with the branch + # view and SP8 drops both columns. variant_count: Mapped[int] = mapped_column(Integer, default=0) variant_index: Mapped[int] = mapped_column(Integer, default=0) created_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow) diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index 8a8a642..8b6aa0f 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -1,4 +1,3 @@ -import copy import json import re import threading @@ -6,10 +5,12 @@ import threading from fastapi import APIRouter, Body, Depends, HTTPException, Request from fastapi.responses import StreamingResponse from sqlalchemy import func -from sqlalchemy.orm import Session, load_only, undefer +from sqlalchemy.orm import Session, load_only from sqlalchemy.orm.attributes import set_committed_value -from .. import auth, images, limits, memorybank, models, schemas, tree, worldstate +from .. import ( + attempts, auth, images, limits, memorybank, models, schemas, tree, worldstate, +) from ..context import build_context, cursors from ..context import history as context_history from ..context import lineage @@ -387,6 +388,10 @@ def create_adventure( type="start", text=fill_placeholders(scenario.prompt, values), ) + # The opening node leaves behind the state the adventure starts + # with, so undoing or retrying the first turn has somewhere to + # roll back to. + attempts.snapshot_outcome(adventure, opening) tree.place_action(db, adventure, opening) db.add(opening) @@ -468,32 +473,12 @@ def override_world_state( return {"state": new_state, "report": report} -def snapshot_state(adventure: models.Adventure) -> dict: - """Deep copy of the shared script_state, to staple onto an action so undo/ - retry can restore it. Independent of later hook mutations.""" - state = adventure.script_state if isinstance(adventure.script_state, dict) else {} - return copy.deepcopy(state) - - -def snapshot_world_state(adventure: models.Adventure) -> dict: - """Deep copy of the RPG world_state, for the same undo/retry rollback as - snapshot_state (Phase 12).""" - state = adventure.world_state if isinstance(adventure.world_state, dict) else {} - return copy.deepcopy(state) - - -# ---------- Retry history (variants) ---------- +# ---------- Retry history (sibling attempts) ---------- # -# Retry used to delete the AI action and generate a replacement. Now the row -# survives and every attempt is appended to `Action.variants`, with -# `variant_index` naming the live one. A variant carries only the parts that -# actually differ between attempts — the narration and the state it produced — -# never the assembled prompt, which is identical across attempts of one turn -# and is by far the biggest thing in `context_snapshot`. - -# The per-attempt slices of context_snapshot. Everything else in the snapshot -# (system/story/memories) is shared by every attempt at the same turn. -VARIANT_SNAPSHOT_KEYS = ("world_state", "script", "raw_output") +# Retry used to delete the AI action and generate a replacement, then kept the +# row and pushed each attempt into a JSON list on it. Now every attempt is its +# own node: same branch, same depth, one of them `live`. `app/attempts.py` owns +# the group and both of its invariants; the endpoints below only ask it things. def world_delta_of(snapshot: dict | None) -> dict | None: @@ -512,62 +497,6 @@ def world_delta_of(snapshot: dict | None) -> dict | None: } -def set_variants(action: models.Action, entries: list[dict]) -> None: - """The ONLY way to write Action.variants. - - `variants` is deferred (it holds every discarded attempt's narration), so - `variant_count` exists to answer "how many attempts?" without fetching it. - Writing the list anywhere else would let the two drift and the pager would - lie about how many takes a turn has. - """ - action.variants = entries - action.variant_count = len(entries) - - -def variant_of(action: models.Action, adventure: models.Adventure) -> dict: - """Freeze an action's *current* content as a variant entry. - - `adventure` supplies the resulting script/world state, so this must be - called before any rollback — those live values are this attempt's outcome. - """ - snapshot = action.context_snapshot if isinstance(action.context_snapshot, dict) else {} - entry = { - "text": action.text, - "reasoning": action.reasoning, - "script_state": snapshot_state(adventure), - "created_at": action.created_at.isoformat() if action.created_at else None, - } - for key in VARIANT_SNAPSHOT_KEYS: - if key in snapshot: - entry[key] = copy.deepcopy(snapshot[key]) - return entry - - -def apply_variant(action: models.Action, adventure: models.Adventure, index: int) -> None: - """Make variant `index` the live one: its text onto the action, its - outcome back onto the adventure.""" - entry = action.variants[index] - action.text = entry.get("text", "") - action.reasoning = entry.get("reasoning") - snapshot = dict(action.context_snapshot) if isinstance(action.context_snapshot, dict) else {} - for key in VARIANT_SNAPSHOT_KEYS: - if key in entry: - snapshot[key] = copy.deepcopy(entry[key]) - else: - snapshot.pop(key, None) - action.context_snapshot = snapshot - action.world_delta = world_delta_of(snapshot) - action.variant_index = index - if isinstance(entry.get("script_state"), dict): - adventure.script_state = copy.deepcopy(entry["script_state"]) - # The world state this attempt left behind lives inside its own snapshot - # slice; absent for adventures with no RPG layer, where there's nothing to - # restore anyway. - world_state = (entry.get("world_state") or {}).get("state") - if isinstance(world_state, dict): - adventure.world_state = copy.deepcopy(world_state) - - @router.patch("/{adventure_id}", response_model=schemas.AdventureOut) def update_adventure( adventure_id: int, @@ -684,12 +613,12 @@ async def generate_turn( """SSE generator: streams the AI continuation through the context/output script hooks, then stores the result. - With `retry_of`, the result is appended as a new variant of that existing - AI action rather than stored as a new one — the discarded attempt stays - readable. The caller must have seeded `retry_of.variants` and rolled the - adventure back first (see `retry_action`); if this generator ends without - saving, that rollback is undone so state can't drift from the text still - on screen.""" + With `retry_of`, the result is stored as a *sibling* of that AI action — + same turn, same coordinate, another take — and the discarded attempt stays + exactly where it was written. The caller must have rolled the adventure + back to before the turn first (see `retry_action`); if this generator ends + without saving, that rollback is undone so state can't drift from the text + still on screen.""" saved = False try: async for event in _generate_turn(adventure, db, pipeline, user, retry_of): @@ -700,8 +629,9 @@ async def generate_turn( finally: if retry_of is not None and not saved: # Provider error, empty reply, a script stop, or the client hanging - # up: put the attempt we rolled away from back in charge. - apply_variant(retry_of, adventure, retry_of.variant_index) + # up: no sibling was written, so the attempt on screen is still the + # live one — put back the state it produced. + attempts.restore_state(adventure, retry_of) db.commit() @@ -719,10 +649,10 @@ async def _generate_turn( ): settings = get_settings(db, user) cfg = auth.resolve_provider_config(settings) - # On a retry the row being regenerated is still attached to the adventure - # (it carries the variant history), so it has to be filtered out of the - # context — otherwise the model is shown the attempt it is replacing as if - # it were established story, and writes a continuation of it. + # On a retry the attempt being replaced is still the live node of its turn + # — it stays live until a replacement exists to take over — so it has to be + # filtered out of the context, or the model is shown the attempt it is + # supposed to be replacing as established story and writes a sequel to it. replacing_id = retry_of.id if retry_of is not None else None if cfg.using_demo: # No embedding/summarization calls on the server-funded key: memory @@ -736,10 +666,6 @@ async def _generate_turn( memories = await memorybank.retrieve_memories( adventure, settings, update_stats=True, exclude_action_id=replacing_id ) - # Scoreboard as it stands before this AI turn's context/output hooks mutate - # it — stapled onto the AI action so retry can start over from here. - state_before = snapshot_state(adventure) - world_state_before = snapshot_world_state(adventure) system_text, story_text, snapshot = build_context( adventure, settings, memories, exclude_action_id=replacing_id ) @@ -809,9 +735,15 @@ async def _generate_turn( # RPG world state (Phase 12): pull the AI's state delta out of the reply, # let the engine referee it, and strip the block from the shown text. - # A retry re-runs the *same* turn, so it keeps that turn's index — using - # next_index here would advance the clock the cooldown rules run on. - ai_index = retry_of.index if retry_of is not None else next_index(adventure) + # A retry re-runs the *same* turn, so it is played at that turn's depth — + # the clock the cooldown rules run on is a position in the story, and a + # second take on turn 12 is still turn 12. (It was `retry_of.index` until + # SP4, which held the same number; depth is the one that stays true once a + # branch has its own numbering.) + # `next_index` because `tree.place_action` still derives a new node's depth + # from its legacy index while the two columns coexist; they hold the same + # number, and SP8 removes the question. + ai_depth = retry_of.depth if retry_of is not None else next_index(adventure) stat_schema = adventure.scenario.stat_schema if adventure.scenario else None if worldstate.has_schema(stat_schema): text, delta = worldstate.extract_delta(text) @@ -819,7 +751,7 @@ async def _generate_turn( yield sse({"type": "error", "detail": "The AI returned only a state update and no story text."}) return new_world_state, ws_report = worldstate.apply_delta( - adventure.world_state, stat_schema, delta, ai_index + adventure.world_state, stat_schema, delta, ai_depth ) adventure.world_state = new_world_state snapshot["world_state"] = {"delta": delta, "report": ws_report, "state": new_world_state} @@ -827,33 +759,33 @@ async def _generate_turn( snapshot["raw_output"] = raw_output reasoning = "".join(reasoning_chunks).strip() or None + ai_action = models.Action( + adventure_id=adventure.id, + # A sibling shares the turn's legacy index for the same reason it + # shares its depth: it is the same turn. Two rows then hold one index, + # which `max_action_index` (a maximum, not a count) survives, and + # nothing else still reads the column. + index=retry_of.index if retry_of is not None else ai_depth, + type="ai", + text=text, + reasoning=reasoning, + context_snapshot=snapshot, + world_delta=world_delta_of(snapshot), + ) + attempts.snapshot_outcome(adventure, ai_action) if retry_of is not None: - # Same row, one more attempt. Writing text/reasoning/snapshot through - # the variant list keeps the row and its history in step. - ai_action = retry_of - ai_action.context_snapshot = snapshot - history = list(ai_action.variants or []) - history.append({ - "text": text, - "reasoning": reasoning, - "script_state": snapshot_state(adventure), - "created_at": models.utcnow().isoformat(), - **{k: copy.deepcopy(snapshot[k]) for k in VARIANT_SNAPSHOT_KEYS if k in snapshot}, - }) - set_variants(ai_action, history) - apply_variant(ai_action, adventure, len(history) - 1) + attempts.add_attempt(db, adventure, retry_of, ai_action) + db.add(ai_action) + # The text at this coordinate has just changed, so whatever was derived + # from it is no longer about the story: withdraw the memory hanging off + # the node and hand the ground back to both passes. Before SP4 this was + # unreachable, because the summarizer held the newest action back until + # a turn had landed on top of it — see `memorybank`. + memorybank.forget_node(db, adventure, retry_of) + cursors.rewind_all(adventure, retry_of.branch_id, ai_depth - 1) + db.flush() + attempts.renumber(attempts.group(db, ai_action)) else: - ai_action = models.Action( - adventure_id=adventure.id, - index=ai_index, - type="ai", - text=text, - reasoning=reasoning, - context_snapshot=snapshot, - world_delta=world_delta_of(snapshot), - 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() @@ -890,10 +822,6 @@ async def run_player_turn( # An empty do/say/story is just a continue. if payload.type != "continue" and payload.text.strip(): - # Scoreboard before the input hook mutates it — the pre-turn state that - # undo restores to (the AI action keeps its own post-input snapshot). - state_before = snapshot_state(adventure) - world_state_before = snapshot_world_state(adventure) # onInput sees the formatted text (as in AI Dungeon: "> You ..."). formatted = format_player_input(payload.type, payload.text) modified, stop = pipeline.run("input", formatted) @@ -906,9 +834,11 @@ async def run_player_turn( index=next_index(adventure), type=payload.type, text=modified, - state_before=state_before, - world_state_before=world_state_before, ) + # The scoreboard once the input hook has run — what this node left + # behind, which is where the AI turn after it starts and where a retry + # of that turn rolls back to. + attempts.snapshot_outcome(adventure, player_action) tree.place_action(db, adventure, player_action) db.add(player_action) db.commit() @@ -956,9 +886,10 @@ def retry_action( ): """Regenerate the last AI action, keeping the discarded attempt. - The row survives: its current content is frozen as a variant, the shared - script/world state rolls back to the pre-turn snapshot, and the new attempt - is appended as the next variant. Nothing the AI wrote is ever thrown away. + The attempt on screen is left exactly as it was written; the shared + script/world state rolls back to what the node in front of it left behind, + and the new take is stored as a sibling at the same coordinate. Nothing the + AI wrote is ever rewritten, let alone thrown away. """ adventure = get_adventure_or_404(adventure_id, db, user) limits.rate_limit("turn", request, user) @@ -969,19 +900,13 @@ def retry_action( newest = last_action(adventure, db) if newest is not None and newest.type == "ai": last_ai = newest - # First retry: the row has no history yet, so record what's on - # screen as variant 0 before anything is rolled back — the live - # script/world state is precisely that attempt's outcome. - if not last_ai.variant_count: - set_variants(last_ai, [variant_of(last_ai, adventure)]) - last_ai.variant_index = 0 # Roll the scoreboard back to before this AI turn's hooks ran, so # regenerating starts fresh instead of stacking output mutations on - # top of the discarded attempt. NULL for pre-migration actions. - if last_ai.state_before is not None: - adventure.script_state = copy.deepcopy(last_ai.state_before) - if last_ai.world_state_before is not None: - adventure.world_state = copy.deepcopy(last_ai.world_state_before) + # top of the attempt being replaced. A no-op where the preceding + # node has no snapshot (a row written before SP4 the migration + # could not derive one for), which leaves the state alone rather + # than resetting it. + attempts.roll_back_before(db, adventure, last_ai) db.commit() db.refresh(adventure) except BaseException: @@ -1011,21 +936,28 @@ def list_variants( ): """Every attempt made for one AI turn. Fetched on demand — the adventure payload carries only the counts, so old narration doesn't ride along on - every page load.""" + every page load. + + Addressed by *any* attempt at the turn, not only the live one: switching + changes which row the story tells, and a client holding the id it was given + a moment ago must still be able to ask about the same turn. + """ get_adventure_or_404(adventure_id, db, user) action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") - variants = action.variants if isinstance(action.variants, list) else [] + rows = attempts.group(db, action) + if len(rows) < 2: + return [] # never retried: the turn is its own only take return [ schemas.VariantOut( index=i, - text=entry.get("text", ""), - reasoning=entry.get("reasoning"), - created_at=entry.get("created_at"), - active=(i == action.variant_index), + text=row.text, + reasoning=row.reasoning, + created_at=row.created_at.isoformat() if row.created_at else None, + active=row.live, ) - for i, entry in enumerate(variants) + for i, row in enumerate(rows) ] @@ -1051,11 +983,11 @@ def select_variant( action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") - variants = action.variants if isinstance(action.variants, list) else [] - if not 0 <= payload.index < len(variants): + rows = attempts.group(db, action) + if not 0 <= payload.index < len(rows) or len(rows) < 2: raise HTTPException(400, "No such attempt for this action") newest = last_action(adventure, db) - if newest is None or newest.id != action.id: + if newest is None or newest.depth != action.depth or newest.branch_id != action.branch_id: raise HTTPException( 400, "Only the latest message can be switched — the story has already " @@ -1063,15 +995,42 @@ def select_variant( ) acquire_turn_lock(adventure_id) try: - apply_variant(action, adventure, payload.index) + chosen = rows[payload.index] + if not chosen.live: + # The story at this coordinate is about to say something else, so + # anything derived from what it used to say is withdrawn — the same + # move a retry makes, for the same reason. + memorybank.forget_node(db, adventure, chosen) + cursors.rewind_all(adventure, chosen.branch_id, (chosen.depth or 0) - 1) + attempts.make_live(db, adventure, chosen) adventure.updated_at = models.utcnow() db.commit() - db.refresh(action) - return action + db.refresh(chosen) + # The row that answers is the one now in the story, which is a + # *different row* from the one addressed — that is the whole change: an + # attempt is a node, so choosing one moves the story onto it rather + # than rewriting anything. + return chosen finally: _active_turns.discard(adventure_id) +def delete_turn( + db: Session, adventure: models.Adventure, node: models.Action +) -> None: + """Remove a turn: every attempt at it, not only the one on screen. + + A discarded attempt is a leaf hanging off the same coordinate, and it is + only reachable *through* that coordinate — leaving it behind when the turn + goes would leave a row nothing can name and no read can see. Whatever the + turn produced is withdrawn once, because a memory hangs off the coordinate + rather than off one of its attempts. + """ + memorybank.forget_node(db, adventure, node) + for attempt in attempts.group(db, node): + db.delete(attempt) + + @router.post("/{adventure_id}/undo", response_model=schemas.ActionPage) def undo_turn( adventure_id: int, db: Session = Depends(get_db), user: models.User = CurrentUser @@ -1099,19 +1058,18 @@ def undo_turn( if not newest or newest[0].type == "start": raise HTTPException(400, "Nothing to undo") last = newest[0] - preceding = newest[1] if len(newest) > 1 else None - # The earliest action removed in this turn holds the pre-turn scoreboard. + before_that = newest[1] if len(newest) > 1 else None first_removed = last - memorybank.forget_node(db, adventure, last) - db.delete(last) - if last.type == "ai" and preceding is not None and preceding.type in ("do", "say", "story"): - first_removed = preceding - memorybank.forget_node(db, adventure, first_removed) - db.delete(first_removed) - if first_removed.state_before is not None: - adventure.script_state = copy.deepcopy(first_removed.state_before) - if first_removed.world_state_before is not None: - adventure.world_state = copy.deepcopy(first_removed.world_state_before) + if last.type == "ai" and before_that is not None and before_that.type in ("do", "say", "story"): + first_removed = before_that + # Where the story stands once the turn is gone: what the node in front + # of the earliest removed one left behind. Read before the deletes, so + # the question is asked of a story that still has them in it. + restore_to = attempts.preceding(db, adventure, first_removed) + delete_turn(db, adventure, last) + if first_removed is not last: + delete_turn(db, adventure, first_removed) + attempts.restore_state(adventure, restore_to) db.flush() # apply the deletes before anything reads the story back db.expire(adventure, ["actions"]) # The tip moved back with them. @@ -1140,22 +1098,28 @@ def export_adventure( ): """Full backup: plot components, story cards, scripts (+state), every action.""" adv = get_adventure_or_404(adventure_id, db, user) - # Export is the one read that genuinely wants every attempt, so it asks for - # the deferred `variants` column up front — iterating adv.actions instead - # would lazy-load it one row at a time. - # - # And the one read deliberately left un-pathed: a backup wants the whole + # The one read deliberately left un-pathed: a backup wants the whole # adventure, not the branch its owner happens to be standing on. `index` # orders it because the v1 bundle is a flat list keyed on index and its # reader has no idea branches exist — which is exactly why SP6 replaces the # format rather than quietly widening this query. + # + # Attempts at one turn share that index (SP4), so the flat list is built by + # folding each group back into the `variants` array the format expects. + # That array is the *only* remaining producer of the v1 shape: nothing in + # the database holds one any more. exported_actions = ( db.query(models.Action) .filter(models.Action.adventure_id == adv.id) - .options(undefer(models.Action.variants)) - .order_by(models.Action.index) + .order_by(models.Action.index, models.Action.variant_index, models.Action.id) .all() ) + turns: list[list[models.Action]] = [] + for action in exported_actions: + if turns and turns[-1][0].index == action.index: + turns[-1].append(action) + else: + turns.append([action]) return { "format": "ai-dnd-adventure-v1", "title": adv.title, @@ -1194,26 +1158,63 @@ def export_adventure( } for s in adv.scripts ], - "actions": [ - { - "index": a.index, "type": a.type, "text": a.text, - "reasoning": a.reasoning, - # Retry history, narration only — a bundle carries no context - # snapshots, so the per-attempt script/world state it would - # restore isn't there to export either. - "variants": [ - {"text": v.get("text", ""), "reasoning": v.get("reasoning"), - "createdAt": v.get("created_at")} - for v in (a.variants or []) - ] or None, - "variantIndex": a.variant_index, - "createdAt": a.created_at.isoformat(), - } - for a in exported_actions - ], + "actions": [_exported_turn(group) for group in turns], } +def _exported_turn(group: list[models.Action]) -> dict: + """One turn as a v1 bundle entry: the attempt in the story, plus the rest. + + Narration only — a bundle carries no context snapshots, so the per-attempt + script/world state a switch would restore isn't there to export either. + """ + live = next((a for a in group if a.live), group[0]) + return { + "index": live.index, "type": live.type, "text": live.text, + "reasoning": live.reasoning, + "variants": [ + {"text": a.text, "reasoning": a.reasoning, + "createdAt": a.created_at.isoformat() if a.created_at else None} + for a in group + ] if len(group) > 1 else None, + "variantIndex": group.index(live), + "createdAt": live.created_at.isoformat(), + } + + +def _imported_turn( + adventure: models.Adventure, entry: dict, index: int +) -> list[models.Action]: + """A v1 bundle entry as the nodes it describes: one per attempt. + + A bundle's `variants` array is the repeating group SP4 unpacked, so + importing one is the same split the migration does — every attempt gets a + row at the turn's coordinate, and `variantIndex` picks which is live. + Clamped, because a hand-edited bundle can name an attempt its own list + doesn't have, and a turn with no live node is a turn no read can see. + """ + kind = str(entry.get("type") or "story")[:20] # VARCHAR(20) + variants = [v for v in (entry.get("variants") or []) if isinstance(v, dict)] + if not variants: + variants = [{"text": entry["text"], "reasoning": entry.get("reasoning")}] + live = min(max(int(entry.get("variantIndex", 0)), 0), len(variants) - 1) + rows = [] + for i, variant in enumerate(variants): + text = str(variant.get("text") or "") + reasoning = variant.get("reasoning") + rows.append(models.Action( + adventure_id=adventure.id, + index=index, + type=kind, + text=text, + reasoning=str(reasoning) if reasoning else None, + live=(i == live), + variant_index=i, + variant_count=len(variants) if len(variants) > 1 else 0, + )) + return rows + + @router.post("/import", response_model=schemas.AdventureOut, status_code=201) def import_adventure( request: Request, @@ -1294,26 +1295,9 @@ def import_adventure( for i, a in enumerate(bundle.get("actions") or []): if isinstance(a, dict) and str(a.get("text") or ""): - variants = [ - {"text": str(v.get("text") or ""), "reasoning": v.get("reasoning"), - "created_at": v.get("createdAt")} - for v in (a.get("variants") or []) - if isinstance(v, dict) - ] - action = models.Action( - adventure_id=adventure.id, - index=int(a.get("index", i)), - type=str(a.get("type") or "story")[:20], # VARCHAR(20) - text=str(a["text"]), - reasoning=str(a["reasoning"]) if a.get("reasoning") else None, - variants=variants or None, - variant_count=len(variants), - # 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) + for action in _imported_turn(adventure, a, int(a.get("index", i))): + tree.place_action(db, adventure, action) + db.add(action) # The bundle's cursors are positions in a flat story and the marks are # nodes, so the translation waits until the actions exist — this is the @@ -1782,15 +1766,10 @@ def update_action( action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") + # One row, one text. Nothing mirrors it any more, so nothing has to be kept + # in step — the edit used to have to be written into the live variant entry + # as well, or paging away and back silently reverted it. action.text = payload.text - # Keep the live variant in step, or paging away and back would silently - # revert the edit. - if action.variant_count: - variants = action.variants if isinstance(action.variants, list) else [] - if 0 <= action.variant_index < len(variants): - history = copy.deepcopy(variants) - history[action.variant_index]["text"] = payload.text - set_variants(action, history) db.commit() return action @@ -1806,11 +1785,10 @@ def delete_action( action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") - # Same as undo: withdraw whatever this node produced. Nothing else needs - # doing — the marks are depths, and a depth does not move because an action - # in front of it went away. - memorybank.forget_node(db, adventure, action) - db.delete(action) + # Same as undo: the turn goes, attempts and all, and whatever it produced + # is withdrawn. Nothing else needs doing — the marks are depths, and a + # depth does not move because an action in front of it went away. + delete_turn(db, adventure, action) db.flush() db.expire(adventure, ["actions"]) # Deleting the newest action moves the tip; deleting a middle one leaves a diff --git a/backend/app/tree.py b/backend/app/tree.py index 11584cc..aa0f370 100644 --- a/backend/app/tree.py +++ b/backend/app/tree.py @@ -22,6 +22,8 @@ script and test ever written too, and its breach is a story quietly missing turns. So the invariant is enforced at the flush instead of asked for. """ +import copy + from sqlalchemy import func, insert, update from sqlalchemy.orm import Session @@ -149,6 +151,27 @@ def attach_memory(memory: models.Memory, node: models.Action) -> None: memory.depth = node.depth +def stamp_outcome(adventure: models.Adventure, action: models.Action) -> None: + """Give a node the state it left behind, if its writer did not. + + The floor under `attempts.snapshot_outcome`, and it is here for the same + reason `place_action` has one: from SP4 a node with no outcome is a node + undo and retry cannot roll back past, and it fails by leaving the + scoreboard where it was rather than by raising. The turn engine records the + outcome itself and this skips those rows; what it catches is every fixture, + script and import that writes a story straight through the ORM. + + What it writes is the truth as of the flush: a writer that changes no state + between two nodes leaves the same state behind both of them. + """ + if action.state_after is None: + state = adventure.script_state if isinstance(adventure.script_state, dict) else {} + action.state_after = copy.deepcopy(state) + if action.world_state_after is None: + world = adventure.world_state if isinstance(adventure.world_state, dict) else {} + action.world_state_after = copy.deepcopy(world) + + def place_new_nodes(session: Session) -> None: """Place every unplaced node about to be inserted. Runs on every flush. @@ -178,11 +201,15 @@ def place_new_nodes(session: Session) -> None: place = place_memory else: continue - if obj.branch_id is not None or obj.adventure_id is None: + if obj.adventure_id is None: continue adventure = session.get(models.Adventure, obj.adventure_id) if adventure is None: continue + if isinstance(obj, models.Action): + stamp_outcome(adventure, obj) + if obj.branch_id is not None: + continue # already placed at its call site head = heads.get(adventure.id) if head is None: head = heads[adventure.id] = head_branch(session, adventure) diff --git a/backend/tests/test_attempt_siblings.py b/backend/tests/test_attempt_siblings.py new file mode 100644 index 0000000..4df9586 --- /dev/null +++ b/backend/tests/test_attempt_siblings.py @@ -0,0 +1,399 @@ +"""Phase 14 SP4 — a retry writes a sibling node instead of rewriting a row. + +`test_retry_variants.py` is the behavioural contract, unchanged since before +the tree, and it still passes: the same URLs, the same payload shape, the same +outcomes. This file asserts the things that are *only* true of the new storage +— that a turn can be several rows, that exactly one of them is the story, and +that the arrangement costs neither an extra prompt nor an extra turn. + + python -m pytest tests/test_attempt_siblings.py -v +""" +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.orm import undefer + +from app import attempts, auth, limits, models, tree +from app.context import cursors, history +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app.providers import PromptParts +from app.routers import adventures + +SCHEMA = {"player": {"hp": {"min": 0, "max": 100, "initial": 100}}} + +GOLD_SCRIPT = """ +const modifier = (text) => { + state.gold = (state.gold || 0) + 10; + return { text }; +}; +modifier(text); +""" + + +class ScriptedProvider: + replies: list = [] + calls = 0 + prompts: list = [] + + def __init__(self, *a, **k): + pass + + async def generate(self, parts: PromptParts, *, temperature, max_tokens): + index = min(ScriptedProvider.calls, len(ScriptedProvider.replies) - 1) + ScriptedProvider.calls += 1 + ScriptedProvider.prompts.append((parts.system, parts.story)) + yield ("text", ScriptedProvider.replies[index]) + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + setup = SessionLocal() + user = models.User(is_guest=False, email="siblings@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")) + scenario = models.Scenario(user_id=user.id, title="S", stat_schema=SCHEMA) + setup.add(scenario) + setup.flush() + adv = models.Adventure( + user_id=user.id, title="Cave", scenario_id=scenario.id, + script_state={}, world_state={"player": {"hp": 100}}, + ) + setup.add(adv) + setup.flush() + setup.add(models.Action(adventure_id=adv.id, index=0, type="start", text="You enter a cave.")) + setup.add(models.AdventureScript( + adventure_id=adv.id, position=0, enabled=True, name="Gold", output_js=GOLD_SCRIPT, + )) + setup.commit() + adv_id, user_id = adv.id, user.id + setup.close() + + ScriptedProvider.replies = ["Attempt one."] + ScriptedProvider.calls = 0 + ScriptedProvider.prompts = [] + monkeypatch.setattr(adventures, "OpenAICompatibleProvider", ScriptedProvider) + monkeypatch.setattr(auth, "resolve_provider_config", lambda s: auth.ProviderConfig( + "http://fake", "k", "test-model", False)) + 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.adv_id = adv_id + try: + yield c + finally: + app.dependency_overrides.clear() + adventures._active_turns.clear() + Base.metadata.drop_all(bind=engine) + + +def _play(client, text="look around", type="do"): + r = client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": type, "text": text}) + assert r.status_code == 200, r.text + + +def _retry(client): + r = client.post(f"/api/adventures/{client.adv_id}/retry") + assert r.status_code == 200, r.text + + +def _page(client) -> dict: + return client.get(f"/api/adventures/{client.adv_id}").json() + + +def _rows(adv_id) -> list[models.Action]: + """Every action row of the adventure, story or not, live or not. + + Undeferred, because the session is closed before the caller looks: the + columns this file is about are exactly the ones a page load never loads. + """ + db = SessionLocal() + try: + return ( + db.query(models.Action) + .filter(models.Action.adventure_id == adv_id) + .options( + undefer(models.Action.state_after), + undefer(models.Action.world_state_after), + undefer(models.Action.context_snapshot), + ) + .order_by(models.Action.depth, models.Action.variant_index) + .all() + ) + finally: + db.close() + + +# ------------------------------------------------------------- the sibling + +def test_a_retry_writes_a_second_row_at_the_same_coordinate(client): + ScriptedProvider.replies = ["Attempt one.", "Attempt two."] + _play(client) + _retry(client) + + rows = _rows(client.adv_id) + ai = [a for a in rows if a.type == "ai"] + assert len(ai) == 2, "a retry is a node, not a rewrite" + assert {(a.branch_id, a.depth) for a in ai} == {(ai[0].branch_id, ai[0].depth)} + assert [a.text for a in ai] == ["Attempt one.", "Attempt two."] + # Exactly one of them is the story, and it is the newer take. + assert [a.live for a in ai] == [False, True] + # ...and the discarded attempt is untouched, not a copy of anything. + assert ai[0].state_after is not None + + +def test_the_story_shows_and_counts_the_turn_once(client): + ScriptedProvider.replies = ["Attempt one.", "Attempt two."] + _play(client) + before = _page(client)["action_count"] + _retry(client) + after = _page(client) + + assert after["action_count"] == before, "a discarded attempt is not a turn" + assert [a["type"] for a in after["actions"]] == ["start", "do", "ai"] + assert after["actions"][-1]["text"] == "Attempt two." + + +def test_a_discarded_attempt_never_reaches_the_prompt(client): + """The trap the branch clause exists to close, at sibling scale: the losing + attempt sits at the same branch and depth as the live one, so anything + reading the story by coordinate alone would replay both.""" + ScriptedProvider.replies = ["Attempt one.", "Attempt two.", "Next turn."] + _play(client) + _retry(client) + _play(client, "go deeper") + + story = ScriptedProvider.prompts[-1][1] + assert "Attempt two." in story + assert "Attempt one." not in story + + +def test_switching_moves_the_story_onto_the_other_row(client): + ScriptedProvider.replies = [ + "A scratch.\n```state\n{\"player.hp\": -5}\n```", + "A beating.\n```state\n{\"player.hp\": -40}\n```", + ] + _play(client) + _retry(client) + newest_id = _page(client)["actions"][-1]["id"] + + r = client.post( + f"/api/adventures/{client.adv_id}/actions/{newest_id}/variant", json={"index": 0}) + assert r.status_code == 200, r.text + # A different row answers — that is the whole change. + assert r.json()["id"] != newest_id + assert r.json()["text"].startswith("A scratch") + + rows = _rows(client.adv_id) + ai = [a for a in rows if a.type == "ai"] + assert [a.live for a in ai] == [True, False] + # Both takes are still there, byte for byte. + assert [a.text.split(".")[0] for a in ai] == ["A scratch", "A beating"] + + +def test_the_assembled_prompt_is_stored_once_per_turn(client): + """A snapshot is ~160 kB of prompt every attempt at a turn shares. Giving + each sibling a copy would have made retry a permanent multiplier on the + biggest column in the database, so the prompt moves with the live flag.""" + ScriptedProvider.replies = ["Attempt one.", "Attempt two."] + _play(client) + _retry(client) + + def holders(): + return [ + a.id for a in _rows(client.adv_id) + if a.type == "ai" and "sections" in (a.context_snapshot or {}) + ] + + live_holder = holders() + assert len(live_holder) == 1 + newest = _page(client)["actions"][-1] + assert live_holder == [newest["id"]] + + client.post(f"/api/adventures/{client.adv_id}/actions/{newest['id']}/variant", + json={"index": 0}) + moved = holders() + assert len(moved) == 1 and moved != live_holder, "the prompt follows the story" + + +# ------------------------------------------------------- removing the turn + +def test_undo_takes_every_attempt_with_it(client): + ScriptedProvider.replies = ["One.", "Two.", "Three."] + _play(client) + _retry(client) + _retry(client) + assert len([a for a in _rows(client.adv_id) if a.type == "ai"]) == 3 + + r = client.post(f"/api/adventures/{client.adv_id}/undo") + assert r.status_code == 200, r.text + assert [a.type for a in _rows(client.adv_id)] == ["start"] + + +def test_deleting_a_retried_turn_deletes_its_attempts(client): + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + _retry(client) + newest = _page(client)["actions"][-1] + + r = client.delete(f"/api/adventures/{client.adv_id}/actions/{newest['id']}") + assert r.status_code == 204, r.text + assert [a.type for a in _rows(client.adv_id)] == ["start", "do"] + + +def test_deleting_a_turn_through_a_discarded_attempt_still_takes_the_turn(client): + """The pager hands out whichever id it last saw, and a switch changes which + row that is. Deleting through the losing sibling must not leave the story + holding a turn with no attempts.""" + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + _retry(client) + discarded = [a for a in _rows(client.adv_id) if a.type == "ai" and not a.live][0] + + r = client.delete(f"/api/adventures/{client.adv_id}/actions/{discarded.id}") + assert r.status_code == 204, r.text + assert [a.type for a in _rows(client.adv_id)] == ["start", "do"] + + +# ------------------------------------------- what the holdback used to cover + +def test_retrying_withdraws_the_memory_the_turn_produced(client): + """Why summarization no longer holds the newest action back. + + A memory covering the newest turn used to be unreachable-by-construction: + the summarizer stopped one action short, because a retry rewrote the row + under a mark that had already moved past it. Now the mark and the memory + both name the node, and replacing what a node says withdraws them — the + same repair undo and delete already made, so the holdback was the only + thing left that a retry needed. + """ + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + newest = history.newest(adventure) + memory = models.Memory( + adventure_id=adventure.id, text="You looked around.", + source_start=1, source_end=newest.depth, + ) + tree.attach_memory(memory, newest) + db.add(memory) + cursors.MEMORY.anchor_at(adventure, newest) + cursors.SUMMARY.anchor_at(adventure, newest) + db.commit() + covered_depth = newest.depth + finally: + db.close() + + _retry(client) + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + assert db.query(models.Memory).count() == 0, "the withdrawn memory is gone" + # ...and the ground it covered is handed back, so the block is summarized + # again from where it began rather than silently skipped. + assert cursors.MEMORY.depth(db, adventure) == 0 + assert cursors.SUMMARY.depth(db, adventure) == 0 + assert covered_depth > 0 + finally: + db.close() + + +def test_a_memory_on_an_earlier_turn_survives_a_retry(client): + """Only the coordinate whose text changed is withdrawn.""" + ScriptedProvider.replies = ["One.", "Two.", "Three."] + _play(client) + _play(client, "go deeper") + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + earlier = history.tail(adventure, 3)[0] + memory = models.Memory( + adventure_id=adventure.id, text="An earlier block.", + source_start=0, source_end=earlier.depth, + ) + tree.attach_memory(memory, earlier) + db.add(memory) + db.commit() + finally: + db.close() + + _retry(client) + + db = SessionLocal() + try: + assert [m.text for m in db.query(models.Memory).all()] == ["An earlier block."] + finally: + db.close() + + +# -------------------------------------------------------------- the group + +def test_the_group_cache_is_renumbered_as_attempts_arrive(client): + ScriptedProvider.replies = ["One.", "Two.", "Three."] + _play(client) + assert _page(client)["actions"][-1]["variant_count"] == 0 # never retried + _retry(client) + _retry(client) + + ai = [a for a in _rows(client.adv_id) if a.type == "ai"] + assert [a.variant_index for a in ai] == [0, 1, 2] + assert {a.variant_count for a in ai} == {3} + + +def test_attempts_module_agrees_with_the_endpoint(client): + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + _retry(client) + newest = _page(client)["actions"][-1] + + listed = client.get( + f"/api/adventures/{client.adv_id}/actions/{newest['id']}/variants").json() + db = SessionLocal() + try: + node = db.get(models.Action, newest["id"]) + group = attempts.group(db, node) + assert [a.text for a in group] == [v["text"] for v in listed] + assert attempts.live_in(group).id == newest["id"] + finally: + db.close() + + +def test_export_folds_the_group_back_into_one_v1_entry(client): + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + _retry(client) + + bundle = client.get(f"/api/adventures/{client.adv_id}/export").json() + ai = [a for a in bundle["actions"] if a["type"] == "ai"] + assert len(ai) == 1, "a v1 bundle carries one entry per turn, not per attempt" + assert [v["text"] for v in ai[0]["variants"]] == ["One.", "Two."] + assert ai[0]["variantIndex"] == 1 + + # ...and importing it splits it back out into the rows it describes. + imported = client.post("/api/adventures/import", json=bundle).json()["id"] + rows = _rows(imported) + ai_rows = [a for a in rows if a.type == "ai"] + assert [(a.text, a.live) for a in ai_rows] == [("One.", False), ("Two.", True)] + assert len({(a.branch_id, a.depth) for a in ai_rows}) == 1 diff --git a/backend/tests/test_egress.py b/backend/tests/test_egress.py index 262a053..7eaf358 100644 --- a/backend/tests/test_egress.py +++ b/backend/tests/test_egress.py @@ -145,12 +145,17 @@ def test_loading_an_adventure_does_not_fetch_context_snapshot(client, sql_log): assert offenders == [], f"context_snapshot was fetched in bulk:\n{offenders[0][:400]}" -def test_state_before_and_world_state_before_are_not_fetched_in_bulk(client, sql_log): - """Both are rollback snapshots, only ever needed for the single action - being undone or retried.""" +def test_the_state_snapshots_are_not_fetched_in_bulk(client, sql_log): + """All four are rollback snapshots, only ever needed for the single node + being undone, retried past or switched to. + + The `_after` pair is the live one since SP4 and the `_before` pair is dead + weight until SP8 drops it — a page load must pay for neither. + """ client.get(f"/api/adventures/{client.adv_id}") selects = action_selects(sql_log) - for column in ("state_before", "world_state_before"): + for column in ("state_before", "world_state_before", + "state_after", "world_state_after"): offenders = [s for s in selects if column in s] assert offenders == [], f"{column} was fetched in bulk" @@ -199,7 +204,8 @@ def test_counting_actions_does_not_name_the_deferred_columns(client, sql_log): assert history.count(adventure) == 12 counts = [s for s in sql_log if "count" in s.lower()] assert counts, "expected a COUNT to be emitted" - for column in ("context_snapshot", "state_before", "world_state_before", "variants"): + for column in ("context_snapshot", "state_after", "world_state_after", + "state_before", "world_state_before", "variants"): assert not any(column in s for s in counts), ( f"{column} is named by the count query:\n{counts[0][:400]}" ) diff --git a/backend/tests/test_history_window.py b/backend/tests/test_history_window.py index 3428138..d649ea0 100644 --- a/backend/tests/test_history_window.py +++ b/backend/tests/test_history_window.py @@ -203,8 +203,7 @@ def test_helpers_agree_with_the_full_list(story): assert [a.id for a in history.slice_(adventure, 10, 6)] == [a.id for a in actions[10:16]] assert [a.id for a in history.tail_range(adventure, 5, 3)] == \ [a.id for a in actions[-8:-5]] - assert memorybank.settled_count(adventure) == len(actions) - 1 - assert history.newest_settled(adventure).id == actions[-2].id + assert history.newest(adventure).id == actions[-1].id for probe in (0, 1, ACTION_COUNT // 2, ACTION_COUNT - 1): boundary = actions[probe].depth diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py index 1e3ddae..32c658e 100644 --- a/backend/tests/test_memory_nodes.py +++ b/backend/tests/test_memory_nodes.py @@ -294,8 +294,11 @@ def test_the_summary_folds_in_only_the_path_it_is_on(forked, monkeypatch): assert "on B" in prompt and "on C" in prompt assert "on A's own continuation" not in prompt assert "on the shared trunk" not in prompt # behind the mark - # Caught up to the settled end of the story: C7 is retryable, C6 is not. - assert cursors.SUMMARY.stored(adventure) == (ids["c"], 6) + # Caught up to the end of the story. Until SP4 that was C6: the newest + # action was held back because retrying it rewrote the row underneath the + # mark. A retry writes a sibling now, and the withdrawal that follows takes + # the mark back with it, so there is nothing to hold back. + assert cursors.SUMMARY.stored(adventure) == (ids["c"], 7) def test_a_block_is_summarized_from_the_path_and_hung_off_its_last_node( @@ -317,13 +320,16 @@ def test_a_block_is_summarized_from_the_path_and_hung_off_its_last_node( monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub) asyncio.run(memorybank._create_due_memories(adventure, settings, db)) - # Two blocks of four from a path of eight, minus the held-back newest: one. - [excerpt] = stub.excerpts - assert "A5" not in excerpt, "a sibling's narration reached the summarizer" - assert ["A0", "A1", "A2", "A3"] == [line for line in excerpt.split() if line[0] in "ABC"] + # Two blocks of four from a path of eight, and since SP4 nothing is held + # back, so both form in one pass. + first, second = stub.excerpts + assert "A5" not in first + second, "a sibling's narration reached the summarizer" + assert ["A0", "A1", "A2", "A3"] == [line for line in first.split() if line[0] in "ABC"] + assert ["B4", "B5", "C6", "C7"] == [line for line in second.split() if line[0] in "ABC"] made = db.query(models.Memory).filter_by(text="Memory 1.").one() assert (made.branch_id, made.depth) == (ids["a"], 3) - assert cursors.MEMORY.stored(adventure) == (ids["a"], 3) + # The mark ends up on the node the *second* block hangs off — the tip. + assert cursors.MEMORY.stored(adventure) == (ids["c"], 7) # ------------------------------------------------------ the cost of forking diff --git a/backend/tests/test_memory_settling.py b/backend/tests/test_memory_settling.py index fbb4ad9..8fe803d 100644 --- a/backend/tests/test_memory_settling.py +++ b/backend/tests/test_memory_settling.py @@ -1,15 +1,18 @@ -"""Memories must never describe an attempt the player can still retry away, -and must never skip a stretch of story. +"""Memories must never describe narration that is no longer in the story, and +must never skip a stretch of it. -Only the last action is retryable, so summarization holds the newest action -back one turn (memorybank.settled_story_actions). Without that, a memory could -cover the just-generated AI turn; retrying it rewrites Action.text but the mark -has already moved past it, so the memory is never regenerated and goes on -describing narration that is no longer in the story. +For six phases the answer was a **holdback**: summarization stopped one action +short of the newest, because only the last action was retryable and a retry +rewrote `Action.text` under a mark that had already moved past it. SP4 ended +that — a retry writes a sibling node and the coordinate's derived work is +withdrawn as it does, which is the same repair undo and delete already made. +So the holdback is gone, and the first half of this file now asserts the +property that replaced it: a block forms as soon as there is a block, and +changing what a coordinate says takes back what was derived from it. -Phase 14 SP3 changed what that mark *is*. It used to be a count of covered -story actions, and the second half of this file is the price of that: deleting -an action from in front of a position slid a never-summarized action into the +Phase 14 SP3 changed what the mark *is*. It used to be a count of covered story +actions, and the second half of this file is the price of that: deleting an +action from in front of a position slid a never-summarized action into the covered range, so every delete had to slide the cursors too. The mark is a node now — `(branch_id, depth)` — and a node does not move when something in front of it is deleted, so those tests assert that nothing happens where they used to @@ -30,7 +33,7 @@ os.environ.pop("DATABASE_URL", None) import pytest from app import memorybank, models, tree -from app.context import cursors +from app.context import cursors, history from app.database import Base, SessionLocal, engine @@ -104,56 +107,25 @@ def run_memories(db, adventure, stub, monkeypatch): asyncio.run(memorybank._create_due_memories(adventure, settings, db)) -# ------------------------------------------------------------------- settling +# --------------------------------------------------- no holdback, since SP4 -def test_settled_actions_holds_back_the_newest(db): - adventure = make_adventure(db, 5) - settled = memorybank.settled_story_actions(adventure) - assert [a.index for a in settled] == [0, 1, 2, 3] +def test_a_block_forms_as_soon_as_the_story_holds_one(db, monkeypatch): + """Covered to action 5 with 12 actions: block 6-11 ends on the *newest* + action, and is summarized now rather than a turn later. - -def test_settled_actions_is_a_prefix_so_cursors_stay_valid(db): - """The safety property behind the whole approach: dropping the newest - action can never renumber or skip an earlier one.""" - adventure = make_adventure(db, 9) - full = memorybank.story_actions(adventure) - settled = memorybank.settled_story_actions(adventure) - assert full[: len(settled)] == settled - - -def test_settled_actions_on_a_one_action_story(db): - adventure = make_adventure(db, 1) - assert memorybank.settled_story_actions(adventure) == [] - - -# ------------------------------------------------------- the bug this prevents - -def test_memory_never_covers_the_newest_retryable_action(db, monkeypatch): - """Covered up to action 5 with 12 actions is exactly the case that used to - bite: the 6-action block ends on the newest action, still retryable.""" + This is exactly the case the holdback existed to refuse. What makes it safe + is no longer that the block stops short — it is that a retry of node 11 + would withdraw this memory on its way past (see + `test_deleting_a_summarized_node_withdraws_its_memory`, the same repair). + """ adventure = make_adventure(db, 12) cover(db, adventure, 6) stub = StubSummarizer() run_memories(db, adventure, stub, monkeypatch) - assert stub.excerpts == [] # only 11 settled — one short of a block - assert db.query(models.Memory).count() == 0 - assert covered_depth(db, adventure) == 5 - - -def test_the_block_lands_a_turn_later_without_the_newest_action(db, monkeypatch): - """One more action and the same block is summarized — minus the new one.""" - adventure = make_adventure(db, 13) - cover(db, adventure, 6) - - stub = StubSummarizer() - run_memories(db, adventure, stub, monkeypatch) - assert len(stub.excerpts) == 1 - excerpt = stub.excerpts[0] - assert "Action 11." in excerpt # the block's real last action - assert "Action 12." not in excerpt # the newest, still retryable + assert "Action 11." in stub.excerpts[0] memory = db.query(models.Memory).one() assert (memory.source_start, memory.source_end) == (6, 11) # The mark and the memory name the same node — that is what keeps them from @@ -162,28 +134,31 @@ def test_the_block_lands_a_turn_later_without_the_newest_action(db, monkeypatch) assert covered_depth(db, adventure) == 11 -def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch): - adventure = make_adventure(db, memorybank.MEMORY_START) +def test_the_first_memory_lands_at_memory_start(db, monkeypatch): + adventure = make_adventure(db, memorybank.MEMORY_START - 1) stub = StubSummarizer() run_memories(db, adventure, stub, monkeypatch) - assert stub.excerpts == [] + assert stub.excerpts == [] # too short to have started at all db.add(models.Action( - adventure_id=adventure.id, index=memorybank.MEMORY_START, type="do", text="Later.", + adventure_id=adventure.id, index=memorybank.MEMORY_START - 1, + type="do", text="Later.", )) db.commit() db.refresh(adventure) run_memories(db, adventure, stub, monkeypatch) - # 12 settled actions = two full blocks, caught up in one run (MAX_MEMORIES_ - # PER_RUN allows 5); neither may reach the newly added newest action. + # MEMORY_START is 12 actions = two full blocks, caught up in one run + # (MAX_MEMORIES_PER_RUN allows 5), and the newest is in the second of them. assert len(stub.excerpts) == 2 - assert not any("Later." in e for e in stub.excerpts) + assert "Later." in stub.excerpts[-1] + assert covered_depth(db, adventure) == memorybank.MEMORY_START - 1 def test_legacy_caught_up_adventure_is_not_rewound(db, monkeypatch): """An adventure summarized under the OLD rule carries a cursor equal to its - action count — one past the settled end. That used to need a clamp on every - post-turn pass, and clamping it to the *settled* count re-covered an action. + action count — one past the end of the story. That used to need a clamp on + every post-turn pass, and clamping it to the settled count re-covered an + action. A mark that names a node has no such edge: the newest action is the node, and "everything after it" is empty until the story grows. @@ -194,7 +169,7 @@ def test_legacy_caught_up_adventure_is_not_rewound(db, monkeypatch): cover(db, adventure, 12) assert covered_depth(db, adventure) == 11 # the newest action, not one past it - assert memorybank.settled_after(adventure, covered_depth(db, adventure)) == -1 + assert history.count_after(adventure, covered_depth(db, adventure)) == 0 # Grow the story and let the next block form. for i in range(12, 25): diff --git a/backend/tests/test_state_revert.py b/backend/tests/test_state_revert.py index d0eed85..62893b7 100644 --- a/backend/tests/test_state_revert.py +++ b/backend/tests/test_state_revert.py @@ -1,6 +1,13 @@ """Tests for undo/retry rolling back the shared script_state scoreboard (plan/11-state-revert-and-retry-fix.md). +Phase 14 SP4 turned the snapshots around. An action used to carry the state as +it stood *before* it ran, and rolling back read the snapshot off the action +being removed. It carries what it left *behind* now, and rolling back reads it +off the node in front — which is the same number arrived at from the other +side, and the only version a retry can use: attempts at one turn share a +starting position and differ precisely in their outcome. + Run from the backend dir: python -m pytest tests/test_state_revert.py -v """ import os @@ -17,7 +24,7 @@ os.environ.pop("DATABASE_URL", None) import pytest from fastapi import HTTPException -from app import memorybank, models +from app import attempts, memorybank, models from app.database import Base, SessionLocal, engine from app.routers import adventures @@ -44,25 +51,39 @@ def _make_adventure(db, script_state): return user, adv -def _add(db, adv, index, type_, text="x", state_before=None): +def _add(db, adv, index, type_, text="x", state_after=None): a = models.Action( adventure_id=adv.id, index=index, type=type_, text=text, - state_before=state_before, + state_after=state_after, ) db.add(a) db.flush() return a +def _forget_snapshots(db, adv): + """Blank every outcome, the way a row written before SP4 looks. + + Straight SQL, because `tree.stamp_outcome` runs on every flush precisely so + that a node written through the ORM cannot end up without one. + """ + db.query(models.Action).filter_by(adventure_id=adv.id).update( + {"state_after": None, "world_state_after": None}, synchronize_session=False + ) + db.commit() + db.expire_all() + + # ---------------------------------------------------------------- undo def test_undo_reverts_state_to_before_the_turn(db): - # A turn took the scoreboard from {gold:0} -> {gold:10}. The player action - # carries the pre-turn snapshot; current state is the mutated one. + # A turn took the scoreboard from {gold:0} -> {gold:10}. The node in front + # of the turn is what says where it started; current state is the mutated + # one. user, adv = _make_adventure(db, {"gold": 10}) - _add(db, adv, 0, "start", state_before=None) - _add(db, adv, 1, "do", state_before={"gold": 0}) - _add(db, adv, 2, "ai", state_before={"gold": 0}) + _add(db, adv, 0, "start", state_after={"gold": 0}) + _add(db, adv, 1, "do", state_after={"gold": 0}) + _add(db, adv, 2, "ai", state_after={"gold": 10}) db.commit() adventures.undo_turn(adv.id, db=db, user=user) @@ -71,12 +92,12 @@ def test_undo_reverts_state_to_before_the_turn(db): assert [a.type for a in adv.actions] == ["start"] -def test_undo_of_bare_continue_uses_ai_snapshot(db): - # A "continue" turn has no player action; the AI action's own snapshot is - # the pre-turn state. +def test_undo_of_bare_continue_uses_the_node_in_front(db): + # A "continue" turn has no player action, so the opening is what the story + # falls back to. user, adv = _make_adventure(db, {"gold": 5}) - _add(db, adv, 0, "start") - _add(db, adv, 1, "ai", state_before={"gold": 0}) + _add(db, adv, 0, "start", state_after={"gold": 0}) + _add(db, adv, 1, "ai", state_after={"gold": 5}) db.commit() adventures.undo_turn(adv.id, db=db, user=user) @@ -86,12 +107,13 @@ def test_undo_of_bare_continue_uses_ai_snapshot(db): def test_undo_leaves_state_untouched_when_snapshot_missing(db): - # Pre-migration actions have state_before = NULL: don't clobber the state. + # A row the SP4 migration could not derive an outcome for: leave the live + # state alone rather than resetting it to nothing. user, adv = _make_adventure(db, {"gold": 10}) _add(db, adv, 0, "start") - _add(db, adv, 1, "do", state_before=None) - _add(db, adv, 2, "ai", state_before=None) - db.commit() + _add(db, adv, 1, "do") + _add(db, adv, 2, "ai") + _forget_snapshots(db, adv) adventures.undo_turn(adv.id, db=db, user=user) @@ -110,7 +132,7 @@ def test_undo_raises_when_nothing_to_undo(db): def test_undo_blocked_by_active_turn_lock(db): user, adv = _make_adventure(db, {}) _add(db, adv, 0, "start") - _add(db, adv, 1, "ai", state_before={}) + _add(db, adv, 1, "ai", state_after={}) db.commit() adventures.acquire_turn_lock(adv.id) # a turn is "generating" @@ -127,7 +149,7 @@ def test_undo_blocked_by_active_turn_lock(db): def test_undo_prunes_memory_covering_removed_actions(db): user, adv = _make_adventure(db, {}) for i in range(4): - _add(db, adv, i, "ai" if i % 2 else "do", state_before={}) + _add(db, adv, i, "ai" if i % 2 else "do", state_after={}) # A memory summarizing actions up to index 3, which undo will delete. covering = models.Memory(adventure_id=adv.id, text="m", source_start=0, source_end=3) keep = models.Memory(adventure_id=adv.id, text="k", source_start=0, source_end=1) @@ -167,28 +189,40 @@ def test_forget_node_withdraws_only_what_that_node_produced(db): # ---------------------------------------------------------------- snapshot -def test_snapshot_state_is_an_independent_deep_copy(db): +def test_snapshot_outcome_is_an_independent_deep_copy(db): _, adv = _make_adventure(db, {"nested": {"n": 1}}) - snap = adventures.snapshot_state(adv) + node = models.Action(adventure_id=adv.id, index=0, type="ai", text="x") + attempts.snapshot_outcome(adv, node) adv.script_state["nested"]["n"] = 99 - assert snap == {"nested": {"n": 1}} # unaffected by later mutation + assert node.state_after == {"nested": {"n": 1}} # unaffected by later mutation -def test_snapshot_state_handles_non_dict(db): +def test_snapshot_outcome_handles_non_dict(db): _, adv = _make_adventure(db, {}) adv.script_state = None - assert adventures.snapshot_state(adv) == {} + node = models.Action(adventure_id=adv.id, index=0, type="ai", text="x") + attempts.snapshot_outcome(adv, node) + assert node.state_after == {} + + +def test_restore_state_ignores_a_node_with_no_outcome(db): + _, adv = _make_adventure(db, {"gold": 7}) + attempts.restore_state(adv, models.Action(adventure_id=adv.id, index=0, type="ai")) + assert adv.script_state == {"gold": 7} + attempts.restore_state(adv, None) + assert adv.script_state == {"gold": 7} # ---------------------------------------------------------------- retry -def test_retry_restores_state_before_regenerating(db, monkeypatch): - # Retry must roll the scoreboard back to the AI action's snapshot so - # regeneration doesn't stack output mutations on the discarded attempt. +def test_retry_restores_the_state_the_turn_started_from(db, monkeypatch): + # Retry must roll the scoreboard back to what the node in front of the AI + # action left behind, so regeneration doesn't stack output mutations on top + # of the attempt being replaced. user, adv = _make_adventure(db, {"gold": 20}) # 20 = double-applied bug value - _add(db, adv, 0, "start") - _add(db, adv, 1, "do", state_before={"gold": 0}) - _add(db, adv, 2, "ai", state_before={"gold": 10}) + _add(db, adv, 0, "start", state_after={"gold": 0}) + _add(db, adv, 1, "do", state_after={"gold": 10}) + _add(db, adv, 2, "ai", state_after={"gold": 20}) db.commit() monkeypatch.setattr(adventures.limits, "rate_limit", lambda *a, **k: None) @@ -202,11 +236,11 @@ def test_retry_restores_state_before_regenerating(db, monkeypatch): adventures.retry_action(adv.id, request=None, db=db, user=user) assert adv.script_state == {"gold": 10} - # The row survives now (it used to be deleted) so the discarded attempt - # stays readable — it's kept as variant 0. + # Nothing is written until a replacement actually arrives: the attempt on + # screen is left exactly as it was, and stays the live one. assert [a.type for a in adv.actions] == ["start", "do", "ai"] last = adv.actions[-1] - assert len(last.variants) == 1 - assert last.variant_index == 0 - assert last.variants[0]["script_state"] == {"gold": 20} # the attempt's outcome + assert last.live is True + assert last.variant_count == 0 + assert last.state_after == {"gold": 20} # its own outcome, untouched adventures._active_turns.discard(adv.id) diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py index 5b52258..1933b85 100644 --- a/backend/tests/test_tree_migration.py +++ b/backend/tests/test_tree_migration.py @@ -29,7 +29,8 @@ from fastapi import Depends from fastapi.testclient import TestClient from sqlalchemy import text -from app import auth, limits, migrations, models, tree +from app import auth, compression, limits, migrations, models, tree +from app.context import history from app.database import Base, SessionLocal, engine, get_db from app.main import app @@ -526,3 +527,218 @@ def test_deleting_the_newest_action_moves_the_head_back(client): assert db.get(models.Adventure, adventure_id).head_depth == 0 finally: db.close() + + +# ---------------------------------------- SP4: variants become sibling rows + +# One turn's retry history as schema 45 stored it: a JSON array on the AI row, +# with `variant_index` naming the entry `text` mirrors. The live one is +# deliberately not the last written — a migration that assumed it was would +# look right on every fixture where the player never went back. +RETRY_VARIANTS = [ + {"text": "Attempt one.", "reasoning": None, + "script_state": {"gold": 10}, "created_at": "2026-01-01T00:00:00", + "raw_output": "Attempt one.", + "world_state": {"delta": {"player.hp": -5}, + "report": {"applied": [{"path": "player.hp", "old": 100, "new": 95}]}, + "state": {"player": {"hp": 95}}}}, + {"text": "Attempt two.", "reasoning": "thinking", + "script_state": {"gold": 20}, "created_at": "2026-01-01T00:01:00", + "raw_output": "Attempt two.", + "world_state": {"delta": {"player.hp": -40}, + "report": {"applied": [{"path": "player.hp", "old": 100, "new": 60}]}, + "state": {"player": {"hp": 60}}}}, + {"text": "Attempt three.", "reasoning": None, + "script_state": {"gold": 30}, "created_at": "2026-01-01T00:02:00", + "raw_output": "Attempt three."}, +] +LIVE_VARIANT = 1 + +# The whole turn's assembled prompt, stored once. The attempts differ only in +# the three slices above, which is the arrangement SP4 has to preserve — giving +# each sibling a copy of this would multiply the biggest column in the database +# by the retry count. +RETRY_SNAPSHOT = { + "sections": [{"label": "history", "text": "A long prompt.", "tokens": 4}], + "prompt": {"system": "S", "story": "A long prompt."}, + "raw_output": "Attempt two.", + "script": {"logs": []}, + "world_state": RETRY_VARIANTS[LIVE_VARIANT]["world_state"], +} + + +@pytest.fixture() +def pre_split(): + """A schema-45 adventure with one retried turn, plus a plain turn each side. + + Separate from `pre_tree` so SP1's assertions keep counting what they were + written to count. The story is: 0 start, 1 do, 2 ai (three attempts), 3 do, + and the adventure's live state is the one attempt 1 produced. + """ + Base.metadata.drop_all(bind=engine) + Base.metadata.create_all(bind=engine) + with engine.begin() as conn: + 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, '')" + )) + conn.execute(text( + "INSERT INTO adventures (user_id, title, script_state, world_state) " + "VALUES (1, 'Retried', :script, :world)" + ), {"script": json.dumps({"gold": 20}), + "world": json.dumps({"player": {"hp": 60}})}) + adventure_id = conn.execute( + text("SELECT id FROM adventures WHERE title = 'Retried'") + ).scalar() + # `state_before` on each row: the scoreboard as that action found it. + # SP4 reads them one row along to build the `state_after` pair. + for index, kind, before in ( + (0, "start", None), (1, "do", {"gold": 0}), + (2, "ai", {"gold": 0}), (3, "do", {"gold": 20}), + ): + conn.execute(text( + 'INSERT INTO actions (adventure_id, "index", type, text, reasoning, ' + "state_before, context_snapshot, variants, variant_count, variant_index) " + "VALUES (:a, :i, :t, :x, :r, :sb, :cs, :v, :vc, :vi)" + ), { + "a": adventure_id, "i": index, "t": kind, + "x": RETRY_VARIANTS[LIVE_VARIANT]["text"] if kind == "ai" else f"Turn {index}.", + "r": RETRY_VARIANTS[LIVE_VARIANT]["reasoning"] if kind == "ai" else None, + "sb": None if before is None else json.dumps(before), + "cs": compression.pack(RETRY_SNAPSHOT) if kind == "ai" else None, + "v": json.dumps(RETRY_VARIANTS) if kind == "ai" else None, + "vc": len(RETRY_VARIANTS) if kind == "ai" else 0, + "vi": LIVE_VARIANT if kind == "ai" else 0, + }) + conn.execute(text("PRAGMA user_version = 45")) + try: + yield adventure_id + finally: + Base.metadata.drop_all(bind=engine) + + +def _attempts(adventure_id) -> list[tuple]: + return rows( + "SELECT variant_index, text, live, variant_count FROM actions " + 'WHERE adventure_id = :a AND "index" = 2 ORDER BY variant_index', + a=adventure_id, + ) + + +def test_each_attempt_becomes_a_row_at_the_turns_coordinate(pre_split): + migrations.bootstrap(engine) + + assert _attempts(pre_split) == [ + (0, "Attempt one.", 0, 3), + (1, "Attempt two.", 1, 3), + (2, "Attempt three.", 0, 3), + ] + # One turn, one coordinate: the siblings share a branch and a depth, and + # keep the legacy index that says which turn they are all takes on. + coordinates = rows( + 'SELECT DISTINCT branch_id, depth FROM actions WHERE adventure_id = :a ' + 'AND "index" = 2', a=pre_split, + ) + assert len(coordinates) == 1 + # ...and the rest of the story is untouched, still one row per turn. + assert scalar("SELECT count(*) FROM actions WHERE adventure_id = :a", a=pre_split) == 6 + + +def test_the_live_attempt_is_the_one_the_row_was_mirroring(pre_split): + """`variant_index` is the only record of which take the player was reading, + and it survives as the `live` flag. Guessing "the newest" instead would + silently rewrite the story of anyone who had paged back.""" + migrations.bootstrap(engine) + + live = rows( + "SELECT text FROM actions WHERE adventure_id = :a AND live = 1 " + 'AND "index" = 2', a=pre_split, + ) + assert live == [("Attempt two.",)] + + +def test_the_prompt_stays_on_the_live_attempt_and_nowhere_else(pre_split): + migrations.bootstrap(engine) + + holders = [] + for variant_index, snapshot in rows( + 'SELECT variant_index, context_snapshot FROM actions WHERE adventure_id = :a ' + 'AND "index" = 2 ORDER BY variant_index', a=pre_split, + ): + stored = compression.unpack(snapshot) if snapshot else {} + if "sections" in stored: + holders.append(variant_index) + else: + # A superseded attempt keeps only what was its own. + assert set(stored) <= set(migrations._ATTEMPT_KEYS) + assert holders == [LIVE_VARIANT] + + +def test_each_attempt_keeps_the_outcome_it_produced(pre_split): + migrations.bootstrap(engine) + + parsed = [ + (i, json.loads(state), json.loads(world) if world else None) + for i, state, world in rows( + "SELECT variant_index, state_after, world_state_after FROM actions " + 'WHERE adventure_id = :a AND "index" = 2 ORDER BY variant_index', + a=pre_split, + ) + ] + assert [(i, s) for i, s, _ in parsed] == [ + (0, {"gold": 10}), (1, {"gold": 20}), (2, {"gold": 30}) + ] + assert parsed[0][2] == {"player": {"hp": 95}} + assert parsed[1][2] == {"player": {"hp": 60}} + # Attempt three recorded no world state — an adventure with no RPG layer, + # or a take made before the column existed. It stays NULL rather than + # borrowing a neighbour's, and switching to it leaves the RPG layer alone: + # exactly what `apply_variant` did with an entry that had no world state. + assert parsed[2][2] is None + + +def test_state_after_is_the_state_before_of_the_turn_in_front(pre_split): + migrations.bootstrap(engine) + + after = dict(rows( + 'SELECT "index", state_after FROM actions WHERE adventure_id = :a ' + "AND live = 1 ORDER BY depth", a=pre_split, + )) + # Action 1's outcome is action 2's starting position, exactly. + assert json.loads(after[1]) == {"gold": 0} + # The tip has nothing in front of it, so what it left behind is what the + # adventure is carrying now. + assert json.loads(after[3]) == {"gold": 20} + + +def test_the_split_survives_being_run_again(pre_split): + migrations.bootstrap(engine) + snapshot = _attempts(pre_split) + before = scalar("SELECT count(*) FROM actions") + + migrations.bootstrap(engine) + with engine.begin() as conn: + migrations._backfill_state_after(conn) + migrations._split_variants_into_siblings(conn) + + assert scalar("SELECT count(*) FROM actions") == before, "attempts were duplicated" + assert _attempts(pre_split) == snapshot + + +def test_the_migrated_story_reads_back_as_one_turn(pre_split): + """The point of all of it: the reads see a four-action story, not six.""" + migrations.bootstrap(engine) + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, pre_split) + assert [a.text for a in history.story_actions(adventure)] == [ + "Turn 0.", "Turn 1.", "Attempt two.", "Turn 3.", + ] + assert history.count(adventure) == 4 + finally: + db.close() diff --git a/backend/tools/stress_session.py b/backend/tools/stress_session.py index 188a81b..ee00ddd 100644 --- a/backend/tools/stress_session.py +++ b/backend/tools/stress_session.py @@ -226,19 +226,19 @@ class FakeEmbeddings: # underneath all of it. Measured on a freshly built default fixture, the # columns a story tree has to migrate correctly look like this: # -# state_before / world_state_before NULL on all 600 rows. SP4 moves these -# from *before* to *after* snapshots, and -# a migration over NULLs proves nothing. +# state_after / world_state_after the same value on all 600 rows (they +# were state_before, and NULL, until SP4 +# turned them round). A rollback over +# identical snapshots proves nothing. # scenario_id / world_state absent. No RPG layer, so the cooldown # clock SP5 must not advance never runs. # adventure_scripts none. script_state rollback is exactly # what a branch switch reuses. # memory_cursor / summary_cursor both 0. SP3 replaces the cursors. -# variants two attempts of byte-identical text, -# variant_index always 0 — so "which -# attempt is live?", the one question -# SP4's migration has to answer, has no -# observable answer. +# sibling attempts two of byte-identical text with the +# first always live — so "which attempt +# is live?", the one question SP4 has to +# answer, has no observable answer. # # --rich fills in exactly those and changes nothing else, so the measuring # fixture's numbers stay comparable run to run. It is a *correctness* fixture: @@ -305,29 +305,23 @@ def rich_world_state(schema: dict, turn: int) -> dict: return ws -def rich_variants(rng: random.Random, index: int) -> tuple[list, int]: - """Distinguishable retry attempts, and which one is live. +def rich_attempts(rng: random.Random, index: int) -> tuple[list[str], int]: + """Distinguishable retry attempts, and which one the story tells. - Every attempt in the default fixture carries the same text with - variant_index pinned at 0. That is the one thing SP4 has to get right and - the one thing that fixture cannot witness, so here the texts differ, the - counts differ, and the live one is often not the last. + Every attempt in the default fixture carries the same text with the live + one pinned at 0. That is the one thing SP4 has to get right and the one + thing that fixture cannot witness, so here the texts differ, the counts + differ, and the live one is often not the last. """ count = 3 if index % 12 == 1 else 2 - entries = [ - { - "text": f"[attempt {n + 1} of {count} at turn {index}] {prose(rng, 240)}", - "reasoning": None, - "script_state": rich_script_state(index), - "created_at": f"2026-01-01T00:{index % 60:02d}:{n:02d}", - } + texts = [ + f"[attempt {n + 1} of {count} at turn {index}] {prose(rng, 240)}" for n in range(count) ] # Deliberately not always the newest: a player who retried twice and then - # went back to the first take is the case that breaks a migration which - # assumes the live attempt is the last one written. - live = 0 if index % 18 == 1 else count - 1 - return entries, live + # went back to the first take is the case that breaks anything assuming the + # live attempt is the last one written. + return texts, (0 if index % 18 == 1 else count - 1) def add_rich_extras(db, args, rng: random.Random, user, adventure) -> None: @@ -387,7 +381,7 @@ def add_second_adventure(db, rng: random.Random, user) -> int: 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), + state_after=rich_script_state(i), ) tree.place_action(db, other, action) db.add(action) @@ -404,28 +398,31 @@ def add_second_adventure(db, rng: random.Random, user) -> int: def _assert_live_variant_invariant(db, adventure_id: int) -> None: - """`text` mirrors the live attempt, on every retried row. + """Exactly one attempt per turn is live, on every turn. - Checked here rather than trusted, because SP4's migration reads exactly - this to decide which sibling node becomes the head of the turn. A fixture - that quietly violated it would let a wrong migration look right. + Checked here rather than trusted, because a coordinate with two live + siblings tells its story twice and a coordinate with none drops a turn out + of it — and both fail by *reading* wrong, never by raising. A fixture that + quietly violated it would let wrong code look right. """ - rows = ( + groups: dict[tuple, list[models.Action]] = {} + for action in ( db.query(models.Action) - .filter(models.Action.adventure_id == adventure_id, - models.Action.variant_count > 0) + .filter(models.Action.adventure_id == adventure_id) .all() - ) - for action in rows: - entries = action.variants or [] - live = entries[action.variant_index]["text"] - if live != action.text: + ): + groups.setdefault((action.branch_id, action.depth), []).append(action) + retried = 0 + for (branch_id, depth), rows in groups.items(): + live = [a for a in rows if a.live] + if len(live) != 1: sys.exit( - f"fixture is inconsistent: action index {action.index} has text " - f"that is not its live attempt (variant_index=" - f"{action.variant_index} of {len(entries)})." + f"fixture is inconsistent: branch {branch_id} depth {depth} has " + f"{len(live)} live attempts out of {len(rows)}." ) - if not rows: + if len(rows) > 1: + retried += 1 + if not retried: sys.exit("--rich built no retried actions; raise --actions above 6.") @@ -476,47 +473,55 @@ def build_fixture(args, rng: random.Random) -> tuple[int, int]: retried = is_ai and i % 6 == 1 if args.rich: # Distinguishable attempts, and a live one that is often not - # the last written. `text` must mirror the live attempt — that - # invariant is what SP4's migration reads to decide which - # sibling becomes the head. - entries, live = rich_variants(rng, i) if retried else ([], 0) - body = entries[live]["text"] if retried else ( - f"[turn {i}] {NARRATION}" if is_ai else f"[turn {i}] {PLAYER_INPUT}" - ) + # the last written. + texts, live = rich_attempts(rng, i) if retried else ([], 0) + if not retried: + texts = [f"[turn {i}] {NARRATION}" if is_ai + else f"[turn {i}] {PLAYER_INPUT}"] else: - entries, live = ( - [{"text": NARRATION, "reasoning": None, "script_state": {}, - "created_at": "2026-01-01T00:00:00"} for _ in range(2)] - if retried else [] - ), 0 - body = NARRATION if is_ai else PLAYER_INPUT - action = models.Action( - adventure_id=adventure.id, - index=i, - type="ai" if is_ai else "do", - text=body, - context_snapshot={"system": SNAPSHOT_SYSTEM, "story": SNAPSHOT_STORY}, - world_delta={"delta": {"player.hp": -3}, - "applied": [{"path": "player.hp", "old": 88, "new": 85}]}, - # Every third AI turn was retried once, so the retry history is - # carrying weight a list response must not pay for. - variants=entries or None, - variant_count=len(entries), - variant_index=live, - # NULL unless --rich. These are the columns a story tree turns - # from *before* pictures into *after* ones, so a fixture that - # leaves them unset cannot witness that change. - state_before=rich_script_state(i) if args.rich else None, - 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) + texts = [NARRATION, NARRATION] if retried else [ + NARRATION if is_ai else PLAYER_INPUT + ] + live = 0 + for n, body in enumerate(texts): + action = models.Action( + adventure_id=adventure.id, + index=i, + type="ai" if is_ai else "do", + text=body, + # The turn's assembled prompt is stored once, on the + # attempt the story tells; a superseded sibling keeps only + # its own slices. That is the invariant `app/attempts.py` + # maintains, and a fixture that ignored it would multiply + # the biggest column in the database by the retry count. + context_snapshot=( + {"system": SNAPSHOT_SYSTEM, "story": SNAPSHOT_STORY} + if n == live else + {"raw_output": body} + ), + world_delta={"delta": {"player.hp": -3}, + "applied": [{"path": "player.hp", "old": 88, "new": 85}]}, + # Every third AI turn was retried once, so a turn is + # sometimes several rows sharing one coordinate. + live=(n == live), + variant_index=n, + variant_count=len(texts) if len(texts) > 1 else 0, + # Monotonic under --rich, so a bad rollback reads as a + # wrong number rather than as nothing. Left to + # `tree.stamp_outcome` otherwise, which writes the + # adventure's (unchanging) state — true, and no witness. + state_after=rich_script_state(i) if args.rich else None, + world_state_after=( + 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( @@ -717,7 +722,7 @@ def parse_args(argv=None): "blind spot, where the bank's cost is invisible") p.add_argument("--rich", action="store_true", help="populate the columns the measuring fixture leaves at " - "their defaults: state_before/world_state_before, an " + "their defaults: state_after/world_state_after, an " "RPG scenario and live world state, adventure scripts, " "non-zero memory/summary cursors, pinned and forgotten " "memories, distinguishable retry attempts, and a second " diff --git a/frontend/src/pages/Play.jsx b/frontend/src/pages/Play.jsx index db434f6..f9d6693 100644 --- a/frontend/src/pages/Play.jsx +++ b/frontend/src/pages/Play.jsx @@ -1692,8 +1692,15 @@ export default function Play() { busy={busy} previewIndex={previewing ? previewing.index : null} onPreview={setPreview} - onSwitched={(updated) => - setActions((prev) => prev.map((a) => (a.id === updated.id ? updated : a)))} + onSwitched={(updated) => { + // Matched on the action we asked about, not on the one + // that came back. Since the story tree made every + // attempt its own row (phase 14 SP4), switching moves + // the story onto a *different* row rather than + // rewriting this one, so the reply carries a new id. + setActions((prev) => prev.map( + (a) => (a.id === action.id ? updated : a))) + }} onError={(message) => setToast({ text: message, isError: true })} /> )}