From c51531709df311782fe4a6ec0950b2ce365a9daa Mon Sep 17 00:00:00 2001 From: parththakkar106 Date: Tue, 18 Aug 2026 00:48:03 +0530 Subject: [PATCH] Mark the story with a node, not with a count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memory bank and the story summary each kept a cursor: how many story actions they had already covered. A count is a position in a list, and this list moves — delete an action in front of the mark and every later one slides down a slot, so the mark now covers one it has never read. All the cursor bookkeeping existed to patch that up. Both marks are now (branch_id, depth): the node up to and including which the work is done. A depth is a coordinate along a path, not an offset into a list, so nothing in front of it can move it. That deletes rather than rewrites `position_of_index`, `note_action_removed`, `_rewind_cursors_to_index`, `prune_dangling_memories` and the every-pass clamp in `run_post_turn`. A memory hangs off the node its block ends on, so a fork inherits its ancestors' memories without copying any, and retrieval selects through the branch clause over the *whole* lineage — recall is long-range by definition and cannot be windowed. Measured: 1,807 B on a story forked twenty times against 1,823 B on a flat one of the same length. Migrations 53-56 translate the old counts into nodes. They rewrite `adventures` and not `actions`, so this one needs no VACUUM FULL. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Dvvqn9ZDR4ixeFPHNbww7 --- backend/app/context/cursors.py | 146 +++++++++ backend/app/context/history.py | 97 ++++-- backend/app/context/lineage.py | 67 ++++- backend/app/memorybank.py | 221 +++++++------- backend/app/migrations.py | 75 +++++ backend/app/models.py | 24 +- backend/app/routers/adventures.py | 42 ++- backend/app/tree.py | 18 ++ backend/tests/test_history_window.py | 44 ++- backend/tests/test_memory_nodes.py | 416 ++++++++++++++++++++++++++ backend/tests/test_memory_settling.py | 181 +++++++---- backend/tests/test_state_revert.py | 19 +- backend/tests/test_tree_migration.py | 85 +++++- backend/tools/stress_session.py | 16 +- plan/14-phase-story-tree.md | 65 +++- plan/STATUS.md | 78 +++-- 16 files changed, 1334 insertions(+), 260 deletions(-) create mode 100644 backend/app/context/cursors.py create mode 100644 backend/tests/test_memory_nodes.py diff --git a/backend/app/context/cursors.py b/backend/app/context/cursors.py new file mode 100644 index 0000000..2e4eb30 --- /dev/null +++ b/backend/app/context/cursors.py @@ -0,0 +1,146 @@ +"""Phase 14 — how far along a story the derived work has got. + +Two things are built from the story and stored beside it: the memories, and the +Story Summary. Both need to know where they left off, and that mark used to be +a *count* — "the first 12 story actions are covered". A count is a position in +a list, and this list moves: delete an action from in front of the mark and +every later action slides down a slot, so the mark now covers one it has never +seen. Every rule in `memorybank` about sliding cursors, rewinding them and +translating between positions and `Action.index` existed to patch that up, and +each was a separate chance to get it wrong in a way nothing reports. + +A cursor here is an **anchor**: `(branch_id, depth)`, the node up to and +including which the work is done. Deleting an action does not move it, because +a depth is not a position — it is a coordinate along a path. "What is not +covered yet" becomes `history.count_after(anchor)`, which is a question about +the story rather than about a list index, and it answers correctly whatever has +been deleted from in front of it. + +The branch half is what makes it survive forking. A depth alone is ambiguous +once two branches have a node 41; the anchor says which one, and +`Path.depth_on` reads it back as a depth on whatever story is being played — +capped at the fork, or "nothing covered" if the anchor sits on ground this path +never travelled. Until forking ships there is one branch and that is always a +no-op, which is the point: the coordinate system is right before anything needs +it to be. + +`NO_DEPTH` (-1) is "nothing covered", so a fresh adventure needs no special +case: every node is deeper than -1. +""" + +from sqlalchemy.orm import Session + +from .. import models +from . import history, lineage + +NO_DEPTH = lineage.NO_DEPTH + + +class Cursor: + """One anchor on the adventure row: the memory bank's, or the summary's. + + A pair of columns rather than a foreign key to the node. The node can be + deleted — that is most of what undo does — and the boundary is still + meaningful afterwards, so a pointer that has to resolve would be a pointer + that keeps not resolving. + """ + + def __init__(self, name: str): + self.name = name + self.branch_field = f"{name}_cursor_branch_id" + self.depth_field = f"{name}_cursor_depth" + + # ------------------------------------------------------------- reading + + def stored(self, adventure: models.Adventure) -> tuple[int | None, int]: + """The anchor exactly as written, unread by any path.""" + depth = getattr(adventure, self.depth_field) + return getattr(adventure, self.branch_field), ( + NO_DEPTH if depth is None else depth + ) + + def depth(self, db: Session, adventure: models.Adventure) -> int: + """The anchor as a depth on the story currently being played.""" + branch_id, depth = self.stored(adventure) + return lineage.path_of(db, adventure).depth_on(branch_id, depth) + + # ------------------------------------------------------------- writing + + def anchor_at(self, adventure: models.Adventure, node: models.Action) -> None: + """Mark the work done up to and including `node`. + + Takes the node's own branch, not the adventure's head: a block of six + actions can end before the fork this branch was made at, and the + coverage belongs where the ground is. + """ + setattr(adventure, self.branch_field, node.branch_id) + setattr(adventure, self.depth_field, lineage.NO_DEPTH + if node.depth is None else node.depth) + + def rewind_to( + self, adventure: models.Adventure, branch_id: int | None, depth: int + ) -> None: + """Move the anchor back to `depth` if it is past it; never forward. + + The one direction that is safe without knowing what else has happened: + re-covering ground costs a summarizer call, skipping it loses a stretch + of story out of the memories for good. + """ + _, current = self.stored(adventure) + if current <= depth: + return + setattr(adventure, self.branch_field, branch_id) + setattr(adventure, self.depth_field, max(depth, NO_DEPTH)) + + +MEMORY = Cursor("memory") +SUMMARY = Cursor("summary") +ALL = (MEMORY, SUMMARY) + + +def rewind_all( + adventure: models.Adventure, branch_id: int | None, depth: int +) -> None: + """Hand a stretch of story back to *both* passes. + + They move together because they cover the same ground from different sides: + the summary folds in the memories, so a memory withdrawn without rewinding + the summary leaves the summary claiming to have read something no longer + there. + """ + for cursor in ALL: + cursor.rewind_to(adventure, branch_id, depth) + + +def anchor_at_position( + adventure: models.Adventure, cursor: Cursor, position: int +) -> None: + """Set `cursor` from a count of covered story actions — a v1 bundle's mark, + or a database written before the anchors existed. + + The position-th story action in depth order is the node that says the same + thing, and goes on saying it once something in front of it is deleted. A + position past the end of the story is not a bad value: an adventure caught + up under the older rule can carry one, and it means the same thing the tip + does, so that is where it lands. + + The SQL half of this rule is `migrations._backfill_cursor_anchors`, which + has to do it for every adventure at once without loading any of them; the + two must agree. + """ + if position <= 0: + return + covered = history.slice_(adventure, position - 1, 1) or history.tail(adventure, 1) + if covered: + cursor.anchor_at(adventure, covered[0]) + + +def position_of(adventure: models.Adventure, depth: int) -> int: + """How many story actions lie at or before `depth` — an anchor read back as + a count. + + The v1 export bundle stores the cursors as positions, and a v1 bundle is + read by builds that have never heard of a depth. This is the one place that + still speaks that coordinate system, and SP6's v2 format retires it. + """ + return max(history.count(adventure) - history.count_after(adventure, depth), 0) diff --git a/backend/app/context/history.py b/backend/app/context/history.py index 984f833..4b4dfe7 100644 --- a/backend/app/context/history.py +++ b/backend/app/context/history.py @@ -14,10 +14,9 @@ story. Three rules hold everything together: -* **One definition of "story action".** The cursors in memorybank are - *positions* in this filtered, depth-ordered list, so SQL and Python must - agree on membership exactly or a cursor silently points at a different - action. `_STORY_TEXT` and `is_story_text()` are that one definition, written +* **One definition of "story action".** Membership decides what a reader sees + and what the summarizer is handed, so SQL and Python must agree on it + exactly. `_STORY_TEXT` and `is_story_text()` are that one definition, written twice; keep them in step. * **Never load twice.** If `adventure.actions` is already in memory (the scripting pipeline hands the whole history to user scripts, as AI Dungeon @@ -32,6 +31,12 @@ Three rules hold everything together: 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. + +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 +"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. """ from sqlalchemy import func, inspect as sa_inspect @@ -162,6 +167,7 @@ def _count_query( adventure: models.Adventure, path: lineage.Path, exclude_action_id: int | None, + entries: int | None = None, ): """A real `SELECT count(...)`. @@ -172,7 +178,7 @@ def _count_query( that greps the SQL cannot tell the two apart. """ return db.query(func.count(models.Action.id)).filter( - *_filters(adventure, path, exclude_action_id) + *_filters(adventure, path, exclude_action_id, entries) ) @@ -311,30 +317,87 @@ def slice_( ) -def position_of_index(adventure: models.Adventure, index: int) -> int: - """The position the story action with `Action.index == index` occupies — - i.e. how many story actions come before it. +def depth_of(action: models.Action) -> int: + """`action.depth`, with the no-depth case spelled once. - Translates between the two coordinate systems that keep tripping this code - up: cursors are positions, `Memory.source_start/_end` are `Action.index` - values, and the two diverge the moment anything is deleted. + A row with no depth is a pre-tree row, which no path contains — so it can + only turn up in an already-loaded collection, and it sorts before the story + rather than after it. """ - in_memory = _from_memory(adventure, None) + return action.depth if action.depth is not None else lineage.NO_DEPTH + + +def count_after( + adventure: models.Adventure, depth: int, exclude_action_id: int | None = None +) -> int: + """How many story actions lie past `depth` on the path. + + The node-anchored replacement for "the story is N long and the cursor is at + M". Deleting an action from in front of the boundary makes this number + smaller, which is true; it does not make the boundary point somewhere else, + which is the bug the positions had. + + `covering_after` says exactly which lineage entries can hold a node deeper + than the boundary, so a cursor near the tip names one branch however many + forks are below it. + """ + in_memory = _from_memory(adventure, exclude_action_id) if in_memory is not None: - return next( - (i for i, a in enumerate(in_memory) if a.index >= index), len(in_memory) - ) + return sum(1 for a in in_memory if depth_of(a) > depth) db = _session(adventure) if db is None: return 0 + path = _path(db, adventure) return ( - _count_query(db, adventure, _path(db, adventure), None) - .filter(models.Action.index < index) + _count_query( + db, adventure, path, exclude_action_id, path.covering_after(depth) + ) + .filter(models.Action.depth > depth) .scalar() or 0 ) +def after( + adventure: models.Adventure, + depth: int, + limit: int, + exclude_action_id: int | None = None, +) -> list[models.Action]: + """The oldest `limit` story actions past `depth`, oldest first. + + "The next block the summarizer has not seen", asked as a fact about the + story rather than as an offset into a list that shifts underneath it. + """ + if limit <= 0: + return [] + in_memory = _from_memory(adventure, exclude_action_id) + if in_memory is not None: + return [a for a in in_memory if depth_of(a) > depth][:limit] + db = _session(adventure) + if db is None: + return [] + path = _path(db, adventure) + return ( + _query(db, adventure, path, exclude_action_id, path.covering_after(depth)) + .filter(models.Action.depth > depth) + .order_by(*_OLDEST_FIRST) + .limit(limit) + .all() + ) + + +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. + + 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. + """ + rows = tail(adventure, 2) + return rows[0] if len(rows) == 2 else None + + def max_action_index(adventure: models.Adventure) -> int: """Highest `Action.index` in the adventure, story text or not. -1 if empty. diff --git a/backend/app/context/lineage.py b/backend/app/context/lineage.py index 428df99..90bc0dc 100644 --- a/backend/app/context/lineage.py +++ b/backend/app/context/lineage.py @@ -83,13 +83,25 @@ class Path: # ---------------------------------------------------------------- SQL - def clause(self, model=models.Action, count: int | None = None): + def clause( + self, + model=models.Action, + count: int | None = None, + unanchored: bool = False, + ): """The branch clause, over `model` (`Action` or `Memory`). `count` limits it to the newest `count` lineage entries — the windowed read. `None` is the whole lineage, which is what anything counting from the *oldest* end (a slice, a total) has to use. + `unanchored` keeps rows with no depth. Only memories ever have one: a + hand-written memory summarises no node, so it has a branch but no + depth, and a capped `depth <= n` would drop it the moment its branch + stopped being the newest entry — a memory vanishing at the first fork + after it was typed. An action with no depth is a pre-tree row that no + read should see, so actions never pass 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. @@ -97,13 +109,16 @@ 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) for b, d in entries]) + return or_(*[self._entry_clause(model, b, d, unanchored) for b, d in entries]) @staticmethod - def _entry_clause(model, branch_id: int, max_depth: int | None): + def _entry_clause(model, branch_id: int, max_depth: int | None, unanchored=False): if max_depth is None: return model.branch_id == branch_id - return and_(model.branch_id == branch_id, model.depth <= max_depth) + within = model.depth <= max_depth + if unanchored: + within = or_(within, model.depth.is_(None)) + return and_(model.branch_id == branch_id, within) # ------------------------------------------------------------- Python @@ -158,6 +173,50 @@ class Path: return i + 1 return total + def covering_after(self, depth: int) -> int: + """How many lineage entries can hold a node deeper than `depth`. + + The counterpart to `prefix_covering`, and unlike it this is exact + rather than an estimate: entry *i* holds nothing deeper than its own + cap, and the caps descend, so the first entry capped at or below + `depth` ends the search — it and everything older is behind the + boundary. Reading "the story after the cursor" therefore names one + branch on any story whose cursor is on its newest branch, however + often it has forked. + """ + for i, (_, max_depth) in enumerate(self.entries): + if max_depth is not None and max_depth <= depth: + return i + return len(self.entries) + + def depth_on(self, branch_id: int | None, depth: int) -> int: + """A stored `(branch_id, depth)` anchor, read as a depth on *this* path. + + An anchor is how far along a story some derived work has got — which + memories cover, what the summary has folded in. It names a node, so + moving to another path has to be answered rather than assumed: + + * the anchor's branch is on this path — the depth stands, capped at the + fork the path takes off that branch, because nothing past the fork is + on this story; + * the branch is not on this path at all — the work was done on ground + this story never travelled, so nothing here is covered. + + The second case cannot arise while an adventure has one branch: the + anchor is always set from a node on it. It exists because the fallback + for "I don't know" must be to redo the work, not to skip it. + """ + if depth <= NO_DEPTH: + return NO_DEPTH + if branch_id is None: + # A pre-tree anchor, or one set by hand. There is one story, so the + # depth is a position in it and means what it says. + return depth + for entry_branch, max_depth in self.entries: + if entry_branch == branch_id: + return depth if max_depth is None else min(depth, max_depth) + return NO_DEPTH + def branch_of(db: Session, adventure: models.Adventure) -> models.Branch | None: """The branch this adventure is being read at, or None if it has none. diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index 91e0a9b..7454e30 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -27,7 +27,7 @@ from sqlalchemy import func, select, update from sqlalchemy.orm import Session, object_session from . import models, tree, vectors -from .context import history, story_actions, truncate_to_last_tokens +from .context import cursors, history, lineage, story_actions, truncate_to_last_tokens from .database import SessionLocal from .providers import OpenAICompatibleProvider, ProviderError from .vectors import cosine # re-exported: the ranking lives here, the maths there @@ -165,20 +165,23 @@ def settled_count(adventure: models.Adventure) -> int: return max(history.count(adventure) - 1, 0) -def settled_slice(adventure: models.Adventure, start: int, length: int) -> list[models.Action]: - """Settled story actions at positions [start, start + length). +def settled_after(adventure: models.Adventure, depth: int) -> int: + """How many settled story actions lie past `depth`. - Callers must already have checked against `settled_count()`; this only - fetches, it does not re-clamp. + "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.slice_(adventure, start, length) + 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_slice` instead, which express the same thing without reading the + `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 @@ -189,68 +192,50 @@ def settled_story_actions(adventure: models.Adventure) -> list[models.Action]: longer in the story. Holding one action back costs a turn of latency and makes that unreachable. - The result is always a prefix of story_actions(), so memory_cursor and - summary_cursor stay valid positions and no action is ever skipped. + 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 _rewind_cursors_to_index(adventure: models.Adventure, index: int) -> None: - """Move both cursors back to the position of Action.index `index`. +def forget_node(db: Session, adventure: models.Adventure, action: models.Action) -> int: + """Withdraw what a node produced, because the node is being removed. - The cursors are *positions* into story_actions() while Memory.source_* are - Action.index values, so the two spaces have to be translated between (they - diverge as soon as any action is deleted). + Call it before deleting `action` (undo, delete-an-action). A memory hangs + off the node whose block it ends on, so "which memories described this?" is + a lookup on `(branch_id, depth)` rather than a scan for rows whose covered + range has fallen off the end of the story — which is what + `prune_dangling_memories` did, and it could only ever notice the damage + after the fact. + + Discarding the memory is half of it. The stretch of story it covered is + still behind the cursors, so without a rewind those actions read as + summarized with nothing describing them, silently, for the rest of the + adventure. `source_start` is where that stretch began; the anchor goes to + the node before it, which is a depth whether or not anything still sits + there. + + Returns how many memories were withdrawn. """ - position = history.position_of_index(adventure, index) - adventure.memory_cursor = min(adventure.memory_cursor, position) - adventure.summary_cursor = min(adventure.summary_cursor, position) - - -def note_action_removed(adventure: models.Adventure, action: models.Action) -> None: - """Keep the cursors pointing at the same actions when one is deleted from - *before* them. Call BEFORE the delete, while the action is still in the list. - - memory_cursor counts actions from the start of the story, so removing an - earlier action slides every later one down a slot — without this, an action - that was never summarized shifts into the "already covered" range and is - skipped forever. - """ - if not history.is_story_text(action.text): - return # not in the list the cursors count, so nothing shifts - # Actions are ordered by index, so "how many come before it" is exactly - # "how many have a lower index" — no need to walk the list to find it. - position = history.position_of_index(adventure, action.index) - if position < adventure.memory_cursor: - adventure.memory_cursor -= 1 - if position < adventure.summary_cursor: - adventure.summary_cursor -= 1 - - -def prune_dangling_memories(adventure: models.Adventure, db: Session) -> int: - """Delete memories that summarized actions which no longer exist (e.g. after - undo). source_start/source_end are Action.index values; a memory is dangling - if any covered action is past the current end of the story. Returns the count - removed. - - Throwing a memory away is not enough on its own: the actions it covered are - still behind memory_cursor, so they would read as summarized with nothing - describing them. Rewind to where the earliest discarded memory began, so - those actions are summarized again. - """ - max_index = history.max_action_index(adventure) - dangling = [ - m for m in adventure.memories - if m.source_end is not None and m.source_end > max_index - ] - if not dangling: + if action.branch_id is None or action.depth is None: + return 0 # a pre-tree row: no path contains it, so nothing hangs off it + doomed = ( + db.query(models.Memory) + .filter( + models.Memory.adventure_id == adventure.id, + models.Memory.branch_id == action.branch_id, + models.Memory.depth == action.depth, + ) + .all() + ) + if not doomed: return 0 - starts = [m.source_start for m in dangling if m.source_start is not None] - for m in dangling: - db.delete(m) + starts = [m.source_start for m in doomed if m.source_start is not None] + for memory in doomed: + db.delete(memory) if starts: - _rewind_cursors_to_index(adventure, min(starts)) - return len(dangling) + cursors.rewind_all(adventure, action.branch_id, min(starts) - 1) + return len(doomed) # ---------- Retrieval (runs inside the turn, before build_context) ---------- @@ -279,9 +264,16 @@ async def retrieve_memories( # walk adventure.memories, which loaded every row of the bank *including # its vector* — ~31 KB a memory, three megabytes a turn, 96% of everything # a turn read. Two ids and a flag per row is about eight bytes. + # + # The branch clause is the *whole* lineage here, not the window the story + # is read through: retrieval is long-range recall, and a memory of what + # happened forty turns ago is exactly what it exists to find. It stays + # affordable because memories are sparse — one per six actions — so the + # ancestry of even a heavily forked story returns tens of tiny rows. catalogue = db.execute( select(models.Memory.id, models.Memory.pinned).where( models.Memory.adventure_id == adventure.id, + lineage.path_of(db, adventure).clause(models.Memory, unanchored=True), models.Memory.forgotten.is_(False), models.Memory.embedded.is_(True), ) @@ -381,17 +373,14 @@ async def run_post_turn(adventure_id: int) -> None: ) if settings is None: return - # Undo/retry can shrink the action list below a stored cursor, which - # would stall summarization until the story grew past it again. - # Deliberately the FULL count, not the settled one: an adventure that - # was caught up under the old rule can have a cursor equal to the action - # count, and clamping to settled would rewind it one step, re-covering - # an already-summarized action in the next block. Both consumers below - # read settled actions and bail on a negative remainder, so a cursor - # briefly sitting one past the settled end is harmless. - total = history.count(adventure) - adventure.memory_cursor = min(adventure.memory_cursor, total) - adventure.summary_cursor = min(adventure.summary_cursor, total) + # No cursor clamp here any more. Undo can leave the story shorter than + # the mark, and a *position* past the end of the list was a stalled + # pass until the story grew back past it — hence a clamp on every + # post-turn run, which had its own trap (clamping to the settled count + # rewound a caught-up adventure a step and re-covered an action). An + # anchor past the tip is not a broken value: `settled_after` just + # reports nothing to do, and the story growing back past it resumes + # exactly where it left off. if adventure.auto_summarize: await _create_due_memories(adventure, settings, db) await _update_story_summary(adventure, settings, db) @@ -408,14 +397,17 @@ async def _create_due_memories( ) -> None: provider = summary_provider(settings) for _ in range(MAX_MEMORIES_PER_RUN): - # Re-counted each pass: a memory just committed doesn't change the - # count, but this loop is the only thing that moves the cursor, so the - # comparison has to be against a total that is still current. - settled = settled_count(adventure) - cursor = adventure.memory_cursor - if settled < MEMORY_START or settled - cursor < MEMORY_INTERVAL: - return - block = settled_slice(adventure, cursor, MEMORY_INTERVAL) + # Re-read each pass: a memory just committed doesn't change the story, + # 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: + 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) + block = history.after(adventure, anchor, MEMORY_INTERVAL) if len(block) < MEMORY_INTERVAL: return excerpt = truncate_to_last_tokens("\n\n".join(a.text for a in block), 2000) @@ -430,46 +422,52 @@ async def _create_due_memories( memory = models.Memory( adventure_id=adventure.id, text=text, - source_start=block[0].index, - source_end=block[-1].index, + source_start=block[0].depth, + source_end=block[-1].depth, ) - # Phase 14: hang it off the node it summarised, so a fork inherits the - # memories of the path it forked from and nothing else. - tree.place_memory(db, adventure, memory) + # Hang it off the node it summarised, so a fork inherits the memories of + # the path it forked from and nothing else — and move the mark to that + # same node. The two are one statement about where this pass has got to, + # and writing them from the same row is what keeps them in step however + # gappy the depths underneath are. + tree.attach_memory(memory, block[-1]) db.add(memory) - adventure.memory_cursor = cursor + MEMORY_INTERVAL + cursors.MEMORY.anchor_at(adventure, block[-1]) db.commit() async def _update_story_summary( adventure: models.Adventure, settings: models.Settings, db: Session ) -> None: - settled = settled_count(adventure) - if settled - adventure.summary_cursor < SUMMARY_INTERVAL: + anchor = cursors.SUMMARY.depth(db, adventure) + uncovered = settled_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) + if caught_up is None: return - # Fold in memories covering the uncovered stretch; fall back to raw story - # text if memory creation is lagging (e.g. it just failed). - # summary_cursor is a position into story_actions(); Memory.source_end is - # an Action.index. Translate the cursor to an index boundary before - # comparing — the two spaces diverge once actions are deleted or empty. - if adventure.summary_cursor < settled: - [first_uncovered] = settled_slice(adventure, adventure.summary_cursor, 1) - boundary = first_uncovered.index - else: - last = settled_slice(adventure, settled - 1, 1) if settled else [] - boundary = last[0].index + 1 if last else 0 - new_events = [ - m.text - for m in adventure.memories - if m.source_end is not None and m.source_end >= boundary - ] + # Fold in the memories of the stretch the summary has not read — every + # memory hanging off a node past the anchor. Both marks and every memory + # are now depths on one path, so there is no translation between coordinate + # systems left to get wrong. Falls back to raw story text if memory + # creation is lagging (e.g. it just failed). + new_events = db.execute( + select(models.Memory.text) + .where( + models.Memory.adventure_id == adventure.id, + lineage.path_of(db, adventure).clause(models.Memory), + models.Memory.depth > anchor, + ) + .order_by(models.Memory.depth) + ).scalars().all() if new_events: events_text = "\n".join(f"- {t}" for t in new_events) else: - block = settled_slice( - adventure, adventure.summary_cursor, settled - adventure.summary_cursor - ) + block = history.after(adventure, anchor, uncovered) events_text = truncate_to_last_tokens("\n\n".join(a.text for a in block), 2000) current = adventure.story_summary.strip() @@ -487,7 +485,7 @@ async def _update_story_summary( if not text: return adventure.story_summary = text - adventure.summary_cursor = settled + cursors.SUMMARY.anchor_at(adventure, caught_up) db.commit() @@ -496,6 +494,13 @@ async def _embed_pending( ) -> None: # A query, not a walk of adventure.memories: this ran every turn and pulled # the whole bank's vectors to find the handful that had none. + # + # No branch clause, deliberately, here and in the eviction below. Being + # embedded is a fact about the row, not about the path being played: + # skipping a sibling's memories would only mean embedding them later, at + # the moment somebody switched branches and wanted them ranked. Capacity is + # the same — the bank belongs to the adventure, and evicting the memories + # of a story nobody is reading is exactly the right thing to evict first. pending = ( db.query(models.Memory) .filter( diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 787c321..16bd906 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -214,6 +214,18 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ # columns above (_backfill_tree, hung off this version because it needs all # of them to exist). (52, "CREATE INDEX IF NOT EXISTS ix_actions_branch_depth ON actions (branch_id, depth)"), + # Phase 14, SP3 — the memory and summary cursors stop being positions in the + # story and become nodes in it: (branch, depth) of the last action each pass + # covered. Legacy `memory_cursor` / `summary_cursor` stay, unread, until SP8 + # drops them beside `actions.index`. + # + # Unlike 46-52 this rewrites `adventures`, not `actions` — a few hundred + # rows against a few hundred thousand — so it needs no VACUUM FULL of its + # own. (The one SP1's deploy asks for is still owed.) + (53, "ALTER TABLE adventures ADD COLUMN memory_cursor_branch_id INTEGER"), + (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"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) @@ -224,6 +236,7 @@ VARIANT_COUNT_VERSION = 37 EMBEDDING_BLOB_VERSION = 38 SNAPSHOT_COMPRESS_VERSION = 43 TREE_BACKFILL_VERSION = 52 +CURSOR_ANCHOR_VERSION = 56 # 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). @@ -496,6 +509,66 @@ def _backfill_tree(conn) -> None: """)) +# A frozen copy of `context.history._STORY_TEXT` as it stood at version 56: +# "text that is not blank once whitespace is stripped". It is written out here +# rather than imported because a migration has to keep meaning what it meant on +# the day it ran, while the module is free to move. `char()` is `chr()` on +# Postgres and there is no third spelling, so it takes a dialect map. +def _story_text_sql(column: str, sqlite: bool) -> str: + char = "char" if sqlite else "chr" + folded = column + for code in (10, 13, 9): # newline, carriage return, tab + folded = f"replace({folded}, {char}({code}), ' ')" + return f"trim({folded}) <> ''" + + +def _backfill_cursor_anchors(conn) -> None: + """Read each adventure's two cursors as nodes instead of as positions. + + `memory_cursor` = 12 meant "the first twelve story actions are covered". + The twelfth story action, in depth order, is the node that says the same + thing and goes on saying it after something in front of it is deleted — so + the translation is a `ROW_NUMBER()` over the story and a lookup at the + cursor's own value. + + Two cases the arithmetic has to survive: + + * **A cursor past the end of the story.** Legitimate — an adventure caught + up under the older rule can have a cursor equal to its action count, and + `run_post_turn` used to clamp it every pass. There is no `rn` to match, + so it falls back to the deepest node there is: still "caught up", which + is what the number meant. + * **A cursor of 0**, which is most adventures. Nothing covered, the column + default already says so, and no row is touched. + + Guarded on `_depth = -1` so a run that dies halfway resumes: every + adventure this has already converted is skipped, and one it has not is + indistinguishable from an untouched row. + """ + sqlite = conn.dialect.name == "sqlite" + story = _story_text_sql("text", sqlite) + for name in ("memory", "summary"): + conn.execute(text(f""" + UPDATE adventures + SET {name}_cursor_branch_id = {_root_branch_of('adventures.id')}, + {name}_cursor_depth = COALESCE( + (SELECT ranked.depth FROM ( + SELECT adventure_id, depth, ROW_NUMBER() OVER ( + PARTITION BY adventure_id ORDER BY depth, id + ) AS rn + FROM actions WHERE {story} + ) AS ranked + WHERE ranked.adventure_id = adventures.id + AND ranked.rn = adventures.{name}_cursor), + (SELECT MAX(a.depth) FROM actions a + WHERE a.adventure_id = adventures.id + AND {_story_text_sql('a.text', sqlite)}), + {NO_DEPTH}) + WHERE adventures.{name}_cursor > 0 + AND adventures.{name}_cursor_depth = {NO_DEPTH} + """)) + + def _get_version(conn) -> int: if conn.dialect.name == "sqlite": return conn.execute(text("PRAGMA user_version")).scalar() or 1 @@ -551,6 +624,8 @@ def bootstrap(engine: Engine) -> None: _backfill_context_snapshot(conn) if version == TREE_BACKFILL_VERSION: _backfill_tree(conn) + if version == CURSOR_ANCHOR_VERSION: + _backfill_cursor_anchors(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 cfe930e..2d2dd16 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -113,9 +113,24 @@ class Adventure(Base): # Phase 6: opt-in per adventure (extra AI calls) auto_summarize: Mapped[bool] = mapped_column(Boolean, default=False) memory_bank_enabled: Mapped[bool] = mapped_column(Boolean, default=False) - # How many actions have already been folded into memories / the story summary. + # LEGACY (Phase 6): how many actions had been folded into memories / the + # story summary, as a *position* in the story. Unread since SP3, and + # unwritten except by a v1 import which is handed one; kept for one release + # so a rollback resumes from a real number, and dropped in SP8 beside + # `actions.index`. The live mark is the anchor pair below. memory_cursor: Mapped[int] = mapped_column(Integer, default=0) summary_cursor: Mapped[int] = mapped_column(Integer, default=0) + # Phase 14, SP3: the same two marks as nodes — (branch, depth) of the last + # action each pass covered. A position slides when an action in front of it + # is deleted and silently starts covering one it has never read; a depth + # does not move, because it is a coordinate along a path rather than an + # offset into a list. NO_DEPTH (-1) is "nothing covered yet", so the first + # block needs no special case. Plain integers, not foreign keys, for the + # same reason `head_branch_id` below is one. See `context/cursors.py`. + memory_cursor_branch_id: Mapped[int | None] = mapped_column(Integer, nullable=True) + memory_cursor_depth: Mapped[int] = mapped_column(Integer, default=-1) + summary_cursor_branch_id: Mapped[int | None] = mapped_column(Integer, nullable=True) + summary_cursor_depth: Mapped[int] = mapped_column(Integer, default=-1) # Phase 14: where the story is being played — which branch, and the depth of # its newest node. Deliberately NOT a ForeignKey: branches.adventure_id # already points this way, and a second constraint back would make the two @@ -225,7 +240,12 @@ class Memory(Base): embedding_blob: Mapped[bytes | None] = mapped_column( LargeBinary, nullable=True, deferred=True ) - # Action index range this memory summarizes (null for manual memories). + # The stretch of story this memory summarizes, as depths on `branch_id` + # (null for a hand-written memory, which summarizes nothing). Written as + # `Action.index` values before SP3, which held the same numbers. + # `source_end` is the depth of the node the memory hangs off, mirrored into + # `depth` below; `source_start` is where it began, which is where the + # summarizer has to resume from if the memory is ever withdrawn. source_start: Mapped[int | None] = mapped_column(Integer, nullable=True) source_end: Mapped[int | None] = mapped_column(Integer, nullable=True) # Phase 14: the node that produced this memory — the last action it diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index dcf67d1..8a8a642 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -10,7 +10,7 @@ from sqlalchemy.orm import Session, load_only, undefer from sqlalchemy.orm.attributes import set_committed_value from .. import auth, images, limits, memorybank, models, schemas, tree, worldstate -from ..context import build_context +from ..context import build_context, cursors from ..context import history as context_history from ..context import lineage from ..database import get_db @@ -1102,19 +1102,18 @@ def undo_turn( preceding = newest[1] if len(newest) > 1 else None # The earliest action removed in this turn holds the pre-turn scoreboard. first_removed = last - memorybank.note_action_removed(adventure, 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.note_action_removed(adventure, first_removed) + 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) - db.flush() # apply deletes so pruning sees the shrunken action list + db.flush() # apply the deletes before anything reads the story back db.expire(adventure, ["actions"]) - memorybank.prune_dangling_memories(adventure, db) # The tip moved back with them. tree.refresh_head(db, adventure) db.commit() @@ -1168,8 +1167,12 @@ def export_adventure( "worldState": adv.world_state, "autoSummarize": adv.auto_summarize, "memoryBankEnabled": adv.memory_bank_enabled, - "memoryCursor": adv.memory_cursor, - "summaryCursor": adv.summary_cursor, + # The bundle's coordinate system is a position in the story, and the + # cursors are nodes now, so they are counted back into one. A v1 bundle + # has to stay readable by builds that never heard of a depth — SP6's v2 + # format carries the anchors themselves. + "memoryCursor": cursors.position_of(adv, cursors.MEMORY.depth(db, adv)), + "summaryCursor": cursors.position_of(adv, cursors.SUMMARY.depth(db, adv)), "memories": [ { "text": m.text, "pinned": m.pinned, "forgotten": m.forgotten, @@ -1312,6 +1315,16 @@ def import_adventure( 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 + # only moment the two coordinate systems can be lined up against each + # other. The legacy columns keep the numbers the bundle gave: they are what + # a rolled-back build would read. + db.flush() + db.expire(adventure, ["actions"]) + cursors.anchor_at_position(adventure, cursors.MEMORY, adventure.memory_cursor) + cursors.anchor_at_position(adventure, cursors.SUMMARY, adventure.summary_cursor) + db.commit() db.refresh(adventure) return adventure @@ -1658,6 +1671,11 @@ def list_memories( # a relationship load takes whole entities, so it picks up whatever the # model happens to carry. `embedding_blob` is deferred and so would stay # out today — this is about the next wide column, not that one. + # + # Adventure-wide, not path-scoped, and that is the split: retrieval reads + # the story being played, the drawer manages the bank. Hiding a branch's + # memories from the drawer would mean memories nobody can find to delete, + # in a phase whose rule is that nothing is ever removed automatically. return ( db.query(models.Memory) .options(load_only(*MEMORY_LIST_COLUMNS)) @@ -1788,13 +1806,13 @@ 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") - # Cursor bookkeeping, same as undo: slide the cursors down if this action - # sits before them, then drop any memory left describing a deleted action. - memorybank.note_action_removed(adventure, action) + # 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) - db.flush() # apply the delete so pruning sees the shrunken action list + db.flush() db.expire(adventure, ["actions"]) - memorybank.prune_dangling_memories(adventure, db) # Deleting the newest action moves the tip; deleting a middle one leaves a # gap in the depths, deliberately — see _backfill_tree. tree.refresh_head(db, adventure) diff --git a/backend/app/tree.py b/backend/app/tree.py index 4e7dc0d..11584cc 100644 --- a/backend/app/tree.py +++ b/backend/app/tree.py @@ -131,6 +131,24 @@ def place_memory( return branch +def attach_memory(memory: models.Memory, node: models.Action) -> None: + """Hang a memory off the node it was derived from. + + The general rule, of which the memory bank is the first instance: anything + derived from the story attaches to the node that produced it, and is then + visible from exactly the paths that node is on. A fork inherits its + ancestors' memories because it inherits their nodes — nothing is copied and + nothing is recreated — and a memory made on a sibling is invisible here + because that node is not on this path. + + Not `place_memory`: this takes the branch from the *node*, which is not + always the head. A block of story can end before the fork the current + branch was made at, and the memory belongs where the ground is. + """ + memory.branch_id = node.branch_id + memory.depth = node.depth + + def place_new_nodes(session: Session) -> None: """Place every unplaced node about to be inserted. Runs on every flush. diff --git a/backend/tests/test_history_window.py b/backend/tests/test_history_window.py index a7b123c..3428138 100644 --- a/backend/tests/test_history_window.py +++ b/backend/tests/test_history_window.py @@ -190,7 +190,7 @@ def test_window_is_ordered_and_free_of_duplicates(story): assert ids == sorted(ids), "window must be oldest-first" -# --------------------------------------------- the cursor arithmetic agrees +# ------------------------------------------- the node-anchored reads agree def test_helpers_agree_with_the_full_list(story): db, adventure, settings = story @@ -204,30 +204,46 @@ def test_helpers_agree_with_the_full_list(story): 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 for probe in (0, 1, ACTION_COUNT // 2, ACTION_COUNT - 1): - target = actions[probe] - expected = next(i for i, a in enumerate(actions) if a.index >= target.index) - assert history.position_of_index(adventure, target.index) == expected + boundary = actions[probe].depth + assert history.count_after(adventure, boundary) == ACTION_COUNT - probe - 1 + assert [a.id for a in history.after(adventure, boundary, 3)] == \ + [a.id for a in actions[probe + 1:probe + 4]] -def test_positions_still_line_up_after_a_middle_action_is_deleted(story): - """The gap in Action.index is exactly what makes positions and indexes - diverge — the case that has broken the cursors twice before.""" +def test_a_depth_boundary_survives_a_middle_action_being_deleted(story): + """The case that has broken the cursors twice before, and the reason they + are depths now. + + A *position* answers "how much story is past this point?" by counting from + the start, so deleting anything in front of the mark changes which action + the mark names. A depth names the same node either way — the only thing + that changes is the count of what comes after, which is what did change. + """ db, adventure, settings = story actions = history.story_actions(adventure) - victim = actions[50] + mark = actions[30].depth + before = history.count_after(adventure, mark) + next_three = [a.id for a in history.after(adventure, mark, 3)] + + victim = actions[10] # in front of the mark db.delete(victim) db.commit() db.expire(adventure) - remaining = history.story_actions(adventure) - assert len(remaining) == ACTION_COUNT - 1 assert history.count(adventure) == ACTION_COUNT - 1 - for probe in (0, 49, 50, 51, ACTION_COUNT - 2): - target = remaining[probe] - expected = next(i for i, a in enumerate(remaining) if a.index >= target.index) - assert history.position_of_index(adventure, target.index) == expected, probe + assert history.count_after(adventure, mark) == before, "the mark moved" + assert [a.id for a in history.after(adventure, mark, 3)] == next_three + + # ...and deleting something *after* it is the one thing that does change + # the count, because that is a fact about the story rather than about the + # coordinate system. + db.delete(history.after(adventure, mark, 1)[0]) + db.commit() + db.expire(adventure) + assert history.count_after(adventure, mark) == before - 1 def test_blank_actions_are_excluded_the_same_way_in_sql_and_python(story): diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py new file mode 100644 index 0000000..1e3ddae --- /dev/null +++ b/backend/tests/test_memory_nodes.py @@ -0,0 +1,416 @@ +"""Phase 14 SP3 — memories hang off nodes, and the marks are nodes too. + +Two claims, and neither of them fails loudly if it is wrong: + +* **A memory belongs to the path that produced it.** A memory made on branch B + must be invisible from A, and the memories of a shared ancestor must be + visible from both — without anything being copied when a fork happens. The + failure mode is a prompt quietly carrying a summary of a story the player + abandoned. +* **Retrieval reads the *whole* lineage, and that stays affordable.** The story + is read through a window, but recall is long-range by definition and cannot + be — so the clause names every ancestor, and the bet is that memories are + sparse enough (one per six actions) for that to be tens of small rows even + twenty forks deep. Measured below rather than asserted. + +Nothing in the product forks yet, so the fork is built by hand, exactly as +`test_branch_clause.py` builds it. + + python -m pytest tests/test_memory_nodes.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 asyncio + +import pytest + +from app import memorybank, models, tree +from app.context import cursors, lineage +from app.database import Base, SessionLocal, engine +from tools import dbmeter + + +class StubEmbedder: + """Returns whatever vector the test set, for any text.""" + + def __init__(self, vector=(1.0, 0.0, 0.0)): + self.vector = list(vector) + + async def embed(self, texts): + return [list(self.vector) for _ in texts] + + +# --------------------------------------------------------------- the fixture + +def make_branch(db, adventure, parent=None, fork_depth=None): + """A branch row whose lineage is its parent's, capped, plus itself — the + computation SP5 will do at fork time, written out so the fixture cannot + pass by agreeing with a bug in the code under test.""" + branch = models.Branch( + adventure_id=adventure.id, + parent_branch_id=parent.id if parent else None, + fork_depth=fork_depth, + lineage=[], + ) + db.add(branch) + db.flush() + inherited = [] + if parent is not None: + for ancestor_id, cap in lineage.entries_of(parent): + capped = fork_depth if cap is None else min(cap, fork_depth) + inherited.append([ancestor_id, capped]) + branch.lineage = [[branch.id, None]] + inherited + db.flush() + return branch + + +def add_node(db, adventure, branch, depth, label, index=None): + action = models.Action( + adventure_id=adventure.id, + index=depth if index is None else index, + branch_id=branch.id, + depth=depth, + type="ai" if depth % 2 else "do", + text=f"{label}{depth}", + ) + db.add(action) + return action + + +def add_memory(db, adventure, text, node, vector=(1.0, 0.0, 0.0), **kwargs): + """A memory of the block ending on `node`, attached the way the post-turn + pass attaches one.""" + memory = models.Memory( + adventure_id=adventure.id, text=text, + source_start=None if node is None else node.depth, + source_end=None if node is None else node.depth, + **kwargs, + ) + if node is not None: + tree.attach_memory(memory, node) + else: + tree.place_memory(db, adventure, memory) + db.add(memory) + db.flush() + memorybank.set_vector(memory, list(vector)) + db.commit() + return memory + + +@pytest.fixture() +def forked(): + """A0..A3, then B4 B5 off A3, then C6 C7 off B5 — with a memory hung off + one node of each branch, and A playing on past the fork it was left at. + + The head is C, so the story is A0 A1 A2 A3 B4 B5 C6 C7 and the memories in + play are A's and B's and C's — but not the one on A5, which is on a sibling + of B4 and belongs to a story nobody is reading. + """ + Base.metadata.create_all(bind=engine) + db = SessionLocal() + user = models.User(is_guest=False, email="nodes@example.com") + db.add(user) + db.flush() + settings = models.Settings( + user_id=user.id, api_key="enc:dummy", model="m", + embedding_model="text-embedding-3-small", memory_top_k=10, + memory_bank_capacity=80, + ) + db.add(settings) + adventure = models.Adventure( + user_id=user.id, title="Forked", script_state={}, memory_bank_enabled=True, + auto_summarize=True, + ) + db.add(adventure) + db.flush() + + a = make_branch(db, adventure) + b = make_branch(db, adventure, parent=a, fork_depth=3) + c = make_branch(db, adventure, parent=b, fork_depth=5) + nodes = {} + for depth in range(4): + nodes[f"A{depth}"] = add_node(db, adventure, a, depth, "A") + for depth in (4, 5): # A kept playing: siblings of B4/B5 + nodes[f"A{depth}"] = add_node(db, adventure, a, depth, "A", index=100 + depth) + for depth in (4, 5): + nodes[f"B{depth}"] = add_node(db, adventure, b, depth, "B") + for depth in (6, 7): + nodes[f"C{depth}"] = add_node(db, adventure, c, depth, "C") + db.flush() + + memories = { + "shared": add_memory(db, adventure, "on the shared trunk", nodes["A3"]), + "sibling": add_memory(db, adventure, "on A's own continuation", nodes["A5"]), + "b": add_memory(db, adventure, "on B", nodes["B5"]), + "c": add_memory(db, adventure, "on C", nodes["C7"]), + } + adventure.head_branch_id = c.id + adventure.head_depth = 7 + db.commit() + + ids = {"a": a.id, "b": b.id, "c": c.id, "nodes": nodes, "memories": memories} + try: + yield db, adventure, settings, ids + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + +def switch_to(db, adventure, branch_id, tip): + adventure.head_branch_id = branch_id + adventure.head_depth = tip + db.commit() + + +def retrieved(adventure, settings) -> set[str]: + memorybank.embedding_provider = lambda s: StubEmbedder() + result = asyncio.run( + memorybank.retrieve_memories(adventure, settings, update_stats=False) + ) + assert result["error"] is None, result["error"] + return {m["text"] for m in result["used"]} + + +# ------------------------------------------------------------- the isolation + +def test_a_memory_on_a_sibling_is_not_retrieved(forked): + """The whole point. A5 is a node of the story that was abandoned when B + forked, and the memory hanging off it must not reach a prompt on C.""" + db, adventure, settings, ids = forked + assert retrieved(adventure, settings) == { + "on the shared trunk", "on B", "on C" + } + + +def test_a_shared_ancestor_is_visible_from_both_branches(forked): + """Nothing is copied at a fork, so the trunk's memories are shared by + construction rather than by duplication.""" + db, adventure, settings, ids = forked + switch_to(db, adventure, ids["a"], 5) + from_a = retrieved(adventure, settings) + assert "on the shared trunk" in from_a + # ...and from A, the branches taken off it are the ones out of reach. + assert from_a == {"on the shared trunk", "on A's own continuation"} + + +def test_the_lineage_is_read_whole_not_windowed(forked): + """The story is read through a window; recall is not. The trunk memory is + four nodes and two forks back, and is still a candidate.""" + db, adventure, settings, ids = forked + path = lineage.path_of(db, adventure) + assert len(path) == 3 + # The window a *story* read would use here names one entry. Retrieval names + # all three, which is the difference this test exists to pin. + assert path.prefix_covering(2) == 1 + assert "on the shared trunk" in retrieved(adventure, settings) + + +def test_a_hand_written_memory_is_not_lost_at_the_first_fork(forked): + """A memory nobody derived summarises no node, so it has a branch but no + depth. A capped `depth <= fork` would drop it the moment its branch stopped + being the newest entry — a memory vanishing some turns after it was typed, + which is exactly the kind of thing nothing reports.""" + db, adventure, settings, ids = forked + switch_to(db, adventure, ids["a"], 5) + typed = add_memory(db, adventure, "typed by hand", None) + assert (typed.branch_id, typed.depth) == (ids["a"], None) + + switch_to(db, adventure, ids["c"], 7) # fork away from where it was written + assert "typed by hand" in retrieved(adventure, settings) + + +# ------------------------------------------------------------------ the marks + +def test_a_mark_moves_to_the_node_the_memory_covers(forked): + """The mark and the memory are one statement about where the pass got to, + so they are written from the same row.""" + db, adventure, settings, ids = forked + cursors.MEMORY.anchor_at(adventure, ids["nodes"]["B5"]) + db.commit() + assert cursors.MEMORY.stored(adventure) == (ids["b"], 5) + assert cursors.MEMORY.depth(db, adventure) == 5 + + +def test_a_mark_from_a_sibling_reads_as_nothing_covered(forked): + """A mark is a node, so moving to another story has to be answered rather + than assumed. Ground this path never travelled is not covered ground, and + the fallback for 'I don't know' has to be redoing the work, not skipping + it.""" + db, adventure, settings, ids = forked + cursors.MEMORY.anchor_at(adventure, ids["nodes"]["C7"]) + db.commit() + switch_to(db, adventure, ids["a"], 5) + assert cursors.MEMORY.depth(db, adventure) == cursors.NO_DEPTH + + +def test_a_mark_on_an_ancestor_is_capped_at_the_fork(forked): + """A6 and A7 are past where this path left A, so a mark deeper than the + fork cannot mean 'covered' for anything on this story.""" + db, adventure, settings, ids = forked + cursors.MEMORY.anchor_at(adventure, ids["nodes"]["A5"]) + db.commit() + assert cursors.MEMORY.depth(db, adventure) == 3 # C forks off B forks off A@3 + + +def test_a_mark_never_moves_forward_on_a_rewind(forked): + db, adventure, settings, ids = forked + cursors.MEMORY.anchor_at(adventure, ids["nodes"]["A3"]) + cursors.rewind_all(adventure, ids["c"], 6) + assert cursors.MEMORY.stored(adventure) == (ids["a"], 3) + + +# ---------------------------------------------------- what the passes read + +def test_the_summary_folds_in_only_the_path_it_is_on(forked, monkeypatch): + """`_update_story_summary` gathers the memories past its mark. On C that is + B's and C's — never the one on A's own continuation, whose depth would + otherwise put it squarely inside the range.""" + db, adventure, settings, ids = forked + monkeypatch.setattr(memorybank, "SUMMARY_INTERVAL", 1) + + class Stub: + def __init__(self): + self.prompts = [] + + async def complete(self, system, user, **kwargs): + self.prompts.append(user) + return "A summary." + + stub = Stub() + monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub) + cursors.SUMMARY.anchor_at(adventure, ids["nodes"]["A3"]) + db.commit() + + asyncio.run(memorybank._update_story_summary(adventure, settings, db)) + + [prompt] = stub.prompts + 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) + + +def test_a_block_is_summarized_from_the_path_and_hung_off_its_last_node( + forked, monkeypatch +): + db, adventure, settings, ids = forked + monkeypatch.setattr(memorybank, "MEMORY_START", 0) + monkeypatch.setattr(memorybank, "MEMORY_INTERVAL", 4) + + class Stub: + def __init__(self): + self.excerpts = [] + + async def complete(self, system, user, **kwargs): + self.excerpts.append(user) + return f"Memory {len(self.excerpts)}." + + stub = Stub() + 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"] + 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 cost of forking + +@pytest.fixture() +def deeply_forked(): + """A story forked twenty times, with a memory every six actions — the + density the post-turn pass actually produces.""" + Base.metadata.create_all(bind=engine) + db = SessionLocal() + user = models.User(is_guest=False, email="deepmem@example.com") + db.add(user) + db.flush() + db.add(models.Settings( + user_id=user.id, api_key="enc:dummy", model="m", + embedding_model="text-embedding-3-small", + # Every candidate is injected, so the measurement covers fetching the + # texts too and not only ranking them. + memory_top_k=50, + )) + + def story(title, forks): + adventure = models.Adventure( + user_id=user.id, title=title, script_state={}, memory_bank_enabled=True, + ) + db.add(adventure) + db.flush() + branch = make_branch(db, adventure) + depth = 0 + nodes = [] + for _ in range(4): + nodes.append(add_node(db, adventure, branch, depth, "n")) + depth += 1 + for _ in range(forks): + branch = make_branch(db, adventure, parent=branch, fork_depth=depth - 1) + for _ in range(2): + nodes.append(add_node(db, adventure, branch, depth, "n")) + depth += 1 + if forks: + branch = make_branch(db, adventure, parent=branch, fork_depth=depth - 1) + for _ in range(84 - depth): + nodes.append(add_node(db, adventure, branch, depth, "n")) + depth += 1 + db.flush() + for node in nodes[5::6]: # one memory per six actions, as the pass makes them + add_memory(db, adventure, f"memory at {node.depth}", node) + adventure.head_branch_id = branch.id + adventure.head_depth = depth - 1 + return adventure + + forked_story = story("Forked", 20) + flat_story = story("Flat", 0) + db.commit() + try: + yield db, flat_story, forked_story + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + +def test_retrieving_from_a_deep_fork_costs_what_a_flat_story_costs(deeply_forked): + """The bet, in bytes. Retrieval names all twenty-two branches instead of + one — but it is fetching an id and a flag per memory, and there are the + same fourteen either way, so the clause is where the difference is and the + clause is not what crosses the wire.""" + db, flat_story, forked_story = deeply_forked + settings = db.query(models.Settings).one() + flat_id, forked_id = flat_story.id, forked_story.id + db.commit() + db.expire_all() + + meter = dbmeter.Meter() + meter.attach(engine) + try: + with meter.scope("flat"): + assert len(retrieved(db.get(models.Adventure, flat_id), settings)) == 14 + flat_bytes = meter.scopes[-1].total.fetched + with meter.scope("forked"): + assert len(retrieved(db.get(models.Adventure, forked_id), settings)) == 14 + forked_bytes = meter.scopes[-1].total.fetched + finally: + meter.detach() + + # Measured 2026-08-18: 1,807 B against 1,823 B — the same fourteen rows, + # named through twenty-two branch terms instead of one. + assert flat_bytes > 0, "the meter saw nothing; it is measuring the wrong connection" + assert forked_bytes < flat_bytes * 1.5, ( + f"retrieval on a 20-fork story cost {forked_bytes:,} B against the " + f"{flat_bytes:,} B a flat story of the same length cost" + ) diff --git a/backend/tests/test_memory_settling.py b/backend/tests/test_memory_settling.py index 26cf80c..fbb4ad9 100644 --- a/backend/tests/test_memory_settling.py +++ b/backend/tests/test_memory_settling.py @@ -1,10 +1,19 @@ -"""Memories must never describe an attempt the player can still retry away. +"""Memories must never describe an attempt the player can still retry away, +and must never skip a stretch of story. 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 -memory cursor has already advanced, so the memory is never regenerated and goes -on describing narration that is no longer in the story. +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. + +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 +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 +assert that the right correction happened. python -m pytest tests/test_memory_settling.py -v """ @@ -20,7 +29,8 @@ os.environ.pop("DATABASE_URL", None) import pytest -from app import memorybank, models +from app import memorybank, models, tree +from app.context import cursors from app.database import Base, SessionLocal, engine @@ -68,6 +78,26 @@ def make_adventure(db, action_count: int) -> models.Adventure: return adventure +def cover(db, adventure, position: int) -> None: + """Mark the first `position` story actions as already summarized. + + Written as a position and translated to the node it names, because that is + what every adventure in the database looked like before SP3 and what a v1 + bundle still carries. `memory_cursor` keeps the old number so the two + coordinate systems can be compared where a test cares. + """ + adventure.memory_cursor = position + adventure.summary_cursor = position + cursors.anchor_at_position(adventure, cursors.MEMORY, position) + cursors.anchor_at_position(adventure, cursors.SUMMARY, position) + db.commit() + + +def covered_depth(db, adventure) -> int: + """The memory mark, as a depth on the story being played.""" + return cursors.MEMORY.depth(db, adventure) + + def run_memories(db, adventure, stub, monkeypatch): monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub) settings = db.query(models.Settings).first() @@ -99,25 +129,23 @@ def test_settled_actions_on_a_one_action_story(db): # ------------------------------------------------------- the bug this prevents def test_memory_never_covers_the_newest_retryable_action(db, monkeypatch): - """cursor=6 with 12 actions is exactly the case that used to bite: the - 6-action block ends on the newest action, which is still retryable.""" + """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.""" adventure = make_adventure(db, 12) - adventure.memory_cursor = 6 - db.commit() + 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 adventure.memory_cursor == 6 + 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) - adventure.memory_cursor = 6 - db.commit() + cover(db, adventure, 6) stub = StubSummarizer() run_memories(db, adventure, stub, monkeypatch) @@ -128,7 +156,10 @@ def test_the_block_lands_a_turn_later_without_the_newest_action(db, monkeypatch) assert "Action 12." not in excerpt # the newest, still retryable memory = db.query(models.Memory).one() assert (memory.source_start, memory.source_end) == (6, 11) - assert adventure.memory_cursor == 12 + # The mark and the memory name the same node — that is what keeps them from + # drifting apart however gappy the depths underneath are. + assert (memory.branch_id, memory.depth) == cursors.MEMORY.stored(adventure) + assert covered_depth(db, adventure) == 11 def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch): @@ -150,21 +181,20 @@ def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch): def test_legacy_caught_up_adventure_is_not_rewound(db, monkeypatch): - """An adventure summarized under the OLD rule can have memory_cursor equal - to its action count. The run_post_turn clamp must use the FULL count, not - the settled one — clamping to settled would rewind the cursor a step and - re-cover an already-summarized action in the next block.""" + """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. + + 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. + """ adventure = make_adventure(db, 12) db.add(models.Memory(adventure_id=adventure.id, text="A", source_start=0, source_end=5)) db.add(models.Memory(adventure_id=adventure.id, text="B", source_start=6, source_end=11)) - adventure.memory_cursor = 12 - adventure.summary_cursor = 12 - db.commit() + cover(db, adventure, 12) - # The clamp as run_post_turn applies it. - count = len(memorybank.story_actions(adventure)) - adventure.memory_cursor = min(adventure.memory_cursor, count) - assert adventure.memory_cursor == 12 # not rewound to 11 + assert covered_depth(db, adventure) == 11 # the newest action, not one past it + assert memorybank.settled_after(adventure, covered_depth(db, adventure)) == -1 # Grow the story and let the next block form. for i in range(12, 25): @@ -191,93 +221,114 @@ def test_no_memories_before_memory_start(db, monkeypatch): # ------------------------------------------- deleting already-summarized ground def orphans(db, adventure) -> list[int]: - """Action indices the cursor calls summarized that no memory describes.""" + """Depths the mark calls summarized that no memory describes. + + The failure this whole section is about, stated once: an action behind the + mark with nothing covering it is never summarized again, and nothing ever + reports it. + """ covered: set[int] = set() for m in db.query(models.Memory).filter_by(adventure_id=adventure.id): covered |= set(range(m.source_start, m.source_end + 1)) - actions = memorybank.story_actions(adventure) - return [a.index for a in actions[: adventure.memory_cursor] if a.index not in covered] + mark = cursors.MEMORY.depth(db, adventure) + return [ + a.depth for a in memorybank.story_actions(adventure) + if a.depth <= mark and a.depth not in covered + ] def summarized_adventure(db): - """13 actions with two memories covering indices 0-11, cursor at 12.""" + """13 actions with two memories covering depths 0-11, the mark on node 11.""" adventure = make_adventure(db, 13) - db.add(models.Memory(adventure_id=adventure.id, text="A", source_start=0, source_end=5)) - db.add(models.Memory(adventure_id=adventure.id, text="B", source_start=6, source_end=11)) - adventure.memory_cursor = 12 - adventure.summary_cursor = 12 - db.commit() + for text, start, end in (("A", 0, 5), ("B", 6, 11)): + node = db.query(models.Action).filter_by( + adventure_id=adventure.id, index=end + ).one() + memory = models.Memory( + adventure_id=adventure.id, text=text, source_start=start, source_end=end + ) + tree.attach_memory(memory, node) + db.add(memory) + cover(db, adventure, 12) db.refresh(adventure) return adventure -def test_deleting_a_middle_action_does_not_skip_a_later_one(db): - """memory_cursor counts positions, so removing an earlier action slides a - never-summarized one into the covered range unless the cursor slides too.""" - adventure = summarized_adventure(db) - victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=5).one() +def test_deleting_a_middle_action_leaves_the_mark_where_it_was(db): + """The bug that motivated the old machinery, and the reason it is gone. - memorybank.note_action_removed(adventure, victim) + A position cursor counted actions from the start, so deleting an earlier + one slid a never-summarized action into the covered range and every delete + had to correct for it. A depth is not a count: node 11 is still node 11 + with node 5 gone. + """ + adventure = summarized_adventure(db) + # Node 4 is inside memory A's block but is not the node it hangs off, so + # nothing is withdrawn — the same reading the old code had, where only a + # memory whose *end* had fallen off the story was pruned. + victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=4).one() + + assert memorybank.forget_node(db, adventure, victim) == 0 db.delete(victim) db.commit() db.refresh(adventure) - assert adventure.memory_cursor == 11 # slid down by one + assert covered_depth(db, adventure) == 11 + assert [m.text for m in db.query(models.Memory).all()] == ["A", "B"] assert orphans(db, adventure) == [] -def test_deleting_a_later_action_leaves_cursors_alone(db): - """Only actions *before* the cursor shift it.""" +def test_deleting_a_later_action_leaves_the_mark_alone(db): adventure = summarized_adventure(db) victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=12).one() - memorybank.note_action_removed(adventure, victim) + memorybank.forget_node(db, adventure, victim) db.delete(victim) db.commit() db.refresh(adventure) - assert adventure.memory_cursor == 12 + assert covered_depth(db, adventure) == 11 assert orphans(db, adventure) == [] -def test_pruning_a_memory_rewinds_to_where_it_started(db): - """Discarding a memory isn't enough — the actions it covered are still - behind the cursor, so they must be handed back to the summarizer.""" - adventure = summarized_adventure(db) - # Delete back past index 11, so memory B (6..11) covers a missing action. - for index in (12, 11): - victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=index).one() - memorybank.note_action_removed(adventure, victim) - db.delete(victim) - db.flush() - db.expire(adventure, ["actions"]) +def test_deleting_a_summarized_node_withdraws_its_memory(db): + """Discarding the memory isn't enough — the story it covered is still + behind the mark, so the mark has to come back to where that block began. - assert memorybank.prune_dangling_memories(adventure, db) == 1 + Memory B ends on node 11, so deleting node 11 is what withdraws it. The old + code found this by scanning for a memory whose covered range had fallen off + the end of the story; the memory hangs off the node now, so it is a lookup. + """ + adventure = summarized_adventure(db) + victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=11).one() + + assert memorybank.forget_node(db, adventure, victim) == 1 + db.delete(victim) db.commit() db.refresh(adventure) assert [m.text for m in db.query(models.Memory).all()] == ["A"] - assert adventure.memory_cursor == 6 # back to where the discarded memory began + assert covered_depth(db, adventure) == 5 # back to where the discarded memory began + assert cursors.SUMMARY.depth(db, adventure) == 5 # and the summary with it assert orphans(db, adventure) == [] def test_repeated_deletes_never_orphan_an_action(db): - """The scenario that motivated this: undo/delete-last, over and over.""" + """The scenario that motivated this: undo/delete-last, over and over. + + No clamp in the loop any more, and no bookkeeping call per delete beyond + withdrawing what the node produced. + """ adventure = summarized_adventure(db) for _ in range(6): actions = memorybank.story_actions(adventure) if not actions: break - victim = max(actions, key=lambda a: a.index) - memorybank.note_action_removed(adventure, victim) + victim = max(actions, key=lambda a: a.depth) + memorybank.forget_node(db, adventure, victim) db.delete(victim) db.flush() db.expire(adventure, ["actions"]) - memorybank.prune_dangling_memories(adventure, db) - count = len(memorybank.story_actions(adventure)) - adventure.memory_cursor = min(adventure.memory_cursor, count) - adventure.summary_cursor = min(adventure.summary_cursor, count) db.commit() db.refresh(adventure) assert orphans(db, adventure) == [] - assert adventure.memory_cursor <= len(memorybank.story_actions(adventure)) diff --git a/backend/tests/test_state_revert.py b/backend/tests/test_state_revert.py index 026c02e..d0eed85 100644 --- a/backend/tests/test_state_revert.py +++ b/backend/tests/test_state_revert.py @@ -140,24 +140,29 @@ def test_undo_prunes_memory_covering_removed_actions(db): assert texts == {"k"} -# ---------------------------------------------------------------- prune helper +# -------------------------------------------------------- withdrawing a node -def test_prune_dangling_memories_counts_and_removes(db): +def test_forget_node_withdraws_only_what_that_node_produced(db): + """Phase 14 SP3: a memory hangs off the node its block ends on, so removing + a node is a lookup rather than a scan for memories that have fallen off the + end of the story.""" user, adv = _make_adventure(db, {}) _add(db, adv, 0, "do") - _add(db, adv, 1, "ai") + second = _add(db, adv, 1, "ai") db.add_all([ - models.Memory(adventure_id=adv.id, text="live", source_start=0, source_end=1), - models.Memory(adventure_id=adv.id, text="dead", source_start=2, source_end=5), + models.Memory(adventure_id=adv.id, text="hangs off node 1", + source_start=0, source_end=1), + models.Memory(adventure_id=adv.id, text="hangs off node 0", + source_start=0, source_end=0), ]) db.commit() - removed = memorybank.prune_dangling_memories(adv, db) + removed = memorybank.forget_node(db, adv, second) db.commit() db.refresh(adv) # expire_on_commit=False: reload the memories collection assert removed == 1 - assert {m.text for m in adv.memories} == {"live"} + assert {m.text for m in adv.memories} == {"hangs off node 0"} # ---------------------------------------------------------------- snapshot diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py index bf8bd77..5b52258 100644 --- a/backend/tests/test_tree_migration.py +++ b/backend/tests/test_tree_migration.py @@ -99,6 +99,13 @@ PRE_TREE_DDL = ( # action never renumbered the ones after it. The gap has to survive as a gap. GAPPED_INDEXES = (0, 1, 2, 4) STRAIGHT_INDEXES = (0, 1) +# "Blank" holds an action whose text is nothing but whitespace. It is a row of +# the adventure but not of the *story*, so a cursor counting covered actions +# never counted it — and migration 56 has to skip it the same way, using a +# frozen copy of the story-text predicate. This is the one duplicated +# definition in the change, so it gets the one case that can tell. +BLANK_INDEXES = (0, 1, 2, 3) +BLANK_AT = 2 @pytest.fixture() @@ -123,11 +130,20 @@ def pre_tree(): "demo_turns_date) VALUES (1, 'v45@example.com', 0, CURRENT_TIMESTAMP, 0, '')" )) + # The cursors as schema 45 held them: counts of covered story actions. + # Gapped's story is 0,1,2,4 — so "3 covered" is the node at depth 2 and + # "4 covered" is the node at depth 4, which is the whole reason a count + # and a depth are not the same number. Straight is caught up past its + # own end (5 covered, 2 actions), which is a state the older rule left + # behind and the clamp used to paper over every post-turn pass. + cursors_at = {"Gapped": (3, 4), "Straight": (5, 0), "Empty": (0, 0), + "Blank": (3, 0)} ids = {} - for name in ("Gapped", "Straight", "Empty"): + for name in ("Gapped", "Straight", "Empty", "Blank"): conn.execute(text( - "INSERT INTO adventures (user_id, title) VALUES (1, :title)" - ), {"title": name}) + "INSERT INTO adventures (user_id, title, memory_cursor, summary_cursor) " + "VALUES (1, :title, :mc, :sc)" + ), {"title": name, "mc": cursors_at[name][0], "sc": cursors_at[name][1]}) ids[name] = conn.execute(text( "SELECT id FROM adventures WHERE title = :title" ), {"title": name}).scalar() @@ -135,14 +151,16 @@ def pre_tree(): for adventure_id, indexes in ( (ids["Gapped"], GAPPED_INDEXES), (ids["Straight"], STRAIGHT_INDEXES), + (ids["Blank"], BLANK_INDEXES), ): for index in indexes: + blank = adventure_id == ids["Blank"] and index == BLANK_AT conn.execute(text( 'INSERT INTO actions (adventure_id, "index", type, text) ' "VALUES (:a, :i, :t, :x)" ), {"a": adventure_id, "i": index, "t": "start" if index == 0 else "do", - "x": f"Turn {index}."}) + "x": " \n\t " if blank else f"Turn {index}."}) # One memory that summarised a block of story, and one written by hand, # which summarised nothing and so belongs to no node. @@ -218,7 +236,7 @@ def test_one_root_branch_per_adventure_with_its_own_lineage(pre_tree): branches = rows( "SELECT id, adventure_id, parent_branch_id, fork_depth, lineage FROM branches" ) - assert len(branches) == 3, "one branch per adventure, including the empty one" + assert len(branches) == 4, "one branch per adventure, including the empty one" for branch_id, _adventure_id, parent, fork_depth, lineage in branches: assert parent is None, "a migrated branch is a root; nothing forked yet" assert fork_depth is None @@ -263,6 +281,54 @@ def test_memories_attach_to_the_node_they_summarised(pre_tree): assert manual and all(depth is None and branch is not None for depth, branch in manual) +def test_the_cursors_become_the_nodes_they_named(pre_tree): + """SP3, migration 56. A count of covered actions and a depth are different + numbers the moment the story has a gap in it, which every adventure anyone + has ever deleted from does.""" + migrations.bootstrap(engine) + + def marks(title): + [row] = rows( + "SELECT memory_cursor_depth, summary_cursor_depth, " + "memory_cursor_branch_id, summary_cursor_branch_id " + "FROM adventures WHERE title = :t", t=title + ) + return row + + # Gapped's story is 0,1,2,4. "3 covered" is the *third* action, at depth 2 — + # reading the count as a depth would have handed the summarizer node 3, + # which does not exist, and quietly skipped node 4 forever. + memory_depth, summary_depth, memory_branch, summary_branch = marks("Gapped") + assert (memory_depth, summary_depth) == (2, 4) + root = scalar( + "SELECT id FROM branches WHERE adventure_id = " + "(SELECT id FROM adventures WHERE title = 'Gapped')" + ) + assert memory_branch == summary_branch == root + + # Straight was caught up under the older rule: 5 covered, 2 actions. There + # is no fifth node to name, and the number meant "caught up", so it lands + # on the tip rather than on nothing. + memory_depth, summary_depth, _, summary_branch = marks("Straight") + assert memory_depth == 1 + assert (summary_depth, summary_branch) == (migrations.NO_DEPTH, None) + + # Nothing covered stays nothing covered, and names no branch. + assert marks("Empty") == (migrations.NO_DEPTH, migrations.NO_DEPTH, None, None) + + # A whitespace-only action is a row but not a story action, so it was never + # counted — "3 covered" of 0,1,[blank],3 is the node at depth 3, not 2. The + # migration's copy of the story-text predicate is the only place that rule + # is written twice, so this is the case that catches it drifting. + assert marks("Blank")[0] == 3 + + # The legacy columns are left exactly as they were: a rolled-back build + # reads them, and this migration is not the one that drops them. + assert rows( + "SELECT memory_cursor, summary_cursor FROM adventures ORDER BY title" + ) == [(3, 0), (0, 0), (3, 4), (5, 0)] # Blank, Empty, Gapped, Straight + + def test_the_branch_clause_index_exists(pre_tree): """SP2's reads are only cheap if this exists — and `create_all` does not add an index to a table it did not create, which is what migration 52 is for.""" @@ -279,7 +345,9 @@ def test_running_it_again_changes_nothing(pre_tree): snapshot = ( rows("SELECT id, branch_id, depth FROM actions ORDER BY id"), rows("SELECT id, adventure_id, lineage FROM branches ORDER BY id"), - rows("SELECT id, head_branch_id, head_depth FROM adventures ORDER BY id"), + rows("SELECT id, head_branch_id, head_depth, memory_cursor_branch_id, " + "memory_cursor_depth, summary_cursor_branch_id, summary_cursor_depth " + "FROM adventures ORDER BY id"), rows("SELECT id, branch_id, depth FROM memories ORDER BY id"), ) @@ -290,11 +358,14 @@ def test_running_it_again_changes_nothing(pre_tree): migrations.bootstrap(engine) with engine.begin() as conn: migrations._backfill_tree(conn) + migrations._backfill_cursor_anchors(conn) assert ( rows("SELECT id, branch_id, depth FROM actions ORDER BY id"), rows("SELECT id, adventure_id, lineage FROM branches ORDER BY id"), - rows("SELECT id, head_branch_id, head_depth FROM adventures ORDER BY id"), + rows("SELECT id, head_branch_id, head_depth, memory_cursor_branch_id, " + "memory_cursor_depth, summary_cursor_branch_id, summary_cursor_depth " + "FROM adventures ORDER BY id"), rows("SELECT id, branch_id, depth FROM memories ORDER BY id"), ) == snapshot diff --git a/backend/tools/stress_session.py b/backend/tools/stress_session.py index 2d64447..188a81b 100644 --- a/backend/tools/stress_session.py +++ b/backend/tools/stress_session.py @@ -138,6 +138,7 @@ from fastapi.testclient import TestClient from sqlalchemy import text from app import auth, limits, memorybank, models, security, seed, tree, worldstate +from app.context import cursors from app.database import Base, SessionLocal, engine, get_db from app.main import app from app.providers import PromptParts @@ -348,8 +349,11 @@ def add_rich_extras(db, args, rng: random.Random, user, adventure) -> None: adventure.scenario_id = scenario.id adventure.world_state = rich_world_state(schema, args.actions) adventure.script_state = rich_script_state(args.actions) - # The post-turn passes SP3 rewrites only do anything when summarization is - # on and the cursors are somewhere other than the start. + # The post-turn passes only do anything when summarization is on and the + # marks are somewhere other than the start. Written as positions here and + # translated into anchors once the actions exist (see build_fixture) — a + # position is what a database being migrated to SP3 still holds, so the + # fixture carries both and they have to say the same thing. adventure.auto_summarize = True adventure.story_summary = RICH_SUMMARY adventure.memory_cursor = max(0, args.actions - 8) @@ -535,6 +539,14 @@ def build_fixture(args, rng: random.Random) -> tuple[int, int]: db.add(memory) if args.rich: + # The memory/summary marks as nodes, translated from the positions + # `add_rich_layers` set now that there are actions to point at — + # through the same call the v1 importer uses. A fixture stamped + # LATEST never meets a migration, so if this is skipped it is the + # one database whose marks are only positions. + db.flush() + cursors.anchor_at_position(adventure, cursors.MEMORY, adventure.memory_cursor) + cursors.anchor_at_position(adventure, cursors.SUMMARY, adventure.summary_cursor) add_second_adventure(db, rng, user) _assert_live_variant_invariant(db, adventure.id) diff --git a/plan/14-phase-story-tree.md b/plan/14-phase-story-tree.md index 2e42d49..7365aba 100644 --- a/plan/14-phase-story-tree.md +++ b/plan/14-phase-story-tree.md @@ -157,7 +157,9 @@ even there only where the change is deliberate and named below. is far shorter. The cap has to count the tree but be explained as the tree, or move. - **The holdback cannot die in SP3.** `settled_story_actions` exists because retry mutates a row in place. Retry stops mutating in SP4, so the holdback is only safe to - delete there — deleting it in SP3 reopens the exact bug it was written for. + delete there — deleting it in SP3 reopens the exact bug it was written for. It survived + SP3 as `memorybank.settled_after`, which is the `- 1` in "how much story is past the + mark"; that subtraction is the whole of it. ## Subphases @@ -356,6 +358,63 @@ on branch B is invisible from branch A, and shared ancestors are visible from bo Memory retrieval reads the *full* lineage (it cannot be windowed) but stays sparse: assert the byte cost on a deep fork. +**Done, 2026-08-18** (branch `sp3-node-cursors`). **330 tests green**, the 318 the branch +started from plus 12 — 11 in `test_branch_clause`'s new sibling `test_memory_nodes.py` +and one on migration 56. The baseline contract passes **unmodified**, which was the pass +condition. `app/context/cursors.py` is the new module; migrations 53–56 add +`memory_cursor_branch_id/_depth` and `summary_cursor_branch_id/_depth` and translate the +old counts into them. + +Seven things worth not rediscovering: + +- **An anchor is a coordinate, not a pointer, and that is what deleted the machinery.** + Half of this subphase was expected to be rewriting the cursor bookkeeping in depth + terms. None of it needed rewriting: `count_after(41)` is well defined with node 41 + deleted, and deleting node 12 does not change what "past node 41" means. So + `note_action_removed`, `_rewind_cursors_to_index`, `position_of_index` and the + post-turn clamp did not become depth-shaped versions of themselves — they became + nothing. **If a mark still needs correcting when the story changes, it is still a + position.** +- **The clamp had its own trap and it also goes.** `run_post_turn` clamped both cursors + to the story length every pass, deliberately against the *full* count, because + clamping to the settled count rewound a caught-up adventure a step and re-covered an + action. An anchor past the tip is not a broken value: `settled_after` reports nothing + to do, and the story growing back past it resumes exactly where it left off. +- **`prune_dangling_memories` became a lookup, and got stricter by accident.** A memory + hangs off the node its block ends on, so "what did this node produce?" is + `(branch_id, depth)` — `memorybank.forget_node`. The scan it replaces could only ever + notice damage *after* the fact (a covered range past `max(index)`), and could not + notice at all when the node was deleted from the middle of a story that still had + later actions. Withdrawing the memory is half the job: the ground it covered is still + behind the mark, so the mark goes back to `source_start - 1` — a depth, whether or not + a row still sits there. +- **A memory with no node had to be spelled out in the clause.** A hand-written memory + summarises nothing, so it carries a branch and a NULL depth. Every ancestor entry in a + lineage clause is capped `depth <= fork`, and NULL fails that — so a typed memory would + have become invisible at the first fork after it was written, with nothing to see but a + prompt that stopped mentioning it. `Path.clause(unanchored=True)` is that case, and + actions never pass it: an action with no depth is a pre-tree row no read should see. +- **Retrieval reads the whole lineage, and it is free.** Measured on two stories of 84 + actions and 14 memories each, one flat and one forked twenty times: **1,807 B against + 1,823 B**. The clause carries 22 branch terms instead of one, and the clause is not what + crosses the wire. The egress shapes are otherwise byte-identical to SP1's — index + 1.8 kB, page load 62.7 kB, turn 733.8 kB. +- **Three reads stay adventure-wide, deliberately.** Embedding and eviction are facts + about the row and about the bank, not about the path — skipping a sibling's memories + would only mean embedding them at the moment somebody switched to them, and evicting the + memories of a story nobody is reading is the right thing to evict first. The Memories + drawer is management rather than retrieval, and hiding a branch's memories there would + make them unfindable in a phase whose rule is that nothing is removed automatically. +- **The v1 bundle still speaks positions, in exactly two places.** Export counts the + anchor back into a position; import translates the other way, but only after the + actions exist, because that is the one moment the two coordinate systems can be lined + up. `cursors.position_of` and `cursors.anchor_at_position` are the whole of what still + knows about positions, and SP6's v2 format retires them. + +Migrations 53–56 rewrite `adventures`, not `actions` — a few hundred rows against a few +hundred thousand — so **this deploy needs no `VACUUM FULL` of its own**. The one SP1 owes +is still owed. + ### SP4 — Variants become sibling nodes Retry stops mutating a row. It writes a sibling leaf at the same depth. @@ -412,7 +471,9 @@ jsdom has no layout, so scroll position still needs eyes. ### SP8 — Drop the legacy columns Only once the tree is proven live. Migration drops `index`, `variants`, `variant_index`, -`variant_count`, followed by `VACUUM FULL actions;`. +`variant_count`, followed by `VACUUM FULL actions;`. **Also `adventures.memory_cursor` +and `summary_cursor`** — unread since SP3, kept only so a rolled-back build resumes from +a real number. They are on `adventures`, so dropping them costs no vacuum. **Verify:** full suite; egress ceilings; a measured before/after size, aggregates only. diff --git a/plan/STATUS.md b/plan/STATUS.md index ee9984c..8f38c32 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -3,7 +3,7 @@ Read this first when picking the project back up. Updated at the end of a working session; the per-phase plan files hold the detail, this holds the thread. -**Last updated: 2026-08-17.** +**Last updated: 2026-08-18.** --- @@ -78,27 +78,32 @@ needed; nothing requires reading a row of anyone's story. ## Pick up here -**`plan/14-phase-story-tree.md`, SP3 — memories and the summary attach to nodes.** SP0 -(the regression contract and the `--rich` fixture), SP1 (schema, migration, and the writer -that keeps new rows on the tree) and SP2 (the branch clause: every action read now selects -on `(branch_id, depth)` through `app/context/lineage.py`) are done and green; nothing is -deployed yet. SP3 turns `memory_cursor`/`summary_cursor` from positions in a shifting list -into node anchors, and deletes the cursor-position machinery that goes with them — -`position_of_index`, `note_action_removed`, `_rewind_cursors_to_index`, -`prune_dangling_memories`. **`settled_story_actions` and the holdback stay until SP4**: -they exist because retry mutates a row in place, and retry stops doing that in SP4, not -in SP3. Deleting them early reopens the exact bug they were written for. +**`plan/14-phase-story-tree.md`, SP4 — variants become sibling nodes.** SP0 (the +regression contract and the `--rich` fixture), SP1 (schema, migration, and the writer that +keeps new rows on the tree), SP2 (the branch clause: every action read selects on +`(branch_id, depth)` through `app/context/lineage.py`) and SP3 (memories hang off nodes, +and both marks are `(branch_id, depth)` through `app/context/cursors.py`) are done and +green; **nothing is deployed yet**. SP4 is where retry stops rewriting a row and writes a +sibling leaf at the same depth instead, where `state_before`/`world_state_before` become +*after* snapshots, and where the legacy `variants` JSON is migrated into rows. It is also +the first subphase allowed to move the baseline test, and only for +`variant_count`/`variant_index` semantics. -**The schema is live in code but not on production.** When SP1 ships, the deploy needs -one `VACUUM FULL actions;` on the direct (non-`-pooler`) endpoint afterwards — it rewrites -every row. See the 144 MB lesson at the top of this file. +**The schema is live in code but not on production.** When this ships, the deploy needs +one `VACUUM FULL actions;` on the direct (non-`-pooler`) endpoint afterwards — SP1's +migration rewrites every row, and SP4's does it again. SP3's own migrations touch +`adventures` only and need no vacuum. See the 144 MB lesson at the top of this file. -Two things to carry into it: +Three things to carry into it: -- **Every action read goes through `context/lineage.py`.** A memory read has to as well — - the same `Path` builds a clause over `Memory` — and it reads the *full* lineage, not a - window: retrieval is long-range recall and cannot be windowed. It stays affordable - because memories are sparse, so assert the byte cost on a deep fork. +- **The holdback dies in SP4 and nowhere earlier.** `settled_story_actions` exists + because retry mutates a row in place; it survived SP3 as the `- 1` inside + `memorybank.settled_after`. Retry stops mutating in SP4, which is the only point at + which removing it does not reopen the bug it was written for. +- **Anything derived attaches to the node that produced it.** A memory now does + (`tree.attach_memory`), and so do both marks. A sibling leaf is a node, so whatever SP4 + derives per attempt hangs off the attempt — and `memorybank.forget_node` is what + withdraws it when the node goes. - **Weigh new columns in bytes.** `actions` is already the table that fills the disk. `tests/test_egress.py` has byte ceilings now — they will tell you. @@ -112,6 +117,39 @@ drive it before rewriting it. --- +## What happened on 2026-08-18 — the tree, SP3 + +The memory bank stopped counting. `memory_cursor` and `summary_cursor` were positions in +the story — "the first twelve actions are covered" — and a position moves when an action +in front of it is deleted, so it silently starts covering one it has never read. Both are +now node anchors, `(branch_id, depth)`, through the new `app/context/cursors.py`; a memory +hangs off the node whose block it ends on; and retrieval selects through the branch +clause, so a memory made on one branch never reaches a prompt on another. **330 tests +green**, and the SP0 baseline still passes unmodified. Branch `sp3-node-cursors`. + +Three things to carry forward: + +- **Most of the work was deleting, and that was the test of the design.** The plan listed + four pieces of cursor machinery to remove and the expectation was that each would come + back in depth-shaped form. None did. `count_after(41)` is well defined with node 41 + deleted and unchanged by anything deleted in front of it, so `note_action_removed`, + `_rewind_cursors_to_index`, `position_of_index` and the every-pass clamp in + `run_post_turn` all became nothing at all. **If a mark still needs correcting when the + story changes, it is still a position.** The one thing a delete still does is withdraw + what the node *produced* — `memorybank.forget_node`, a lookup on `(branch_id, depth)` + where `prune_dangling_memories` was a scan that could only notice damage afterwards. +- **A NULL is not a small depth, and it nearly cost a feature.** A hand-written memory + summarises no node, so it has a branch and no depth; every ancestor entry in a lineage + clause is capped `depth <= fork`, and NULL fails that test. A memory somebody typed + would have disappeared at the first fork after they typed it, with no error anywhere — + just a prompt that stopped mentioning it. `Path.clause(unanchored=True)` names that case + explicitly, and actions are deliberately not given it. +- **Reading the whole ancestry for recall is free.** Retrieval cannot be windowed — that + is the point of it — so the clause names every branch in the lineage. Two 84-action + stories with 14 memories each, one flat and one forked twenty times: **1,807 B against + 1,823 B**. Twenty-two branch terms cost nothing, because the clause is not what crosses + the wire. Index (1.8 kB), page load (62.7 kB) and turn (733.8 kB) are unmoved. + ## What happened on 2026-08-17, part five — the tree, SP2 Every read of an action now goes through one module. `app/context/lineage.py` turns a @@ -407,7 +445,7 @@ the SQLite dev parity this codebase protects on purpose). ``` cd backend -.venv/Scripts/python.exe -m pytest tests/ # 297 tests (~38s) +.venv/Scripts/python.exe -m pytest tests/ # 330 tests (~55s) .venv/Scripts/python.exe -m tools.stress_session # egress report (SQLite) # Same harness against a real Postgres. The target must be a THROWAWAY database