Refresh what a switch changes, not what a turn changes
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g
This commit is contained in:
committed by
Parth
co-authored by
Claude Opus 5
parent
84827f0f37
commit
811368d048
@@ -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}")
|
||||||
@@ -1091,7 +1091,7 @@ function orderBranches(branches) {
|
|||||||
// Delete is here rather than in some later subphase because nothing prunes a
|
// 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
|
// 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.
|
// 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 [branches, setBranches] = useState(null)
|
||||||
const [failed, setFailed] = useState(null)
|
const [failed, setFailed] = useState(null)
|
||||||
const [busyId, setBusyId] = useState(null)
|
const [busyId, setBusyId] = useState(null)
|
||||||
@@ -1113,6 +1113,10 @@ function BranchPanel({ advId, refreshKey, onSwitched, onError }) {
|
|||||||
try {
|
try {
|
||||||
await work()
|
await work()
|
||||||
setTick((t) => t + 1)
|
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) {
|
} catch (err) {
|
||||||
onError(err.message)
|
onError(err.message)
|
||||||
} finally {
|
} finally {
|
||||||
@@ -1841,8 +1845,12 @@ export default function Play() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={`play-layout ${panel ? 'with-panel' : ''}`}>
|
<div className={`play-layout ${panel ? 'with-panel' : ''}`}>
|
||||||
|
{/* 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. */}
|
||||||
<WorldStateDrawer advId={id} refreshKey={`${actions.length}:${stateKey}`} />
|
<WorldStateDrawer advId={id} refreshKey={`${actions.length}:${stateKey}`} />
|
||||||
<StatusDrawer advId={id} refreshKey={actions.length} />
|
<StatusDrawer advId={id} refreshKey={`${actions.length}:${stateKey}`} />
|
||||||
<div className="page play-page">
|
<div className="page play-page">
|
||||||
<div className="page-header">
|
<div className="page-header">
|
||||||
<h1>{adventure.title}</h1>
|
<h1>{adventure.title}</h1>
|
||||||
@@ -2021,7 +2029,10 @@ export default function Play() {
|
|||||||
onWorldStateChanged={() => setStateKey((k) => k + 1)} />
|
onWorldStateChanged={() => setStateKey((k) => k + 1)} />
|
||||||
) : panel === 'memory' ? (
|
) : panel === 'memory' ? (
|
||||||
<MemoryPanel adventure={adventure} setAdventure={setAdventure}
|
<MemoryPanel adventure={adventure} setAdventure={setAdventure}
|
||||||
refreshKey={actions.length} />
|
// 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 === 'scripts' ? (
|
||||||
<ScriptsPanel advId={id} />
|
<ScriptsPanel advId={id} />
|
||||||
) : panel === 'branches' ? (
|
) : panel === 'branches' ? (
|
||||||
@@ -2035,11 +2046,16 @@ export default function Play() {
|
|||||||
// the two operations that move the head.
|
// the two operations that move the head.
|
||||||
refreshKey={`${actions.length}:${stateKey}`}
|
refreshKey={`${actions.length}:${stateKey}`}
|
||||||
onSwitched={adoptWindow}
|
onSwitched={adoptWindow}
|
||||||
|
onTreeChanged={() => setStateKey((k) => k + 1)}
|
||||||
onError={(message) => setToast({ text: message, isError: true })}
|
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.
|
||||||
<InsightsPanel advId={id} inspectActionId={inspectActionId}
|
<InsightsPanel advId={id} inspectActionId={inspectActionId}
|
||||||
onClearInspect={() => setInspectActionId(null)} refreshKey={actions.length} />
|
onClearInspect={() => setInspectActionId(null)}
|
||||||
|
refreshKey={`${actions.length}:${stateKey}`} />
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -745,12 +745,32 @@ Five things worth not rediscovering:
|
|||||||
group renumbers whenever an attempt is added, so an ordinal held across that points at
|
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,
|
a different take — the same reason SP4's note called the pager's index match "one line,
|
||||||
and SP7 removes the pager anyway".
|
and SP7 removes the pager anyway".
|
||||||
- **The panel reloaded on the wrong thing, and only a browser could say so.** Its refresh
|
- **Four panels reloaded on the wrong thing, and only a browser could say so.**
|
||||||
key was `actions.length`. A fork taken from the story column swaps one 60-action window
|
`actions.length` was the refresh key for the Branches panel, the Status drawer
|
||||||
for another 60-action window, so the length never changes, and the panel went on
|
(script state), Insights and the Memory Bank. **A branch switch does not change the
|
||||||
drawing a one-branch tree while the story was already being read on a second. The
|
length of the story** — it changes which story it is. So the Branches panel drew a
|
||||||
server was correct throughout; nothing in 396 tests could see it. It keys off the
|
one-branch tree while the reader was already on a second, Insights showed the prompt
|
||||||
counter `adoptWindow` bumps now.
|
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,
|
**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
|
60 actions and ~16,200 px each. The same DOM node stayed at viewport top 792 → 787 — a
|
||||||
|
|||||||
+16
-7
@@ -149,13 +149,22 @@ read the same `GET /branches`.
|
|||||||
`rename` had no column and no route; `delete` had no route. Migration 61 adds
|
`rename` had no column and no route; `delete` had no route. Migration 61 adds
|
||||||
`branches.name`, and the two endpoints came with it.
|
`branches.name`, and the two endpoints came with it.
|
||||||
|
|
||||||
**One bug, and only a browser could have found it.** The panel refreshed on
|
**One bug class, in four places, and only a browser could have found it.** The Branches
|
||||||
`actions.length`. Forking from the story column swaps a 60-action window for another
|
panel, the Status drawer, Insights and the Memory Bank all refreshed on `actions.length`
|
||||||
60-action window, so the length never changes — the panel kept drawing a one-branch tree
|
— and **a branch switch does not change the length of the story, it changes which story
|
||||||
while the story was already being read on the second branch. The server was right the
|
it is.** So the tree showed one branch while the reader was on a second, Insights showed
|
||||||
whole time and no test could see it. That is now three bugs on this frontend found by
|
the prompt for the path just left, and the scoreboard kept the other line's numbers. The
|
||||||
exercising it rather than by testing it, and the second found in a path that had just
|
server was right the whole time and no test could see any of it. All four key on
|
||||||
shipped.
|
`${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
|
**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
|
each: the same DOM node held viewport top 792 → 787, and the view stayed 48,174 px from
|
||||||
|
|||||||
Reference in New Issue
Block a user