diff --git a/backend/app/bundle.py b/backend/app/bundle.py index 21066db..bf97c70 100644 --- a/backend/app/bundle.py +++ b/backend/app/bundle.py @@ -32,8 +32,19 @@ the import recomputes them: * `lineage` is a cache of `parent` plus `fork_depth`. Shipping it as well would put a second source of truth for one fact into a file anyone can hand-edit, and the two could then disagree without any read reporting it. -* The head depth is the tip of the head branch, which is a fact about the nodes - that arrived with it. +* The head depth used to be derived the same way — the tip of the head branch, + a fact about the nodes that arrived with it. M3 moved it to the other side of + the rule. Undo no longer deletes, so a story can be read at a position behind + its retained tip, and where the reader stopped is a decision nobody can + recompute from the rows: the same tree exports identically whether the user + undid three turns or none. `headDepth` is therefore written, and an import + that ignored it would silently Redo the story to its newest retained turn, + which is the Phase 0B export finding this milestone exists to close. + +A bundle written before M3 has no `headDepth` key, and there is nothing lost in +that: at the time it was written the head could not sit behind the tip, so the +derived answer *was* the recorded one. Such a file imports by deriving, exactly +as it always did. Every hand-editable coordinate is therefore checked before a row is written, in `plan`, rather than repaired afterwards. An import that fails partway leaves an @@ -117,6 +128,9 @@ def export(db: Session, adventure: models.Adventure) -> dict: # belongs to is the root. `_local` places its nodes there. "branches": [_exported_branch(b, local) for b in branches] or [_ROOT], "headBranch": local.get(adventure.head_branch_id, 0), + # M3. Where the story is being read, which is not always where it + # ends. See the head-depth rule at the top of this module. + "headDepth": adventure.head_depth, "memoryCursor": _exported_anchor(adventure, cursors.MEMORY, local), "summaryCursor": _exported_anchor(adventure, cursors.SUMMARY, local), "memories": [_exported_memory(m, local) for m in adventure.memories], @@ -151,6 +165,15 @@ def _exported_branch(branch: models.Branch, local: dict[int, int]) -> dict: # unnamed tree byte-identical to the one SP6 wrote. if branch.name: out["name"] = branch.name + # M3. A branch the story left behind is a decision too — the depth a + # divergent write departed at — so it goes in the file by the same rule that + # puts the fork points in it. A restored backup that could not tell abandoned + # history from active history would have lost the only thing distinguishing + # them, since every row of both arrives either way. Omitted when the branch + # is active, which keeps a file for a never-undone tree as it was. + if branch.superseded_at is not None: + out["supersededAt"] = branch.superseded_at.isoformat() + out["supersededDepth"] = branch.superseded_depth return out @@ -252,11 +275,16 @@ def plan(bundle: dict, version: str) -> dict: _planned_nodes(bundle, len(branches)) if version == FORMAT else _planned_v1_nodes(bundle) ) + head = _as_index(bundle.get("headBranch"), len(branches), default=0) return { "branches": branches, "nodes": nodes, "memories": _planned_memories(bundle, len(branches)), - "head": _as_index(bundle.get("headBranch"), len(branches), default=0), + "head": head, + # None means the file does not say, which is every version 1 bundle and + # every version 2 bundle written before M3. `_point_the_head` derives it + # then, which is what those files were written expecting. + "headDepth": _planned_head_depth(bundle, branches, nodes, head), # Version 2 records where the derived work reached. Version 1 counted # it, and a count cannot become a node until the nodes exist. See # `settle`. @@ -268,6 +296,58 @@ def plan(bundle: dict, version: str) -> dict: } +def _derived_tip(branches: list[dict], nodes: list[dict], head: int) -> int: + """Returns where the head branch's story ends, which is where a file that + does not state a head depth is opened. + + This is the rule `tree.refresh_head` applies and the one every bundle + written before M3 was exported under: the deepest node that arrived on the + head branch, or, for a branch that carries none of its own, the fork point + it inherits its story up to. + """ + depths = [n["depth"] for n in nodes if n["branch"] == head] + if depths: + return max(depths) + fork_depth = branches[head].get("forkDepth") + return fork_depth if fork_depth is not None else lineage.NO_DEPTH + + +def _planned_head_depth( + bundle: dict, branches: list[dict], nodes: list[dict], head: int +) -> int | None: + """Returns the active head depth the file states, or None if it states none. + + Checked here rather than repaired later, for the reason the module docstring + gives: a coordinate that disagrees with the rows is not a value any read can + be given, and an import that discovers it afterwards has already written + half a tree. + + Two bounds. A head past the retained tip is a file claiming the story is + read somewhere it does not reach — the shape a truncated or hand-edited + export takes, and the one that would silently move the reader forward. + Below, `NO_DEPTH` is the floor, because an adventure with no story at all + sits one step in front of its first node. + + A head *behind* the tip is not an error. It is the whole point: it is a + story the user undid and did not redo, and it must import undone. + """ + stated = bundle.get("headDepth") + if stated is None: + return None + tip = _derived_tip(branches, nodes, head) + if not _is_int(stated) or stated < lineage.NO_DEPTH: + raise HTTPException( + 400, f"The file gives the active head depth as {stated!r}." + ) + if stated > tip: + raise HTTPException( + 400, + f"The file reads its story at depth {stated}, but branch {head} " + f"ends at {tip}.", + ) + return stated + + def _planned_branches(bundle: dict) -> list[dict]: raw = bundle.get("branches") entries = [b for b in raw if isinstance(b, dict)] if isinstance(raw, list) else [] @@ -277,8 +357,9 @@ def _planned_branches(bundle: dict) -> list[dict]: for i, entry in enumerate(entries): parent = entry.get("parent") name = _planned_branch_name(entry, i) + left = _planned_supersession(entry) if parent is None: - specs.append(dict(_ROOT, **({"name": name} if name else {}))) + specs.append(dict(_ROOT, **({"name": name} if name else {}), **left)) continue # A branch may fork only from a branch listed before it. The export # writes them that way, because branches are numbered in creation order @@ -302,10 +383,27 @@ def _planned_branches(bundle: dict) -> list[dict]: specs.append({ "parent": parent, "forkDepth": fork_depth, **({"name": name} if name else {}), + **left, }) return specs +def _planned_supersession(entry: dict) -> dict: + """Returns the branch's disposition as the file gives it, or nothing. + + Both keys or neither. A time with no depth cannot say what was displaced and + a depth with no time is not a record of anything having happened, so a file + carrying one of them is treated as carrying neither rather than half of a + fact — no read depends on these columns, and inventing the missing half + would be the only way to get a wrong answer out of them. + """ + at = _as_time(entry.get("supersededAt")) + depth = entry.get("supersededDepth") + if at is None or not _is_int(depth): + return {} + return {"supersededAt": at, "supersededDepth": depth} + + def _planned_branch_name(entry: dict, i: int) -> str | None: """Returns the name a branch entry carries, or `None` if nobody named it. @@ -479,6 +577,8 @@ def _write_branches( lineage=[], name=spec.get("name"), created_at=models.utcnow(), + superseded_at=spec.get("supersededAt"), + superseded_depth=spec.get("supersededDepth"), ) ).inserted_primary_key[0] entries = [[new_id, None]] @@ -568,22 +668,24 @@ def _write_memories( def _point_the_head( adventure: models.Adventure, story: dict, ids: list[int] ) -> None: - """Sets which branch the story is played on, and how deep it goes. + """Sets which branch the story is played on, and where it is being read. - The branch comes from the file and the depth does not. The tip of a branch is - whatever arrived on it, and a branch with no nodes of its own sits at its - fork point, which is the last node its story contains. That node is borrowed - but it is still the tip. This is the rule `tree.refresh_head` applies, run - here before a flush. + Both come from the file now (M3). A bundle that states its head depth is + opened exactly where its owner left it, undone turns and all, and the turns + past that point arrive as the retained future they were exported as. + + A file that states none is opened at the tip of its head branch — whatever + arrived on it, or, for a branch carrying no nodes of its own, the fork point + that is the last node its story contains. That is the rule + `tree.refresh_head` applies and the only answer a pre-M3 bundle can be given, + because at the time it was written the head could not be anywhere else. """ head = story["head"] adventure.head_branch_id = ids[head] - depths = [n["depth"] for n in story["nodes"] if n["branch"] == head] - if depths: - adventure.head_depth = max(depths) + if story["headDepth"] is not None: + adventure.head_depth = story["headDepth"] return - fork_depth = story["branches"][head].get("forkDepth") - adventure.head_depth = fork_depth if fork_depth is not None else lineage.NO_DEPTH + adventure.head_depth = _derived_tip(story["branches"], story["nodes"], head) def _write_anchors( diff --git a/backend/app/head.py b/backend/app/head.py index 49d33b6..796826e 100644 --- a/backend/app/head.py +++ b/backend/app/head.py @@ -228,6 +228,73 @@ def can_undo(db: Session, adventure: models.Adventure) -> bool: return undo_target(db, adventure) is not None +def displaced_history_under( + db: Session, adventure: models.Adventure, node: models.Action +) -> bool: + """Returns whether story the reader cannot see descends from `node`. + + This is the question an in-place edit has to ask. Editing rewrites one row + and re-evaluates nothing, which is what makes it a correction rather than a + new continuation. That is harmless while everything descending from the row + is on screen: the reader can see what their correction has to stay + consistent with. It stops being harmless the moment a continuation descends + from the row and is *not* on screen, because the edit then silently changes + the words an invisible stretch of story was written from. That is the one + way M3's retained history can be made to contradict itself. + + Refusing is deliberately the whole of the fix. Making such an edit fork, so + the original text and its future stay whole, is + `STORY-BRANCH-SEMANTICS.md` §14-15 — and §15 requires re-evaluating the + state the edited prose implies, which is M5's extraction pass. Neither is + started here. + + The question is asked as one shape rather than two, because the two ways a + descendant becomes invisible turn out to be the same fact. An undone future + sits past the head on this very lineage; a displaced line sits past a fork + on a branch the story left. In both cases there is a live node, deeper than + this one, that descends from it and is not on the path being read — and the + departed branch is usually an *ancestor* of the branch now being read, which + is why "branches other than the active one" is the wrong set to look at. + + Only the deepest live node on each descending branch is examined. Whether a + node is on the read path is monotone in depth: a branch is on the path with + a cap, and a node is visible when its depth is at or under that cap. So if + the deepest one is visible, every shallower one is too, and if it is not, + the answer is already yes. + + A node that is not live has no descendants of its own — a take the story + moved past keeps a continuation only by being forked, and that fork is a + branch this loop asks about anyway — so editing one is always safe. + """ + if not node.live or node.depth is None: + return False + read = lineage.path_of(db, adventure) + branches = ( + db.query(models.Branch) + .filter(models.Branch.adventure_id == adventure.id) + .all() + ) + for branch in branches: + # Uncapped: the question is what this branch's story descends from, not + # how much of it the reader is currently being shown. + if not lineage.Path(lineage.entries_of(branch)).contains(node): + continue + deepest = ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adventure.id, + models.Action.branch_id == branch.id, + models.Action.live.is_(True), + models.Action.depth > node.depth, + ) + .order_by(models.Action.depth.desc(), models.Action.id.desc()) + .first() + ) + if deepest is not None and not read.contains(deepest): + return True + return False + + # ------------------------------------------------------------------ writing def move_to(db: Session, adventure: models.Adventure, depth: int) -> None: diff --git a/backend/app/routers/adventures/__init__.py b/backend/app/routers/adventures/__init__.py index fdc2c48..8941e18 100644 --- a/backend/app/routers/adventures/__init__.py +++ b/backend/app/routers/adventures/__init__.py @@ -40,7 +40,7 @@ from . import ( # noqa: F401 from ... import limits # noqa: F401 `adventures.limits` is patched by tests. from .crud import SNIPPET_MAX, _snippet from .paging import ACTION_PAGE -from .takes import retry_action, undo_turn +from .takes import redo_turn, retry_action, undo_turn from .turns import world_delta_of __all__ = [ @@ -48,6 +48,7 @@ __all__ = [ "SNIPPET_MAX", "_snippet", "limits", + "redo_turn", "retry_action", "router", "undo_turn", diff --git a/backend/app/routers/adventures/actions.py b/backend/app/routers/adventures/actions.py index cd1cfee..6115923 100644 --- a/backend/app/routers/adventures/actions.py +++ b/backend/app/routers/adventures/actions.py @@ -8,7 +8,7 @@ coordinate through `nodes.delete_turn`. from fastapi import Depends, HTTPException from sqlalchemy.orm import Session -from ... import attempts, models, schemas, tree +from ... import attempts, head, models, schemas, tree from ...database import get_db from . import turns @@ -41,6 +41,11 @@ def list_actions( ], total=total, has_more=has_more, + # Every page carries them, not just the newest window: the client reads + # the flags off whichever page arrived last, and scrolling up must not + # be able to grey out a Redo that is still available (M3). + can_undo=head.can_undo(db, adventure), + can_redo=head.can_redo(db, adventure), ) @@ -55,6 +60,27 @@ def update_action( action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") + # An edit rewrites this row and re-evaluates nothing after it, which is what + # makes it a correction rather than a new continuation. That is safe while + # everything descending from the row is on screen, and unsafe the moment + # something descends from it that is not — an undone future, or a line a + # divergence left behind. The reader cannot see that story, so they cannot + # see what their correction has just contradicted (M3). + # + # Refusing is the whole of the fix, deliberately. Making the edit fork, so + # that the original text and its future stay whole, is + # `STORY-BRANCH-SEMANTICS.md` §14-15 — and §15 requires re-evaluating the + # state the edited prose implies, which is M5's extraction pass. Neither is + # started here. What is closed is the one case where the application could + # produce retained history that silently disagrees with itself. + if head.displaced_history_under(db, adventure, action): + raise HTTPException( + 400, + "This turn has a later story that is not on screen — undone, or " + "left behind by a new continuation. Editing it here would change " + "the words that story was written from. Redo to bring it back " + "first, or play the turn again to start a new line from here.", + ) # One row holds one text. Nothing mirrors it now, so nothing else has to be # updated. The edit used to have to be written into the live variant entry # as well, or paging away and back reverted it. diff --git a/backend/app/routers/adventures/bundle_io.py b/backend/app/routers/adventures/bundle_io.py index 8cf3f24..b94dd36 100644 --- a/backend/app/routers/adventures/bundle_io.py +++ b/backend/app/routers/adventures/bundle_io.py @@ -7,7 +7,7 @@ only check ownership and hand the work over. from fastapi import Body, Depends, Request from sqlalchemy.orm import Session -from ... import bundle, limits, models, schemas +from ... import bundle, head, limits, models, schemas from ...database import get_db from .deps import CurrentUser, current_adventure, router @@ -62,7 +62,14 @@ def import_adventure( db.commit() db.refresh(adventure) + # A campaign exported while undone imports undone (M3), so the history + # controls have to be right on the response that opens it — otherwise the + # first thing the reader sees about a story with a retained future is a + # greyed-out Redo. + out = schemas.AdventureOut.model_validate(adventure) + out.can_undo = head.can_undo(db, adventure) + out.can_redo = head.can_redo(db, adventure) # This is not a funnel step. A returning player imports a bundle, so it # says nothing about how far a first-time visitor got. It is counted anyway, # because it is the clearest evidence that anyone uses the export format. - return adventure + return out diff --git a/backend/tests/test_attempt_siblings.py b/backend/tests/test_attempt_siblings.py index 632659c..dd63d55 100644 --- a/backend/tests/test_attempt_siblings.py +++ b/backend/tests/test_attempt_siblings.py @@ -200,18 +200,40 @@ def test_the_assembled_prompt_is_stored_once_per_turn(client): assert len(moved) == 1 and moved != live_holder, "the prompt follows the story" -# ------------------------------------------------------- removing the turn +# ------------------------------------------------- stepping behind the turn -def test_undo_takes_every_attempt_with_it(client): +def test_undo_hides_every_attempt_and_keeps_them_all(client): + """M3 rewrote this test. Undo used to delete the turn, and the assertion was + that it took the whole sibling group with it rather than leaving orphaned + attempts at a coordinate the story no longer reached. + + The group still moves as one, but it moves out of the story rather than out + of the database: one Undo steps behind the turn, so none of its three + attempts is in what the story tells, and all three are still on disk for the + Redo that walks back into them. The old assertion is kept as the second half + — what the story reads — and the row count is the new first half. + """ ScriptedProvider.replies = ["One.", "Two.", "Three."] _play(client) _retry(client) _retry(client) - assert len([a for a in _rows(client.adv_id) if a.type == "ai"]) == 3 + before = _rows(client.adv_id) + assert len([a for a in before if a.type == "ai"]) == 3 r = client.post(f"/api/adventures/{client.adv_id}/undo") assert r.status_code == 200, r.text - assert [a.type for a in _rows(client.adv_id)] == ["start"] + # What the story tells: the opening, and none of the turn's attempts. + assert [a["type"] for a in r.json()["actions"]] == ["start"] + # What it holds: every row that was there before, attempts included. + after = _rows(client.adv_id) + assert len(after) == len(before) + assert {a.id for a in after} == {a.id for a in before} + assert len([a for a in after if a.type == "ai"]) == 3 + # And the group is reachable again, whole, with the same take live. + live_before = [a.id for a in before if a.type == "ai" and a.live] + r = client.post(f"/api/adventures/{client.adv_id}/redo") + assert r.status_code == 200, r.text + assert [a.id for a in _rows(client.adv_id) if a.type == "ai" and a.live] == live_before def test_deleting_a_retried_turn_deletes_its_attempts(client): diff --git a/backend/tests/test_branch_forking.py b/backend/tests/test_branch_forking.py index 5ab2721..4c0f6b3 100644 --- a/backend/tests/test_branch_forking.py +++ b/backend/tests/test_branch_forking.py @@ -432,26 +432,41 @@ def test_a_memory_on_the_line_left_behind_is_out_of_range_on_the_fork(client): # ------------------------------------------------------------------- undo -def test_undo_stops_at_the_fork(client): - """Undoing a turn on a fork must never reach into the branch it forked - from. Those turns belong to that branch's story too.""" +def test_undo_walks_off_a_fork_into_the_story_it_inherits(client): + """M3 rewrote this test, and reversed half of it. + + Undo used to refuse at a fork point, and it had to: it deleted the turns it + stepped over, and the turns before the fork belong to the parent branch's + story as well. Refusing was the only way to stop one branch's Undo from + removing rows another branch was reading. + + Nothing is deleted now, so there is nothing to protect the parent from. A + forked branch inherits the story up to its fork, that inherited story is + part of what this branch tells, and Undo walks back through it like any + other retained history. The floor is the campaign opening, not the fork. + """ discarded = _divergent_story(client) _fork(client, discarded) rows_before = len(_rows(client.adv_id)) r = client.post(f"/api/adventures/{client.adv_id}/undo") assert r.status_code == 200, r.text - # The promoted attempt is removed, and the player action before it - # stays, because that action belongs to the parent and the parent - # still has it. - assert len(_rows(client.adv_id)) == rows_before - 1 - assert _texts(client) == ["You enter a cave.", "> You look around."] + # One Undo steps over a whole turn, so it takes the player's action with the + # reply to it — and that player action is the parent's row, sitting in front + # of the fork. Stepping behind it is a read moving backwards, not a branch + # reaching into another branch's rows: nothing moved either way. + assert len(_rows(client.adv_id)) == rows_before + assert _texts(client) == ["You enter a cave."] - # Nothing is left of this branch's own turns, so undo must refuse - # instead of removing the parent's turns. + # The opening is the floor, and it is the parent's node too. r = client.post(f"/api/adventures/{client.adv_id}/undo") assert r.status_code == 400 - assert "forked from" in r.json()["detail"] + assert "Nothing to undo" in r.json()["detail"] + + # Redo walks back out to where the fork was left, taking the turn whole. + assert client.post(f"/api/adventures/{client.adv_id}/redo").status_code == 200 + assert _texts(client) == ["You enter a cave.", "> You look around.", "Attempt one."] + assert len(_rows(client.adv_id)) == rows_before # ----------------------------------------------------------- the tree view diff --git a/backend/tests/test_head_cursor.py b/backend/tests/test_head_cursor.py new file mode 100644 index 0000000..9cb3afc --- /dev/null +++ b/backend/tests/test_head_cursor.py @@ -0,0 +1,873 @@ +"""M3: the active head, and what moving it costs. + +This file is the acceptance contract for the milestone that stopped Undo from +deleting. Its subject is one invariant and the behaviour that follows from it: + + Undo deletes zero accepted turns. + +Everything else here is a consequence. Redo exists because the turns are still +there. Divergence retires a future rather than removing it. Memory and summary +coverage narrow and widen again as the head moves, without anything being +re-embedded. Export carries where the reader stopped, because that is now a +decision rather than a fact about the newest row. + +The tests are named for the acceptance items they discharge — D01-D10, E01-E04, +I01-I03, I07, L01-L02 in `planning/V1-ACCEPTANCE-TESTS.md` — so a reader can go +from a failing test to the requirement it belongs to without a map. + +The world state is instrumentation here, not the subject. Each scripted reply +banks ten gold, which makes "the state at this position" a number a test can +assert instead of a paragraph it has to interpret. M5 replaces that machinery +with genre-neutral narrative state; these tests then need the *instrumentation* +moved, not the assertions removed, because what they measure is where the story +is being read. + + python -m pytest tests/test_head_cursor.py -v +""" +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient + +from app import limits, models +from app.providers import ProviderError +from app.context import cursors, lineage +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app import auth, tree +from app.routers import adventures + +from fakes import GOLD_PER_TURN, GOLD_SCHEMA, ScriptedProvider, gold_replies + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + setup = SessionLocal() + user = models.User(is_guest=False, email="head@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")) + scenario = models.Scenario(user_id=user.id, title="S", stat_schema=GOLD_SCHEMA) + setup.add(scenario) + setup.flush() + adv = models.Adventure( + user_id=user.id, title="Tavern", scenario_id=scenario.id, + world_state={"player": {"hp": 100, "gold": 0}}, + ) + setup.add(adv) + setup.flush() + setup.add(models.Action(adventure_id=adv.id, type="start", text="You enter the tavern.")) + setup.commit() + adv_id, user_id = adv.id, user.id + setup.close() + + ScriptedProvider.replies = gold_replies() + monkeypatch.setattr(adventures.turns, "OpenAICompatibleProvider", ScriptedProvider) + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + + def _current_user(db=Depends(get_db)): + return db.get(models.User, user_id) + + app.dependency_overrides[auth.get_current_user] = _current_user + c = TestClient(app) + c.adv_id = adv_id + try: + yield c + finally: + app.dependency_overrides.clear() + adventures.turns._active_turns.clear() + Base.metadata.drop_all(bind=engine) + + +# ------------------------------------------------------------------ helpers + +def _play(client, text="look around", type="do", after_id=None): + payload = {"type": type, "text": text} + if after_id is not None: + payload["after_id"] = after_id + r = client.post(f"/api/adventures/{client.adv_id}/actions", json=payload) + assert r.status_code == 200, r.text + return r + + +def _turns(client, count): + for n in range(count): + _play(client, f"turn {n}") + + +def _undo(client): + return client.post(f"/api/adventures/{client.adv_id}/undo") + + +def _redo(client): + return client.post(f"/api/adventures/{client.adv_id}/redo") + + +def _adventure(client) -> dict: + r = client.get(f"/api/adventures/{client.adv_id}") + assert r.status_code == 200, r.text + return r.json() + + +def _texts(client) -> list[str]: + return [a["text"] for a in _adventure(client)["actions"]] + + +def _rows(adv_id) -> list[models.Action]: + """Every action row, story or not, live or not, head or no head.""" + db = SessionLocal() + try: + return ( + db.query(models.Action) + .filter(models.Action.adventure_id == adv_id) + .order_by(models.Action.branch_id, models.Action.depth, models.Action.id) + .all() + ) + finally: + db.close() + + +def _gold(adv_id) -> int: + db = SessionLocal() + try: + adv = db.get(models.Adventure, adv_id) + return (adv.world_state or {}).get("player", {}).get("gold", 0) + finally: + db.close() + + +def _head(adv_id) -> tuple[int, int]: + db = SessionLocal() + try: + adv = db.get(models.Adventure, adv_id) + return adv.head_branch_id, adv.head_depth + finally: + db.close() + + +# ------------------------------------------------------- D01, D02, D03: undo + +def test_d01_one_undo_returns_the_transcript_and_the_state(client): + _turns(client, 2) + assert _gold(client.adv_id) == 2 * GOLD_PER_TURN + told = _texts(client) + + page = _undo(client) + assert page.status_code == 200, page.text + + # Both halves of the turn step back together: a player's action and the + # reply to it are one accepted story step. + assert [a["text"] for a in page.json()["actions"]] == told[:-2] + assert _gold(client.adv_id) == 1 * GOLD_PER_TURN + + +def test_d02_five_consecutive_undos_each_land_where_they_should(client): + _turns(client, 7) + assert _gold(client.adv_id) == 7 * GOLD_PER_TURN + + for step in range(1, 6): + assert _undo(client).status_code == 200 + assert _gold(client.adv_id) == (7 - step) * GOLD_PER_TURN + # Two rows per turn, plus the opening. + assert len(_texts(client)) == 1 + 2 * (7 - step) + + +def test_d03_undo_walks_back_to_the_campaign_opening_and_stops(client): + _turns(client, 7) + + for _ in range(7): + assert _undo(client).status_code == 200 + + assert _texts(client) == ["You enter the tavern."] + assert _gold(client.adv_id) == 0 + # The opening is the floor. There is no pre-campaign position to reach. + refused = _undo(client) + assert refused.status_code == 400 + assert "Nothing to undo" in refused.json()["detail"] + assert _adventure(client)["can_undo"] is False + + +def test_the_m3_invariant_undo_deletes_zero_accepted_turns(client): + """The row count is the milestone. Everything else in this file is a + consequence of it, so it is asserted on its own, over the whole retained + tree rather than over the story being told.""" + _turns(client, 5) + before = [a.id for a in _rows(client.adv_id)] + + for _ in range(5): + assert _undo(client).status_code == 200 + + after = [a.id for a in _rows(client.adv_id)] + assert after == before + assert len(_texts(client)) == 1 # and yet the story is back at its opening + + +# -------------------------------------------------------------- D04, L02: redo + +def test_d04_redo_restores_the_continuation_and_its_state(client): + _turns(client, 4) + whole = _texts(client) + + _undo(client) + _undo(client) + assert _gold(client.adv_id) == 2 * GOLD_PER_TURN + + assert _redo(client).status_code == 200 + assert _gold(client.adv_id) == 3 * GOLD_PER_TURN + assert _redo(client).status_code == 200 + assert _gold(client.adv_id) == 4 * GOLD_PER_TURN + assert _texts(client) == whole + # Nowhere further forward to go, and the control says so. + assert _adventure(client)["can_redo"] is False + assert _redo(client).status_code == 400 + + +def test_l02_state_matches_the_position_in_both_directions(client): + """Each position's state is the snapshot the node left behind, so arriving + from in front of it and arriving from behind it must agree.""" + _turns(client, 5) + going_back = [] + for _ in range(5): + _undo(client) + going_back.append(_gold(client.adv_id)) + + coming_forward = [] + for _ in range(5): + _redo(client) + coming_forward.append(_gold(client.adv_id)) + + assert going_back == [40, 30, 20, 10, 0] + assert coming_forward == [10, 20, 30, 40, 50] + + +# ------------------------------------------------- D05, E01, E04: divergence + +def test_d05_a_new_turn_below_the_head_retires_redo_and_keeps_the_future(client): + _turns(client, 4) + abandoned = {a.id for a in _rows(client.adv_id)} + + _undo(client) + _undo(client) + assert _adventure(client)["can_redo"] is True + + ScriptedProvider.replies = ["A different road.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go the other way") + + # Ordinary Redo cannot walk into the old future any more... + assert _adventure(client)["can_redo"] is False + assert _redo(client).status_code == 400 + # ...and not one row of it was deleted to achieve that. + kept = {a.id for a in _rows(client.adv_id)} + assert abandoned <= kept + # The story now tells the new continuation. + assert _texts(client)[-1].startswith("A different road.") + + +def test_the_departed_branch_records_where_the_story_left_it(client): + """`DATA-MODEL.md` §5 gives a branch a disposition. It is stored as the fact + that produced it — the depth the story left at — and nothing reads it to + decide behaviour, so this test is what makes a divergence observable.""" + _turns(client, 3) + _undo(client) + left_at = _head(client.adv_id)[1] + + _play(client, "elsewhere") + + db = SessionLocal() + try: + superseded = ( + db.query(models.Branch) + .filter( + models.Branch.adventure_id == client.adv_id, + models.Branch.superseded_at.isnot(None), + ) + .all() + ) + assert len(superseded) == 1 + assert superseded[0].superseded_depth == left_at + finally: + db.close() + + +def test_e01_e04_a_fact_from_the_abandoned_future_is_not_current(client): + """E01 and E04 are the same mechanism measured twice: the state that is + current is the one belonging to the position the story is read at, so a + number only the abandoned future ever reached cannot survive a divergence.""" + ScriptedProvider.replies = [ + "You find a purse.\n```state\n{\"player.gold\": 10}\n```", + "You find the hoard.\n```state\n{\"player.gold\": 500}\n```", + ] + _play(client, "search") + _play(client, "keep searching") + assert _gold(client.adv_id) == 510 + + _undo(client) + assert _gold(client.adv_id) == 10 + + ScriptedProvider.replies = ["You leave empty-handed.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go home") + + assert _gold(client.adv_id) == 11, "the hoard belonged to a story this one is not" + + +# ------------------------------------------------------ E02, E03: derived work + +def _attach_memory(adv_id, text, at_depth): + """Writes a memory ending on the live node at `at_depth`, the way the + summarizer would, and anchors the memory cursor there.""" + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adv_id) + node = ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adv_id, + models.Action.depth == at_depth, + models.Action.live.is_(True), + ) + .first() + ) + memory = models.Memory( + adventure_id=adv_id, text=text, source_start=0, source_end=node.depth, + ) + tree.attach_memory(memory, node) + db.add(memory) + cursors.MEMORY.anchor_at(adventure, node) + cursors.SUMMARY.anchor_at(adventure, node) + db.commit() + finally: + db.close() + + +def _retrievable(adv_id) -> set[str]: + """The memories the story can reach, through the clause `memorybank` + retrieves with.""" + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adv_id) + rows = ( + db.query(models.Memory) + .filter( + models.Memory.adventure_id == adv_id, + lineage.path_of(db, adventure).clause(models.Memory), + ) + .all() + ) + return {m.text for m in rows} + finally: + db.close() + + +def test_e02_an_abandoned_memory_is_unreachable_and_still_on_disk(client): + _turns(client, 3) + tip_depth = _head(client.adv_id)[1] + _attach_memory(client.adv_id, "Mara reveals she is a spy.", tip_depth) + assert _retrievable(client.adv_id) == {"Mara reveals she is a spy."} + + _undo(client) + # Negative control: undone, the revelation is not retrievable... + assert _retrievable(client.adv_id) == set() + _play(client, "talk about the weather") + # ...and after diverging it stays unreachable, on a line the story left. + assert _retrievable(client.adv_id) == set() + db = SessionLocal() + try: + assert db.query(models.Memory).count() == 1, "unreachable, not deleted" + finally: + db.close() + + +def test_e02_positive_control_the_memory_returns_on_the_line_it_belongs_to(client): + """The negative control alone would pass if memories were simply broken. + Redo puts the head back on the lineage the memory was written for, and it + must be retrievable again — without having been re-embedded.""" + _turns(client, 3) + _attach_memory(client.adv_id, "Mara reveals she is a spy.", _head(client.adv_id)[1]) + + _undo(client) + assert _retrievable(client.adv_id) == set() + _redo(client) + assert _retrievable(client.adv_id) == {"Mara reveals she is a spy."} + + +def test_e03_a_summary_anchor_cannot_claim_coverage_past_the_head(client): + """E03 without redesigning the summarizer. The anchor says how far the + derived work has read; resolved against a head-capped path it can never + report a stretch in the abandoned future as already covered, so that content + is re-derived for the new line rather than carried into it.""" + _turns(client, 3) + tip_depth = _head(client.adv_id)[1] + _attach_memory(client.adv_id, "Everything up to the reveal.", tip_depth) + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + assert cursors.SUMMARY.depth(db, adventure) == tip_depth + finally: + db.close() + + _undo(client) + _play(client, "a different question") + + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + covered = cursors.SUMMARY.depth(db, adventure) + assert covered < tip_depth, "coverage from the abandoned line was claimed" + finally: + db.close() + + +# ------------------------------------------------ D06-D08: retry and takes + +def test_d06_d08_retry_keeps_the_earlier_take_and_reuses_the_parent_state(client): + ScriptedProvider.replies = gold_replies("Take") + _play(client, "knock") + first = [a for a in _rows(client.adv_id) if a.type == "ai"][0] + + r = client.post(f"/api/adventures/{client.adv_id}/retry") + assert r.status_code == 200, r.text + + ai_rows = [a for a in _rows(client.adv_id) if a.type == "ai"] + assert len(ai_rows) == 2, "the earlier take is retained" + assert first.id in {a.id for a in ai_rows} + # Both takes sit at the same coordinate, which is what makes them takes + # rather than turns, and the state is one turn's worth either way. + assert {a.depth for a in ai_rows} == {first.depth} + assert _gold(client.adv_id) == GOLD_PER_TURN + + +def test_d07_a_retry_from_behind_the_tip_branches_instead_of_amending(client): + """A turn with an accepted future is not a leaf, whatever the capped read + says. Retrying it has to leave that future on the line it was written for, + which is a branch — the same operation `add_take` performs for a turn the + story has already moved past.""" + _turns(client, 3) + _undo(client) + before = {a.id for a in _rows(client.adv_id)} + + r = client.post(f"/api/adventures/{client.adv_id}/retry") + assert r.status_code == 200, r.text + + after = {a.id for a in _rows(client.adv_id)} + assert before <= after, "the retained future kept every row" + db = SessionLocal() + try: + assert db.query(models.Branch).filter_by(adventure_id=client.adv_id).count() == 2 + finally: + db.close() + + +def test_switching_a_take_is_refused_while_a_kept_future_hangs_off_it(client): + """The one operation that cannot be made safe by branching, because it + changes which take is live in place. It reports rather than guesses.""" + ScriptedProvider.replies = gold_replies() + _play(client, "knock") + client.post(f"/api/adventures/{client.adv_id}/retry") + _play(client, "go in") + _undo(client) + + newest = [a for a in _rows(client.adv_id) if a.type == "ai" and a.live][0] + r = client.post( + f"/api/adventures/{client.adv_id}/actions/{newest.id}/variant", json={"index": 0} + ) + assert r.status_code == 400 + assert "undone but kept" in r.json()["detail"] + + +# ---------------------------------------------------------- D09, D10: editing + +def test_d09_d10_replaying_a_turn_forks_and_keeps_the_old_line(client): + """D09 and D10 through the operation the product actually offers. Editing + an earlier turn and replaying it is one endpoint — `takes` — and it is + already the "return to the parent state, then continue differently" shape + the semantics ask for. The plain text edit beside it is a correction to + prose that creates no continuation, and M3 does not change it. + """ + ScriptedProvider.replies = [ + "You accuse her.\n```state\n{\"player.gold\": 10}\n```", + "She draws a knife.\n```state\n{\"player.gold\": 20}\n```", + ] + _play(client, "I accuse Mara of stealing the key.") + _play(client, "wait") + accusation = [a for a in _rows(client.adv_id) if a.type == "do"][0] + old_future = {a.id for a in _rows(client.adv_id)} + + ScriptedProvider.replies = ["She shakes her head.\n```state\n{\"player.gold\": 1}\n```"] + r = client.post( + f"/api/adventures/{client.adv_id}/actions/{accusation.id}/takes", + json={"text": "I quietly ask Mara whether she has seen the key."}, + ) + assert r.status_code == 200, r.text + + # The old future is retained whole... + assert old_future <= {a.id for a in _rows(client.adv_id)} + # ...the edited input is what the story now tells... + told = _texts(client) + assert any("quietly ask Mara" in t for t in told) + assert not any("accuse her" in t for t in told) + # ...and no state from the abandoned line leaked into it. + assert _gold(client.adv_id) == 1 + + +# ------------------------------------------------------------------ L01 + +def test_l01_a_failed_turn_accepts_no_narration_and_strands_no_state(client): + """A provider that fails must not leave the story half-advanced. + + The head does move, by exactly one node, and that is deliberate rather than + a gap in the guarantee: A05 keeps the player's submitted text so it can be + tried again, and the head sits on it. What L01 forbids is an *accepted* + narration with half-written state behind it, a head that has moved past a + turn that did not happen, or earlier story becoming unreachable — so those + are what this asserts. + """ + _turns(client, 2) + branch, depth = _head(client.adv_id) + told = _texts(client) + banked = _gold(client.adv_id) + ai_before = [a.id for a in _rows(client.adv_id) if a.type == "ai"] + + ScriptedProvider.replies = [ProviderError("the model refused")] + client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": "do", "text": "the turn that fails"}) + + # One step, onto the retained player action, on the same branch. + assert _head(client.adv_id) == (branch, depth + 1) + # No narration was accepted, and no state moved with a turn that did not + # finish. + assert [a.id for a in _rows(client.adv_id) if a.type == "ai"] == ai_before + assert _gold(client.adv_id) == banked + # Every earlier turn is still readable, in order, unchanged. + assert _texts(client)[:len(told)] == told + + # And the story still moves: one Undo steps back over the stranded input + # and the position is a complete turn again. + ScriptedProvider.replies = gold_replies() + assert _undo(client).status_code == 200 + assert _texts(client) == told + assert _gold(client.adv_id) == banked + + +# ------------------------------------------------- I01, I02, I03, I07: bundles + +def _export(client) -> dict: + r = client.get(f"/api/adventures/{client.adv_id}/export") + assert r.status_code == 200, r.text + return r.json() + + +def _import(client, bundle) -> dict: + """Imports a bundle and returns the new adventure. + + "A fresh data directory" is an adventure this file has never touched: the + import allocates its own branch rows and its own nodes, and resolves the + file's local branch numbers against them, which is the whole of what the + round trip has to get right. Sharing a database with the original does not + weaken that — the two adventures share no row. + """ + r = client.post("/api/adventures/import", json=bundle) + assert r.status_code == 201, r.text + return r.json() + + +def _story_of(adv_id) -> list[str]: + """The story an imported adventure tells, read through its own head.""" + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adv_id) + rows = ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adv_id, + lineage.path_of(db, adventure).clause(models.Action), + ) + .order_by(models.Action.depth, models.Action.id) + .all() + ) + return [a.text for a in rows] + finally: + db.close() + + +def test_i01_i02_a_campaign_round_trips(client): + _turns(client, 3) + told = _texts(client) + + imported = _import(client, _export(client)) + + assert _story_of(imported["id"]) == told + assert _gold(imported["id"]) == 3 * GOLD_PER_TURN + + +def test_i03_the_bundle_carries_the_history_the_story_no_longer_tells(client): + """A backup that keeps only the active line is not a backup: the retained + future is what Redo and every later recovery feature read.""" + _turns(client, 3) + _undo(client) + _play(client, "another way") + active = _texts(client) + + bundle = _export(client) + assert len(bundle["branches"]) == 2 + imported = _import(client, bundle) + + assert _story_of(imported["id"]) == active + # Every row of the abandoned line arrived too, on its own branch. + db = SessionLocal() + try: + assert ( + db.query(models.Action).filter_by(adventure_id=imported["id"]).count() + == len(_rows(client.adv_id)) + ) + assert db.query(models.Branch).filter_by(adventure_id=imported["id"]).count() == 2 + finally: + db.close() + + +def test_i07_an_undone_head_survives_export_and_import(client): + """The Phase 0B finding this milestone exists to close. Before M3 the head + depth was derived on import from the newest retained row, so a campaign + exported after two Undos came back silently redone to its tip.""" + _turns(client, 5) + whole = _texts(client) + _undo(client) + _undo(client) + undone = _texts(client) + assert len(undone) == len(whole) - 4 + + bundle = _export(client) + assert bundle["headDepth"] == _head(client.adv_id)[1] + + imported = _import(client, bundle) + + # It opens exactly where it was left... + assert _story_of(imported["id"]) == undone + assert imported["can_redo"] is True + # ...the later turns arrived as retained history... + db = SessionLocal() + try: + assert ( + db.query(models.Action).filter_by(adventure_id=imported["id"]).count() + == len(_rows(client.adv_id)) + ) + finally: + db.close() + # ...and Redo still walks forward into them, twice, to the same story. + for _ in range(2): + r = client.post(f"/api/adventures/{imported['id']}/redo") + assert r.status_code == 200, r.text + assert _story_of(imported["id"]) == whole + + +def test_a_bundle_written_before_m3_opens_at_its_tip(client): + """Backward compatibility. A file with no `headDepth` is one written when + the head could not be anywhere but the tip, so deriving it is not a + fallback — it is the position that file recorded.""" + _turns(client, 3) + told = _texts(client) + bundle = _export(client) + del bundle["headDepth"] + + imported = _import(client, bundle) + + assert _story_of(imported["id"]) == told + + +def test_a_bundle_that_reads_past_its_own_story_is_refused(client): + """Checked in `plan`, before a row is written, for the reason the module + docstring gives: an import that discovers a bad coordinate afterwards has + already written half a tree.""" + _turns(client, 2) + bundle = _export(client) + bundle["headDepth"] = 999 + + r = client.post("/api/adventures/import", json=bundle) + assert r.status_code == 400 + assert "ends at" in r.json()["detail"] + + +def test_the_bundle_carries_which_branches_the_story_left(client): + """Every row of an abandoned line arrives on an import either way, so the + disposition is the only thing that distinguishes it from an active one. + Losing it on a round trip would leave a restored backup unable to tell them + apart — which is what the later cleanup and recovery features select on.""" + _turns(client, 3) + _undo(client) + _play(client, "another way") + + imported = _import(client, _export(client)) + + db = SessionLocal() + try: + left = ( + db.query(models.Branch) + .filter( + models.Branch.adventure_id == imported["id"], + models.Branch.superseded_at.isnot(None), + ) + .all() + ) + origin = ( + db.query(models.Branch) + .filter( + models.Branch.adventure_id == client.adv_id, + models.Branch.superseded_at.isnot(None), + ) + .all() + ) + assert len(left) == len(origin) == 1 + assert left[0].superseded_depth == origin[0].superseded_depth + finally: + db.close() + + +def test_a_bundle_with_half_a_disposition_imports_as_active(client): + """Neither column is read to decide anything, so a file carrying one of them + is taken as carrying neither rather than having its missing half invented.""" + _turns(client, 3) + _undo(client) + _play(client, "another way") + bundle = _export(client) + for branch in bundle["branches"]: + branch.pop("supersededDepth", None) + + imported = _import(client, bundle) + + db = SessionLocal() + try: + assert ( + db.query(models.Branch) + .filter( + models.Branch.adventure_id == imported["id"], + models.Branch.superseded_at.isnot(None), + ) + .count() + == 0 + ) + finally: + db.close() + + +# ------------------------------------- the narrator-edit guard (M3 closeout) + +def _edit(client, action_id, text): + return client.patch( + f"/api/adventures/{client.adv_id}/actions/{action_id}", json={"text": text} + ) + + +def _live_ai_at(adv_id, depth): + return [ + a for a in _rows(adv_id) + if a.type == "ai" and a.live and a.depth == depth + ][0] + + +def test_editing_a_turn_with_nothing_after_it_is_still_allowed(client): + """The guard has to stay out of the way of the operation it protects. A + correction to the newest turn contradicts nothing, and that is the case the + edit control exists for.""" + _turns(client, 2) + newest = [a for a in _rows(client.adv_id) if a.type == "ai"][-1] + + r = _edit(client, newest.id, "Corrected.") + + assert r.status_code == 200, r.text + assert _texts(client)[-1] == "Corrected." + + +def test_editing_a_turn_on_the_visible_story_is_still_allowed(client): + """Mid-story is not by itself unsafe: everything descending from the turn is + on screen, so the reader can see what their correction has to agree with. + Making that case fork is `STORY-BRANCH-SEMANTICS.md` §14-15 and belongs to + M5 with the state re-evaluation it needs.""" + _turns(client, 3) + older = [a for a in _rows(client.adv_id) if a.type == "ai"][0] + + r = _edit(client, older.id, "Mara wears a green cloak.") + + assert r.status_code == 200, r.text + assert "Mara wears a green cloak." in _texts(client) + + +def test_editing_a_turn_with_an_undone_future_is_refused(client): + """The first unsafe case: the story past the head is not on screen, so an + edit here would silently change the words it was written from.""" + _turns(client, 3) + _undo(client) + at_head = [a for a in _rows(client.adv_id) if a.type == "ai" and a.live] + target = sorted(at_head, key=lambda a: a.depth)[-2] + before = target.text + + r = _edit(client, target.id, "Something else entirely.") + + assert r.status_code == 400 + assert "not on screen" in r.json()["detail"] + # Refused, not partially applied. + assert _rows(client.adv_id)[0].adventure_id == client.adv_id + assert [a.text for a in _rows(client.adv_id) if a.id == target.id] == [before] + + +def test_editing_a_turn_a_divergence_left_behind_is_refused(client): + """The second unsafe case, and the one a head check alone would miss: after + a divergence the head is back at a tip, but a displaced line still runs on + past the shared turn.""" + _turns(client, 3) + shared = [a for a in _rows(client.adv_id) if a.type == "ai"][0] + _undo(client) + _undo(client) + _play(client, "a different road") + assert _adventure(client)["can_redo"] is False, "the head is at a tip again" + + r = _edit(client, shared.id, "Rewritten under both lines.") + + assert r.status_code == 400 + assert "left behind by a new continuation" in r.json()["detail"] + + +def test_a_turn_the_displaced_line_does_not_descend_from_is_still_editable(client): + """The guard must be narrow. A branch that forked *before* a turn does not + descend from it, so a correction there contradicts nothing on that line.""" + _turns(client, 2) + _undo(client) + _play(client, "a different road") # forks below turn 1 + _turns(client, 2) # and continues past the fork + newest = [a for a in _rows(client.adv_id) if a.type == "ai" and a.live] + target = sorted(newest, key=lambda a: a.depth)[-1] + + r = _edit(client, target.id, "Corrected on the live line.") + + assert r.status_code == 200, r.text + + +def test_a_take_that_is_not_live_stays_editable(client): + """A take the story is not telling has no continuation of its own — keeping + one is what forking is for — so correcting its text cannot contradict + anything.""" + ScriptedProvider.replies = gold_replies() + _play(client, "knock") + client.post(f"/api/adventures/{client.adv_id}/retry") + discarded = [a for a in _rows(client.adv_id) if a.type == "ai" and not a.live][0] + + r = _edit(client, discarded.id, "The take nobody chose, corrected.") + + assert r.status_code == 200, r.text + + +def test_the_guard_lifts_when_the_story_is_brought_back(client): + """Refusal is a redirection, not a dead end: the error names Redo, so Redo + has to make the edit possible again.""" + _turns(client, 3) + _undo(client) + live = sorted( + [a for a in _rows(client.adv_id) if a.type == "ai" and a.live], + key=lambda a: a.depth, + ) + target = live[-2] + assert _edit(client, target.id, "x").status_code == 400 + + _redo(client) + + r = _edit(client, target.id, "Corrected once the story was whole again.") + assert r.status_code == 200, r.text diff --git a/backend/tests/test_state_revert.py b/backend/tests/test_state_revert.py index dec9e08..2225766 100644 --- a/backend/tests/test_state_revert.py +++ b/backend/tests/test_state_revert.py @@ -62,6 +62,39 @@ def _add(db, adv, index, type_, text="x", state_after=None): return a +def _row_count(db, adv): + """Every action row the adventure holds, live or not, head or no head.""" + return db.query(models.Action).filter_by(adventure_id=adv.id).count() + + +def _all_types(db, adv): + """The retained story in depth order, which is not the same as the story + being told once the head has moved back behind the tip.""" + rows = ( + db.query(models.Action) + .filter_by(adventure_id=adv.id) + .order_by(models.Action.depth, models.Action.id) + .all() + ) + return [a.type for a in rows] + + +def _retrievable(db, adv): + """The memories the story can currently reach, read through the same clause + `memorybank` retrieves with — which is capped at the active head.""" + from app.context import lineage + + rows = ( + db.query(models.Memory) + .filter( + models.Memory.adventure_id == adv.id, + lineage.path_of(db, adv).clause(models.Memory), + ) + .all() + ) + return {m.text for m in rows} + + def _forget_snapshots(db, adv): """Blank every outcome, the way a row written before SP4 looks. @@ -81,16 +114,31 @@ def test_undo_reverts_state_to_before_the_turn(db): # A turn moved world_state from {gold:0} to {gold:10}. The node in # front of the turn records where it started. The current state is # the mutated one. + # + # M3 rewrote what the second half of this test asserts. Undo used to delete + # the turn, so the story was short afterwards because the rows were gone. + # It now moves the head, so the story is short because it is being read + # from somewhere earlier — and the rows are all still there. The state + # assertion is unchanged, because `attempts.restore_state` is unchanged: + # the state still comes off the node the story now ends on. user, adv = _make_adventure(db, {"gold": 10}) _add(db, adv, 0, "start", state_after={"gold": 0}) _add(db, adv, 1, "do", state_after={"gold": 0}) _add(db, adv, 2, "ai", state_after={"gold": 10}) db.commit() + before = _row_count(db, adv) - adventures.undo_turn(adv.id, db=db, adventure=adv) + page = adventures.undo_turn(adv.id, db=db, adventure=adv) assert adv.world_state == {"gold": 0} - assert [a.type for a in adv.actions] == ["start"] + # What the story now tells. + assert [a.type for a in page.actions] == ["start"] + assert adv.head_depth == 0 + # What it still holds. Zero accepted turns deleted, which is the M3 + # invariant this file is the closest test to. + assert _row_count(db, adv) == before + assert _all_types(db, adv) == ["start", "do", "ai"] + assert page.can_redo is True def test_undo_of_bare_continue_uses_the_node_in_front(db): @@ -101,10 +149,30 @@ def test_undo_of_bare_continue_uses_the_node_in_front(db): _add(db, adv, 1, "ai", state_after={"gold": 5}) db.commit() - adventures.undo_turn(adv.id, db=db, adventure=adv) + page = adventures.undo_turn(adv.id, db=db, adventure=adv) assert adv.world_state == {"gold": 0} - assert [a.type for a in adv.actions] == ["start"] + assert [a.type for a in page.actions] == ["start"] + assert _all_types(db, adv) == ["start", "ai"] + + +def test_redo_puts_back_the_state_the_turn_left_behind(db): + """The other half of the same mechanism: undo and redo restore the same + snapshot from opposite directions, because it belongs to the node rather + than to the direction of travel.""" + user, adv = _make_adventure(db, {"gold": 10}) + _add(db, adv, 0, "start", state_after={"gold": 0}) + _add(db, adv, 1, "do", state_after={"gold": 0}) + _add(db, adv, 2, "ai", state_after={"gold": 10}) + db.commit() + + adventures.undo_turn(adv.id, db=db, adventure=adv) + page = adventures.redo_turn(adv.id, db=db, adventure=adv) + + assert adv.world_state == {"gold": 10} + assert [a.type for a in page.actions] == ["start", "do", "ai"] + assert adv.head_depth == 2 + assert page.can_redo is False def test_undo_leaves_state_untouched_when_snapshot_missing(db): @@ -147,20 +215,45 @@ def test_undo_blocked_by_active_turn_lock(db): adventures.turns._active_turns.discard(adv.id) -def test_undo_prunes_memory_covering_removed_actions(db): +def test_undo_stops_retrieving_a_memory_without_deleting_it(db): + """M3 rewrote this test. Undo used to prune the memories covering the turns + it deleted, because those turns were gone and a summary of them described + story the adventure no longer had. + + Nothing is deleted now, and nothing needs pruning either. A memory carries + the coordinate of the node its block ends on, so one derived from a turn + that is now past the head falls outside the head-capped path clause and + stops being retrievable — and becomes eligible again on Redo, without having + been deleted and re-embedded. That is `STORY-BRANCH-SEMANTICS.md` §33 + holding as a consequence of the head rather than as its own mechanism. + """ user, adv = _make_adventure(db, {}) for i in range(4): _add(db, adv, i, "ai" if i % 2 else "do", state_after={}) - # A memory summarizing actions up to index 3, which undo will delete. - covering = models.Memory(adventure_id=adv.id, text="m", source_start=0, source_end=3) - keep = models.Memory(adventure_id=adv.id, text="k", source_start=0, source_end=1) + db.commit() + # A memory ending on the turn undo will step behind, and one ending before + # it. The coordinate is what the clause reads; `source_*` only says which + # stretch the summarizer covered. + covering = models.Memory( + adventure_id=adv.id, text="m", source_start=0, source_end=3, + branch_id=adv.head_branch_id, depth=3, + ) + keep = models.Memory( + adventure_id=adv.id, text="k", source_start=0, source_end=1, + branch_id=adv.head_branch_id, depth=1, + ) db.add_all([covering, keep]) db.commit() - adventures.undo_turn(adv.id, db=db, adventure=adv) # removes indexes 2 & 3 + adventures.undo_turn(adv.id, db=db, adventure=adv) # head moves to depth 1 - texts = {m.text for m in adv.memories} - assert texts == {"k"} + assert _retrievable(db, adv) == {"k"} + # Still on disk, still embedded, still attached to the adventure. + assert {m.text for m in adv.memories} == {"k", "m"} + + adventures.redo_turn(adv.id, db=db, adventure=adv) + + assert _retrievable(db, adv) == {"k", "m"} # -------------------------------------------------------- withdrawing a node diff --git a/frontend/src/api.js b/frontend/src/api.js index 6599428..5895213 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -124,6 +124,9 @@ export const api = { exportAdventure: (id) => request(`/adventures/${id}/export`), importAdventure: (bundle) => request('/adventures/import', { method: 'POST', body: JSON.stringify(bundle) }), undo: (advId) => request(`/adventures/${advId}/undo`, { method: 'POST' }), + // Undo moves the story back without deleting it, so there is somewhere to + // move forward to again (M3). Both answer with the newest window. + redo: (advId) => request(`/adventures/${advId}/redo`, { method: 'POST' }), getAdventureContext: (advId) => request(`/adventures/${advId}/context`), // "Update from scenario": GET describes what would change, POST applies it. previewRefresh: (advId) => request(`/adventures/${advId}/refresh`), diff --git a/frontend/src/pages/Play/index.jsx b/frontend/src/pages/Play/index.jsx index 2bc71f0..0eff45e 100644 --- a/frontend/src/pages/Play/index.jsx +++ b/frontend/src/pages/Play/index.jsx @@ -104,6 +104,12 @@ export default function Play() { const [total, setTotal] = useState(0) const [hasMore, setHasMore] = useState(false) const [loadingOlder, setLoadingOlder] = useState(false) + // Whether Undo and Redo have anywhere to go, as the server last reported it + // (M3). Neither is derivable here: Undo stops at the campaign opening, which + // may be off the top of the loaded window, and Redo depends on the retained + // future, which the transcript is never sent. So the flags ride on every + // window the server hands back and this holds the newest answer. + const [history, setHistory] = useState({ undo: false, redo: false }) const storyEndRef = useRef(null) const abortRef = useRef(null) const pinnedRef = useRef(true) // autoscroll only while the reader is at the bottom @@ -155,10 +161,19 @@ export default function Play() { setActions(adv.actions) setTotal(adv.action_count ?? adv.actions.length) setHasMore(adv.actions.length < (adv.action_count ?? adv.actions.length)) + // A campaign closed while undone reopens undone, so the controls have + // to be told before the reader can press one. + setHistory({ undo: !!adv.can_undo, redo: !!adv.can_redo }) }) .catch(() => navigate('/')) }, [id, navigate]) + // Record what the server says the history controls can do. Every response + // that carries a window carries them, so this is called wherever one lands. + const noteHistory = useCallback((page) => { + setHistory({ undo: !!page.can_undo, redo: !!page.can_redo }) + }, []) + // Take the story the server just handed back, whole. // // Switching a branch and forking one both answer with the newest window of @@ -173,11 +188,12 @@ export default function Play() { setActions(page.actions) setTotal(page.total) setHasMore(page.has_more) + noteHistory(page) // The world state comes back to what that branch's tip left // behind, so anything drawn from them is now showing another line's // numbers until it re-reads. setStateKey((k) => k + 1) - }, []) + }, [noteHistory]) // Fetch the page above the one on screen and prepend it. // @@ -216,6 +232,7 @@ export default function Play() { } setTotal(page.total) setHasMore(page.has_more) + noteHistory(page) } catch { // Leave hasMore alone: a failed fetch should let the reader try again // by scrolling, not permanently hide the rest of their story. But hold @@ -227,7 +244,7 @@ export default function Play() { loadingOlderRef.current = false setLoadingOlder(false) } - }, [actions, hasMore, id]) + }, [actions, hasMore, id, noteHistory]) // Put the viewport back after a prepend. useLayoutEffect, not useEffect: // this has to run before the browser paints, or the reader sees the story @@ -277,6 +294,12 @@ export default function Play() { setReasoningStream(null) setActions((prev) => [...prev, event.action]) setTotal((n) => n + 1) + // A turn was accepted, so there is something to undo and nothing left to + // redo: writing below a moved-back head is the divergence that retires + // the old future, and writing at the tip never had a future in front of + // it. Set here rather than fetched, because the turn arrives over SSE and + // this is the same answer the server would give. + setHistory({ undo: true, redo: false }) } else if (event.type === 'error') { setStreaming(null) setReasoningStream(null) @@ -363,30 +386,42 @@ export default function Play() { }) } - async function undo() { + // Undo and Redo differ only in which way the head moves, so they share + // everything else: the same window shape comes back, the same reader state is + // dropped, and the same world state has been restored underneath. + async function moveHead(call) { setToast(null) setPreview(null) setPinned(null) try { - // A window, not the whole story — undo is the action most likely to be - // repeated several times running, so it must not re-fetch everything. - const page = await api.undo(id) + // A window, not the whole story — these are the actions most likely to be + // repeated several times running, so they must not re-fetch everything. + const page = await call(id) setActions(page.actions) setTotal(page.total) setHasMore(page.has_more) + noteHistory(page) + // The story is at a different position, so the state panels are showing + // the numbers of a turn that is no longer where the story ends. + setStateKey((k) => k + 1) } catch (err) { setToast({ text: err.message, isError: true }) } } - // Ctrl+Z undo / Ctrl+R retry, ignored while typing in a field. + const undo = () => moveHead(api.undo) + const redo = () => moveHead(api.redo) + + // Ctrl+Z undo / Ctrl+Shift+Z redo / Ctrl+R retry, ignored while typing. useEffect(() => { const lastIsAi = actions.length > 0 && actions[actions.length - 1].type === 'ai' - const canUndo = actions.length > 0 && actions[actions.length - 1].type !== 'start' const onKey = (e) => { if (!(e.ctrlKey || e.metaKey) || busy) return if (e.target.closest?.('input, textarea, select, [contenteditable]')) return - if (e.key.toLowerCase() === 'z' && canUndo) { + if (e.key.toLowerCase() === 'z' && e.shiftKey && history.redo) { + e.preventDefault() + redo() + } else if (e.key.toLowerCase() === 'z' && !e.shiftKey && history.undo) { e.preventDefault() undo() } else if (e.key.toLowerCase() === 'r' && lastIsAi) { @@ -467,6 +502,7 @@ export default function Play() { setActions(adv.actions) setTotal(adv.action_count ?? adv.actions.length) setHasMore(adv.actions.length < (adv.action_count ?? adv.actions.length)) + setHistory({ undo: !!adv.can_undo, redo: !!adv.can_redo }) // The branch and the world state can both have moved. setStateKey((k) => k + 1) } catch { /* stale beats wrong */ } @@ -490,7 +526,6 @@ export default function Play() { if (!adventure) return null const lastIsAi = actions.length > 0 && actions[actions.length - 1].type === 'ai' - const canUndo = actions.length > 0 && actions[actions.length - 1].type !== 'start' return (
@@ -673,7 +708,12 @@ export default function Play() {
- + + {/* Undo keeps what it steps back over, so this walks the story + forward into it again. Disabled once a new turn has been written + from here: that future is retained, but it is no longer the + continuation this story tells. */} +