diff --git a/backend/app/context/lineage.py b/backend/app/context/lineage.py index 0d6e609..e19169d 100644 --- a/backend/app/context/lineage.py +++ b/backend/app/context/lineage.py @@ -87,7 +87,6 @@ class Path: self, model=models.Action, count: int | None = None, - unanchored: bool = False, ): """The branch clause, over `model` (`Action` or `Memory`). @@ -95,12 +94,12 @@ class Path: 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. + Every row this reads has a depth. Memories used to be the exception — + a hand-written one had a branch and no depth, and needed an escape + clause here to survive being capped at a fork. SP7 anchors them at the + head instead (`tree.place_memory`), which is a better answer to the same + problem: the memory is not exempt from the path, it is *on* one. A row + with no depth is now a pre-tree leftover that no read should see. Actions also have to be *live* (SP4). A coordinate can hold several attempts at the same turn, and the story tells one of them; the losing @@ -115,19 +114,16 @@ class Path: entries = self.entries if count is None else self.entries[:count] if not entries: return false() - on_path = or_(*[self._entry_clause(model, b, d, unanchored) for b, d in entries]) + on_path = or_(*[self._entry_clause(model, b, d) for b, d in entries]) if model is models.Action: return and_(on_path, models.Action.live.is_(True)) return on_path @staticmethod - def _entry_clause(model, branch_id: int, max_depth: int | None, unanchored=False): + def _entry_clause(model, branch_id: int, max_depth: int | None): if max_depth is None: return model.branch_id == branch_id - within = model.depth <= max_depth - if unanchored: - within = or_(within, model.depth.is_(None)) - return and_(model.branch_id == branch_id, within) + return and_(model.branch_id == branch_id, model.depth <= max_depth) # ------------------------------------------------------------- Python diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index efb93b3..ba42df1 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -229,7 +229,7 @@ async def retrieve_memories( 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), + lineage.path_of(db, adventure).clause(models.Memory), models.Memory.forgotten.is_(False), models.Memory.embedded.is_(True), ) diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 593e751..89ac45f 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -257,6 +257,15 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ # than one per turn, so unlike SP1's and SP4's this rewrite is a few hundred # rows against a few hundred thousand and needs no VACUUM FULL of its own. (61, "ALTER TABLE branches ADD COLUMN name VARCHAR(80)"), + # Phase 14, SP7 — every memory gets a node. A hand-written memory used to + # keep a NULL depth, which no fork could cap, so it followed the reader onto + # branches whose story it never described. New ones anchor at the head; the + # ones already written land at **depth 0 of the branch they are on**, which + # is the only choice that takes nothing away from anybody: 0 is at or before + # every fork point, so a memory stays visible from exactly the paths it is + # visible from today. Anchoring them at the tip instead would have emptied + # them out of every branch forked earlier than they were typed. + (62, "UPDATE memories SET depth = 0 WHERE depth IS NULL"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) diff --git a/backend/app/models.py b/backend/app/models.py index 69c7e01..9f1e7f2 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -261,8 +261,10 @@ class Memory(Base): # automatically, and a memory covering a stretch of branch B is invisible # from any path that does not go through B. # - # `depth` is NULL for a hand-written memory, which no node produced; that - # reads as "belongs to the adventure, not to a path". + # Every memory has one, including a hand-written one: it takes the head at + # the moment it was written (SP7). A NULL depth used to mean "belongs to the + # adventure, not to a path", which is a category no fork could cap — the + # memory followed the reader onto branches whose story it never described. branch_id: Mapped[int | None] = mapped_column( ForeignKey("branches.id", ondelete="CASCADE"), nullable=True ) diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index 93da26f..da8e661 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -4,7 +4,7 @@ import threading from fastapi import APIRouter, Body, Depends, HTTPException, Request from fastapi.responses import StreamingResponse -from sqlalchemy import func, select +from sqlalchemy import func from sqlalchemy.orm import Session, load_only, undefer from sqlalchemy.orm.attributes import set_committed_value @@ -1881,36 +1881,23 @@ def list_memories( # 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. - rows = ( + # **The bank you can see is the bank the model can see.** Filtered by the + # same clause retrieval uses, so the drawer answers one question rather than + # two: an adventure-wide list would show memories from branches this story + # never went down, which are never retrieved, and a reader has no way to + # tell those apart from the ones actually in play. Nothing is stranded by + # this — a memory lives on a branch, so switching to that branch shows it, + # and deleting the branch takes its memories with it. + return ( db.query(models.Memory) .options(load_only(*MEMORY_LIST_COLUMNS)) - .filter(models.Memory.adventure_id == adventure_id) + .filter( + models.Memory.adventure_id == adventure_id, + lineage.path_of(db, adventure).clause(models.Memory), + ) .order_by(models.Memory.id) .all() ) - # ...but listing them alike would be its own lie. A memory on a branch this - # story never travelled is never retrieved, so showing it beside one that is - # tells the player the model remembers something it cannot see. The ids come - # from **the predicate retrieval itself uses**, deliberately: two spellings - # of "on this path" would eventually disagree, and the failure would be a - # badge that says the opposite of what the model gets. - on_path = { - row[0] for row in db.execute( - select(models.Memory.id).where( - models.Memory.adventure_id == adventure_id, - lineage.path_of(db, adventure).clause( - models.Memory, unanchored=True - ), - ) - ) - } - for row in rows: - row.on_path = row.id in on_path - return rows @router.post("/{adventure_id}/memories", response_model=schemas.MemoryOut, status_code=201) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index e43882f..0412b64 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -289,12 +289,6 @@ class MemoryOut(ORMModel): last_used_at: datetime | None source_start: int | None source_end: int | None - # Whether this memory is on the story currently being read (Phase 14, SP7). - # The drawer lists the whole bank so nothing becomes impossible to find and - # delete, but a memory belonging to another branch will never be retrieved - # into context — and a list that showed the two alike would be telling the - # player the model knows something it cannot see. - on_path: bool = True created_at: datetime diff --git a/backend/app/tree.py b/backend/app/tree.py index 5ce0b1e..77fd308 100644 --- a/backend/app/tree.py +++ b/backend/app/tree.py @@ -208,16 +208,27 @@ def place_memory( memory: models.Memory, branch: models.Branch | None = None, ) -> models.Branch: - """Attach a memory to the node that produced it. + """Attach a memory to the node it belongs to. `source_end` is the index of the last action the memory summarises, which is - that node's depth. A hand-written memory summarises nothing, so its depth - stays NULL and it belongs to the adventure rather than to a path. + that node's depth. A hand-written memory summarises nothing, so it takes the + head instead: **the story you were reading when you wrote it.** + + That anchor is what makes a memory mean one thing (SP7). Before it, a + hand-written memory kept a NULL depth and "belonged to the adventure rather + than to a path" — which sounded harmless and meant it followed you onto + branches whose story it did not describe, because a NULL cannot be capped at + a fork. Every memory now sits at a coordinate, so "is this part of the story + I am reading?" has one answer for every row in the bank, and it is the same + answer the lineage already gives for nodes. """ branch = branch or head_branch(db, adventure) memory.branch_id = branch.id - if memory.depth is None and memory.source_end is not None: - memory.depth = memory.source_end + if memory.depth is None: + memory.depth = ( + memory.source_end if memory.source_end is not None + else adventure.head_depth + ) return branch diff --git a/backend/tests/test_branch_forking.py b/backend/tests/test_branch_forking.py index 4109cb1..2168ac0 100644 --- a/backend/tests/test_branch_forking.py +++ b/backend/tests/test_branch_forking.py @@ -443,7 +443,7 @@ def test_a_memory_on_the_line_left_behind_is_out_of_range_on_the_fork(client): path = lineage.path_of(db, adventure) visible = db.query(models.Memory).filter( models.Memory.adventure_id == adventure.id, - path.clause(models.Memory, unanchored=True), + path.clause(models.Memory), ).all() assert visible == [], "a sibling's memory reached this branch" # ...and the mark reads as one depth short of it, so the block is due diff --git a/backend/tests/test_branch_management.py b/backend/tests/test_branch_management.py index c4c975a..ff0f6a8 100644 --- a/backend/tests/test_branch_management.py +++ b/backend/tests/test_branch_management.py @@ -361,38 +361,50 @@ def _add_memory(client, text): return r.json()["id"] -def test_the_drawer_keeps_every_memory_but_says_which_are_off_the_path(client): - """Both halves of the split, in one test, because either alone is a bug. +def test_a_hand_written_memory_is_anchored_where_it_was_written(client): + """It takes the head, so it is a memory *of a story* rather than of an + adventure. A NULL depth is a coordinate no fork can cap.""" + root, forked = _forked(client) + memory_id = _add_memory(client, "Took the other door.") - Listing only the path's memories would leave the rest impossible to find - and delete, in a phase whose rule is that nothing is removed automatically. - Listing them all *alike* would tell the player the model remembers - something that is never retrieved on this branch. + db = SessionLocal() + try: + memory = db.get(models.Memory, memory_id) + adventure = db.get(models.Adventure, client.adv_id) + assert memory.branch_id == forked, "the branch being read" + assert memory.depth is not None, "never NULL again" + assert memory.depth == adventure.head_depth + finally: + db.close() + + +def test_the_drawer_shows_the_path_being_read_and_nothing_else(client): + """The bank you can see is the bank the model can see. + + An adventure-wide list would show memories from branches this story never + went down — which are never retrieved — and a reader cannot tell those from + the ones actually in play. """ root, forked = _forked(client) on_the_fork = _add_memory(client, "Took the other door.") _switch(client, root) on_the_root = _add_memory(client, "Went the long way instead.") - listed = {m["id"]: m for m in _memories(client)} - assert set(listed) == {on_the_fork, on_the_root}, "the whole bank, always" - assert listed[on_the_root]["on_path"] is True - assert listed[on_the_fork]["on_path"] is False, "written on a branch we left" + assert {m["id"] for m in _memories(client)} == {on_the_root}, \ + "the fork's memory is not on this story" - # And it follows the reader rather than being a property of the memory — - # but **asymmetrically**, which is the part worth pinning. A fork borrows - # its parent's story, so a memory written on the parent is on the fork's - # path too. The reverse is not true: the parent never went down the fork. + # Switching to the fork shows its own memory — and the root's, because a + # fork borrows its ancestors up to the point it left them. The relationship + # is asymmetric on purpose; the parent never went down the fork. _switch(client, forked) - listed = {m["id"]: m for m in _memories(client)} - assert listed[on_the_fork]["on_path"] is True - assert listed[on_the_root]["on_path"] is True, "an ancestor's memory is shared" + listed = {m["id"] for m in _memories(client)} + assert on_the_fork in listed + assert on_the_root not in listed, "written after the fork left this branch" -def test_the_off_path_flag_agrees_with_what_retrieval_can_see(client): - """The flag has to come from retrieval's own predicate, not a second - spelling of it. Two spellings would drift, and the failure mode is a badge - claiming the opposite of what the model is actually given.""" +def test_the_drawer_and_retrieval_agree_on_what_is_visible(client): + """One predicate, so a memory can never be listed but unretrievable (or the + reverse). Two spellings of "on this path" would eventually drift.""" root, forked = _forked(client) _add_memory(client, "Took the other door.") _switch(client, root) @@ -401,32 +413,42 @@ def test_the_off_path_flag_agrees_with_what_retrieval_can_see(client): db = SessionLocal() try: adventure = db.get(models.Adventure, client.adv_id) - visible = { + retrievable = { row[0] for row in db.execute( select(models.Memory.id).where( models.Memory.adventure_id == adventure.id, - lineage.path_of(db, adventure).clause( - models.Memory, unanchored=True - ), + lineage.path_of(db, adventure).clause(models.Memory), ) ) } finally: db.close() - flagged = {m["id"] for m in _memories(client) if m["on_path"]} - assert flagged == visible + assert {m["id"] for m in _memories(client)} == retrievable -def test_a_memory_from_a_deleted_branch_is_gone_from_the_drawer(client): - """Not merely off-path — the row goes with the branch, through the cascade.""" +def test_deleting_a_branch_deletes_the_memories_written_on_it(client): + """Not merely out of view — the row goes with the branch, through the + cascade. That is what keeps "the drawer shows only your path" from + stranding anything: a memory you cannot see is on a branch you can still + switch to, and deleting that branch takes it for good.""" root, forked = _forked(client) doomed = _add_memory(client, "Took the other door.") _switch(client, root) - assert doomed in {m["id"] for m in _memories(client)} + + db = SessionLocal() + try: + assert db.get(models.Memory, doomed) is not None, "still on its own branch" + finally: + db.close() assert _delete(client, forked).status_code == 204 - assert doomed not in {m["id"] for m in _memories(client)} + + db = SessionLocal() + try: + assert db.get(models.Memory, doomed) is None, "gone with the branch" + finally: + db.close() # ------------------------------------------------------------------- backup diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py index 32c658e..0ac9965 100644 --- a/backend/tests/test_memory_nodes.py +++ b/backend/tests/test_memory_nodes.py @@ -212,18 +212,50 @@ def test_the_lineage_is_read_whole_not_windowed(forked): 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.""" +def test_a_hand_written_memory_is_anchored_where_it_was_typed(forked): + """SP7: a typed memory takes the head, so it obeys the same rule as a + summarised one. + + It used to carry no depth, which sounded like "belongs to the whole + adventure" and behaved like "cannot be capped at a fork" — it followed the + reader onto branches whose story it never described. Anchoring it makes the + bank answer one question rather than two. + """ 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) + assert (typed.branch_id, typed.depth) == (ids["a"], 5), "the head it was typed at" - switch_to(db, adventure, ids["c"], 7) # fork away from where it was written - assert "typed by hand" in retrieved(adventure, settings) + +def test_a_typed_memory_survives_a_fork_of_the_ground_it_was_typed_on(forked): + """The half of the old behaviour that was right, kept. + + Typed on the shared trunk it is still there after forking away — but + because the fork's path goes through that node, not because the memory was + exempt from being capped. + """ + db, adventure, settings, ids = forked + switch_to(db, adventure, ids["a"], 3) # the trunk B, and so C, branch from + add_memory(db, adventure, "typed on the trunk", None) + + switch_to(db, adventure, ids["c"], 7) + assert "typed on the trunk" in retrieved(adventure, settings) + + +def test_a_typed_memory_does_not_follow_you_onto_a_path_it_is_not_on(forked): + """And the half that was wrong, fixed. + + A5 is A's own continuation past the point B left it, so it is a sibling of + the story C tells — precisely where the `sibling` memory sits, and excluded + for precisely the same reason. Typing rather than summarising buys no + exemption from the path. + """ + db, adventure, settings, ids = forked + switch_to(db, adventure, ids["a"], 5) + add_memory(db, adventure, "typed off the path", None) + + switch_to(db, adventure, ids["c"], 7) + assert "typed off the path" not in retrieved(adventure, settings) # ------------------------------------------------------------------ the marks diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py index 1933b85..ed1b033 100644 --- a/backend/tests/test_tree_migration.py +++ b/backend/tests/test_tree_migration.py @@ -276,10 +276,12 @@ def test_memories_attach_to_the_node_they_summarised(pre_tree): assert depth == source_end, "the memory hangs off the last action it covered" assert branch_id is not None - # A hand-written memory has no node: it gets a branch, but no depth, which - # SP3 reads as belonging to the adventure rather than to a path. + # A hand-written memory summarised no node, so SP7's migration 62 lands it + # at depth 0 of its branch. 0 is at or before every fork point, so it stays + # visible from exactly the paths it was visible from before — anchoring + # takes nothing out of anybody's existing bank. manual = rows("SELECT depth, branch_id FROM memories WHERE source_end IS NULL") - assert manual and all(depth is None and branch is not None for depth, branch in manual) + assert manual and all(depth == 0 and branch is not None for depth, branch in manual) def test_the_cursors_become_the_nodes_they_named(pre_tree): @@ -439,7 +441,13 @@ def test_a_blank_adventure_has_a_branch_before_anything_is_played(client): db.close() -def test_a_hand_written_memory_gets_a_branch_but_no_depth(client): +def test_a_hand_written_memory_is_anchored_at_the_head(client): + """SP7: nothing carries a NULL depth any more. + + On an adventure with no story yet the head is NO_DEPTH (-1), which reads as + "before the first node" and so is in range of every branch — right for a + note written before anything has happened. + """ adventure_id = client.post("/api/adventures", json={}).json()["id"] created = client.post( @@ -450,8 +458,9 @@ def test_a_hand_written_memory_gets_a_branch_but_no_depth(client): db = SessionLocal() try: memory = db.query(models.Memory).filter_by(adventure_id=adventure_id).one() + adventure = db.get(models.Adventure, adventure_id) assert memory.branch_id is not None - assert memory.depth is None + assert memory.depth == adventure.head_depth == -1 finally: db.close() diff --git a/frontend/src/index.css b/frontend/src/index.css index 9f933e5..cc40de0 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -1362,17 +1362,6 @@ button:disabled { opacity: 0.45; cursor: default; transform: none; box-shadow: n border-radius: 999px; padding: 0 8px; } -/* A memory belonging to another branch. Set back rather than hidden: it is - still findable and still deletable, but it is never sent to the AI while - this branch is being read, and a row that looked identical to a live one - would be claiming the opposite. The dashed edge is the tell that carries - even when the badge scrolls out of view. */ -.memory-row.off-path { - opacity: 0.6; - border-style: dashed; - background: transparent; -} -.memory-badge.off-path { color: var(--text-dim); } .script-report { margin-top: 14px; border-top: 1px solid var(--border); padding-top: 8px; } .script-error { color: var(--danger); font-size: 0.85rem; margin-bottom: 4px; white-space: pre-wrap; } diff --git a/frontend/src/pages/Play.jsx b/frontend/src/pages/Play.jsx index 3010457..e1ec388 100644 --- a/frontend/src/pages/Play.jsx +++ b/frontend/src/pages/Play.jsx @@ -337,8 +337,7 @@ function MemoryRow({ memory, onChange, onDelete }) { } return ( -