From 811368d048a56f58c5abb222f3105791fa83c9c8 Mon Sep 17 00:00:00 2001 From: parththakkar106 Date: Tue, 18 Aug 2026 13:48:16 +0530 Subject: [PATCH] Refresh what a switch changes, not what a turn changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A branch switch does not change the length of the story. It changes which story it is. Four panels keyed on actions.length and so could not tell the difference: the Branches panel drew one branch while the reader was already on a second, Insights showed the prompt built for the path just left, the script-state drawer kept the other line's numbers, and the Memory Bank did not notice a deleted branch taking its memories with it. Only the world-state drawer was right, and only because it happened to carry stateKey already. They all key on the pair now, and deleting a branch bumps it too — that is the one operation that changes what is stored without a turn being played and without the story on the current path moving by a single action. tools/branch_fixture.py is the thing that could show it. The stress fixture's world state is empty, so it cannot answer whether a switch puts the scoreboard back, and its story is one branch. This builds a small bootable adventure with a stat schema, a gold script, two takes on one turn that differ by 35 hit points, a fork, and a memory on each side — with both branches the same length on purpose, because equal length is precisely the case a length-based key cannot see. Verified in a browser with the drawer open: hp 60 to 95 and back, the bar redrawn, the story swapped to the other take, Insights carrying the scratch and not the beating. The Memory Bank deliberately does not change on a switch: the drawer is adventure-wide so a memory is always findable to delete, and retrieval is the path-scoped half. 396 tests, build clean, no new lint. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g --- backend/tools/branch_fixture.py | 170 ++++++++++++++++++++++++++++++++ frontend/src/pages/Play.jsx | 24 ++++- plan/14-phase-story-tree.md | 32 ++++-- plan/STATUS.md | 23 +++-- 4 files changed, 232 insertions(+), 17 deletions(-) create mode 100644 backend/tools/branch_fixture.py diff --git a/backend/tools/branch_fixture.py b/backend/tools/branch_fixture.py new file mode 100644 index 0000000..b6bd2e0 --- /dev/null +++ b/backend/tools/branch_fixture.py @@ -0,0 +1,170 @@ +"""Build a bootable adventure that has actually gone two ways (Phase 14, SP7). + +`tools.stress_session --keep` answers the scroll question and nothing else: its +world state and script state are both empty, so it cannot show whether a branch +switch puts the scoreboard back. This builds the small counterpart — a stat +schema, a script that counts gold, two attempts at one turn that do visibly +different damage, a fork, and a hand-written memory on each branch. + +**The two branches are deliberately the same length.** Both paths are five +actions, so `actions.length` is identical either side of a switch. That is what +makes this fixture worth keeping: every panel that refreshed on the length of +the story looked correct until something asked it to tell two equal-length +branches apart, and then four of them went on showing the branch just left. + + cd backend + .venv/Scripts/python.exe -m tools.branch_fixture /tmp/branches.db + AIDND_DB_PATH=/tmp/branches.db .venv/Scripts/python.exe \\ + -m uvicorn app.main:app --port 8010 + +Then open http://127.0.0.1:8010/ — the SPA is served out of `frontend/dist`, so +run `npm run build` first if it is stale. Expect hp 60 on "The hard way down" +and hp 95 on the fork, and expect both to move the moment you switch. + +No LLM is called: the provider is scripted and its replies carry their own +fenced `state` blocks. +""" +import os +import sys + +# Run from `backend/`; Python puts this script's own directory on the path, not +# the working one. +sys.path.insert(0, os.getcwd()) + +OUT = sys.argv[1] if len(sys.argv) > 1 else "branchfixture.db" +if os.path.exists(OUT): + os.remove(OUT) +os.environ["AIDND_DB_PATH"] = OUT +os.environ.pop("AIDND_DATABASE_URL", None) +os.environ.pop("DATABASE_URL", None) +os.environ.pop("AIDND_MULTI_USER", None) + +from fastapi.testclient import TestClient # noqa: E402 +from sqlalchemy import text # noqa: E402 + +from app import auth, limits, models # noqa: E402 +from app.database import Base, SessionLocal, engine # noqa: E402 +from app.main import app # noqa: E402 +from app.migrations import LATEST_VERSION # noqa: E402 +from app.routers import adventures # noqa: E402 + +SCHEMA = { + "player": { + "hp": {"min": 0, "max": 100, "initial": 100}, + "mana": {"min": 0, "max": 50, "initial": 50, "cooldown": 2}, + } +} + +GOLD_SCRIPT = """ +const modifier = (text) => { + state.gold = (state.gold || 0) + 10; + return { text }; +}; +modifier(text); +""" + + +class ScriptedProvider: + replies: list = [] + calls = 0 + + def __init__(self, *a, **k): + pass + + async def generate(self, parts, *, temperature, max_tokens): + i = min(ScriptedProvider.calls, len(ScriptedProvider.replies) - 1) + ScriptedProvider.calls += 1 + yield ("text", ScriptedProvider.replies[i]) + + +Base.metadata.create_all(bind=engine) +db = SessionLocal() +# Local mode looks for the row with email IS NULL and is_guest false. +user = models.User(is_guest=False, email=None) +db.add(user) +db.flush() +db.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")) +scenario = models.Scenario(user_id=user.id, title="Thornwick", stat_schema=SCHEMA) +db.add(scenario) +db.flush() +adv = models.Adventure( + user_id=user.id, title="The Hollow Beneath Thornwick", scenario_id=scenario.id, + script_state={}, world_state={"player": {"hp": 100, "mana": 50}}, + memory_bank_enabled=True, +) +db.add(adv) +db.flush() +db.add(models.Action( + adventure_id=adv.id, index=0, type="start", + text="The cellar door has been shut since your grandmother died. " + "Tonight the lantern is lit and the key is in your hand.")) +db.add(models.AdventureScript( + adventure_id=adv.id, position=0, enabled=True, name="Gold", output_js=GOLD_SCRIPT)) +db.commit() +adv_id = adv.id +db.close() + +adventures.OpenAICompatibleProvider = ScriptedProvider +auth.resolve_provider_config = lambda s: auth.ProviderConfig( + "http://fake", "k", "test-model", False) +limits.rate_limit = lambda *a, **k: None +limits.check_row_cap = lambda *a, **k: None + +client = TestClient(app) +base = f"/api/adventures/{adv_id}" + + +def play(text): + r = client.post(f"{base}/actions", json={"type": "do", "text": text}) + assert r.status_code == 200, r.text + + +# Turn one: a scratch. Retried into a beating. The story continues from the +# beating, so the scratch is the attempt left behind — and the two differ by 35 +# hit points, which is the number a switch has to put back. +ScriptedProvider.replies = [ + "You ease the door open and a nail catches your wrist.\n" + "```state\n{\"player.hp\": -5}\n```", + "The door slams back and takes you off your feet, down four steps onto " + "stone.\n```state\n{\"player.hp\": -40}\n```", + "The cellar is colder than the night outside, and it smells faintly sweet.", + "Something moves along the far wall, keeping the dark between you.", +] +play("light the lantern and open the cellar door") +r = client.post(f"{base}/retry") +assert r.status_code == 200, r.text +play("go down, one hand on the wall") + +db = SessionLocal() +discarded = [ + a.id for a in db.query(models.Action) + .filter(models.Action.adventure_id == adv_id, models.Action.type == "ai") + .order_by(models.Action.id) if not a.live +][0] +db.close() + +client.post(f"{base}/memories", json={ + "text": "Fell down the cellar stairs; badly hurt, moving slowly."}) + +r = client.post(f"{base}/actions/{discarded}/fork") +assert r.status_code == 200, r.text +play("keep the lantern high and look for the far wall") +client.post(f"{base}/memories", json={ + "text": "Only a scratched wrist; still quick on your feet."}) + +# Leave the reader on the original line, so the difference is one click away. +root = client.get(f"{base}/branches").json()[0]["id"] +client.patch(f"{base}/branches/{root}", json={"name": "The hard way down"}) +client.post(f"{base}/branches/{root}/switch") + +with engine.begin() as conn: + conn.execute(text(f"PRAGMA user_version = {LATEST_VERSION}")) + +for b in client.get(f"{base}/branches").json(): + print(f" branch {b['id']}: name={b['name']!r} fork_depth={b['fork_depth']} " + f"own={b['own_actions']} head={b['is_head']}") +state = client.get(f"{base}/world-state").json()["state"] +print(f" world state on head: {state.get('player')}") +print(f" script state on head: {client.get(f'{base}/script-state').json()['state']}") +print(f" memories: {len(client.get(f'{base}/memories').json())}") +print(f"fixture written: {OUT}") diff --git a/frontend/src/pages/Play.jsx b/frontend/src/pages/Play.jsx index 1e2cc85..e1ec388 100644 --- a/frontend/src/pages/Play.jsx +++ b/frontend/src/pages/Play.jsx @@ -1091,7 +1091,7 @@ function orderBranches(branches) { // Delete is here rather than in some later subphase because nothing prunes a // tree on its own — this panel is the first place a fork can be made, so it // has to be the first place one can be unmade. -function BranchPanel({ advId, refreshKey, onSwitched, onError }) { +function BranchPanel({ advId, refreshKey, onSwitched, onTreeChanged, onError }) { const [branches, setBranches] = useState(null) const [failed, setFailed] = useState(null) const [busyId, setBusyId] = useState(null) @@ -1113,6 +1113,10 @@ function BranchPanel({ advId, refreshKey, onSwitched, onError }) { try { await work() setTick((t) => t + 1) + // Deleting a branch takes its memories with it, and nothing else on the + // screen would hear about that — no turn is played, and the story on the + // current path does not change by a single action. + onTreeChanged() } catch (err) { onError(err.message) } finally { @@ -1841,8 +1845,12 @@ export default function Play() { return (
+ {/* Both drawers read per-adventure state that a branch switch puts back, + so neither can key on the story's length alone: switching between two + branches whose windows are both full changes every number in here + without changing `actions.length` by one. */} - +

{adventure.title}

@@ -2021,7 +2029,10 @@ export default function Play() { onWorldStateChanged={() => setStateKey((k) => k + 1)} /> ) : panel === 'memory' ? ( + // The bank is adventure-wide, so a switch does not change it — + // but deleting a branch deletes the memories that hung off it, + // and that happens without a turn being played. + refreshKey={`${actions.length}:${stateKey}`} /> ) : panel === 'scripts' ? ( ) : panel === 'branches' ? ( @@ -2035,11 +2046,16 @@ export default function Play() { // the two operations that move the head. refreshKey={`${actions.length}:${stateKey}`} onSwitched={adoptWindow} + onTreeChanged={() => setStateKey((k) => k + 1)} onError={(message) => setToast({ text: message, isError: true })} /> ) : ( + // Insights is the prompt as it would be sent *now*, which is built + // from the story on the current path — so of everything on this + // screen it is the panel a branch switch changes most completely. setInspectActionId(null)} refreshKey={actions.length} /> + onClearInspect={() => setInspectActionId(null)} + refreshKey={`${actions.length}:${stateKey}`} /> )}
)} diff --git a/plan/14-phase-story-tree.md b/plan/14-phase-story-tree.md index e63457f..5300798 100644 --- a/plan/14-phase-story-tree.md +++ b/plan/14-phase-story-tree.md @@ -745,12 +745,32 @@ Five things worth not rediscovering: group renumbers whenever an attempt is added, so an ordinal held across that points at a different take — the same reason SP4's note called the pager's index match "one line, and SP7 removes the pager anyway". -- **The panel reloaded on the wrong thing, and only a browser could say so.** Its refresh - key was `actions.length`. A fork taken from the story column swaps one 60-action window - for another 60-action window, so the length never changes, and the panel went on - drawing a one-branch tree while the story was already being read on a second. The - server was correct throughout; nothing in 396 tests could see it. It keys off the - counter `adoptWindow` bumps now. +- **Four panels reloaded on the wrong thing, and only a browser could say so.** + `actions.length` was the refresh key for the Branches panel, the Status drawer + (script state), Insights and the Memory Bank. **A branch switch does not change the + length of the story** — it changes which story it is. So the Branches panel drew a + one-branch tree while the reader was already on a second, Insights showed the prompt + for the path just left, and the scoreboard kept the other line's numbers. The server + was correct throughout, and nothing in 396 tests could see any of it. All four now key + on `${actions.length}:${stateKey}`, and `stateKey` is bumped by `adoptWindow` — the two + operations that move the head — plus branch deletion, which removes that branch's + memories without a turn being played. + + `tools/branch_fixture.py` exists because of this: it builds two branches of **equal + path length**, which is the case `actions.length` cannot distinguish at all. The + `--keep` fixture could not have found it, and neither could a fixture whose branches + happened to differ in length. + +**What a switch puts back was checked end to end, not just server-side.** SP5 already +proved `restore_state` in `test_switching_restores_the_script_and_world_state`; what had +never been looked at is whether the *screen* re-reads it. On `tools/branch_fixture.py`, +switching between the two branches moves the World State drawer from **hp 60 to hp 95** +live, redraws the bar, swaps the story to the other take (`hp -5`, not `hp -40`) and +repoints Insights at the other path — "History: 5 of 5 actions", carrying the scratch and +not the beating. The Memory Bank deliberately does *not* change: the drawer is +adventure-wide so a memory can always be found and deleted, and it is retrieval that is +path-scoped (`test_memory_nodes.py`). That split is worth stating out loud, because +"memories did not change when I switched" reads as a bug and is the design. **The scroll path was driven, and it holds.** Three prepends on the 602-action fixture, 60 actions and ~16,200 px each. The same DOM node stayed at viewport top 792 → 787 — a diff --git a/plan/STATUS.md b/plan/STATUS.md index 561450d..ef4ff5a 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -149,13 +149,22 @@ read the same `GET /branches`. `rename` had no column and no route; `delete` had no route. Migration 61 adds `branches.name`, and the two endpoints came with it. -**One bug, and only a browser could have found it.** The panel refreshed on -`actions.length`. Forking from the story column swaps a 60-action window for another -60-action window, so the length never changes — the panel kept drawing a one-branch tree -while the story was already being read on the second branch. The server was right the -whole time and no test could see it. That is now three bugs on this frontend found by -exercising it rather than by testing it, and the second found in a path that had just -shipped. +**One bug class, in four places, and only a browser could have found it.** The Branches +panel, the Status drawer, Insights and the Memory Bank all refreshed on `actions.length` +— and **a branch switch does not change the length of the story, it changes which story +it is.** So the tree showed one branch while the reader was on a second, Insights showed +the prompt for the path just left, and the scoreboard kept the other line's numbers. The +server was right the whole time and no test could see any of it. All four key on +`${actions.length}:${stateKey}` now. + +`backend/tools/branch_fixture.py` was written to catch exactly this and is worth keeping: +a small bootable adventure with real stats whose **two branches have equal path length**, +which is the case a length-based key cannot distinguish. `--keep` could not have found +it. Run it, switch branches, and watch hp go 60 ↔ 95 with the drawer open. + +That is now four bugs on this frontend found by exercising it rather than by testing it, +two of them in paths that had just shipped. The pattern is not subtle any more: **this +frontend has no test runner, so anything not driven by hand is unverified.** **The scroll path is finally driven.** 602-action fixture, three prepends of ~16,200 px each: the same DOM node held viewport top 792 → 787, and the view stayed 48,174 px from