From 4af6e17406142d6533b8a6a2533669a1ad895f9b Mon Sep 17 00:00:00 2001 From: parththakkar106 Date: Tue, 18 Aug 2026 17:41:23 +0530 Subject: [PATCH] Answer the review, and keep the opening node's bank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine findings from a review of the phase-14 stack. The one about a retry withdrawing a memory is not a bug — a memory anchored to a node describes that node, and it goes when the node goes. The root is the exception, and it is the only one: migration 62 parked every memory written before memories had coordinates on depth 0, so withdrawing the opening node would retire a whole bank nobody attached there. A memory with no source range covers no story and now stays; a summary that genuinely ends there is still withdrawn. The rest are repairs. * The adventure list quoted whichever attempt was written last rather than the one the story tells, so switching back left the index disagreeing with the page. * A v1 import gave a typed memory no depth, rebuilding the NULL the migration exists to remove — invisible until the imported adventure forked. * The action cap counted a v1 file's turns, and a turn expands into a row per saved attempt, so a file inside the cap could write a multiple of it. * Forking a live node on a borrowed ancestor promoted a sibling on a branch the caller never named. It is a branch switch, and now says so. * Switching attempts left the state, status and memory panels reading the previous take: the story does not change length, so nothing keyed on its length noticed. Same class as the branch-switch bug this phase already fixed. * A retry after switching back numbered the new attempt into the middle of the group instead of the end. * The cursor backfill numbered every action in the table once per adventure; correlated to the adventure being updated, it is an index lookup instead. * Renaming a branch answered own_actions=0. And one behaviour change recorded rather than repaired: script-visible history and actionCount no longer count blank-text rows. That is the right shape and there is no reading compatible with both, so plan/14 says so. 409 tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015H5qiyiR7gtFQaoDphHZ3g --- backend/app/attempts.py | 12 ++++- backend/app/bundle.py | 10 ++++ backend/app/context/lineage.py | 8 +++ backend/app/memorybank.py | 14 +++++ backend/app/migrations.py | 22 ++++++-- backend/app/routers/adventures.py | 52 ++++++++++++++++--- backend/app/scripting/pipeline.py | 9 ++++ backend/tests/test_attempt_siblings.py | 50 ++++++++++++++++++ backend/tests/test_branch_forking.py | 13 +++-- backend/tests/test_branch_management.py | 18 +++++++ backend/tests/test_bundle_v2.py | 69 +++++++++++++++++++++++++ backend/tests/test_memory_nodes.py | 50 ++++++++++++++++++ frontend/src/pages/Play.jsx | 9 ++++ plan/14-phase-story-tree.md | 11 ++++ 14 files changed, 330 insertions(+), 17 deletions(-) diff --git a/backend/app/attempts.py b/backend/app/attempts.py index a17a967..eaf8ed1 100644 --- a/backend/app/attempts.py +++ b/backend/app/attempts.py @@ -150,7 +150,17 @@ def add_attempt( replacement.branch_id = previous.branch_id replacement.depth = previous.depth replacement.live = True - replacement.variant_index = previous.variant_index + 1 + # The end of the group, not one past `previous` — which is only the same + # thing when `previous` is the newest take. Switch a three-take turn back to + # take 1 and retry, and `previous.variant_index + 1` collides with take 2; + # `renumber` then breaks the tie by id and files the new attempt *between* + # takes 2 and 3, so the pager walks the takes in an order they were not made + # in. `group` is oldest-first, and `replacement` is not in it yet. + siblings = group(db, previous) + replacement.variant_index = 1 + max( + (s.variant_index for s in siblings if s.variant_index is not None), + default=previous.variant_index or 0, + ) previous.live = False # The replacement was assembled with a fresh snapshot, so the prompt for # this turn is now the one it carries; the superseded attempt keeps only diff --git a/backend/app/bundle.py b/backend/app/bundle.py index c1bf286..25bb954 100644 --- a/backend/app/bundle.py +++ b/backend/app/bundle.py @@ -545,6 +545,16 @@ def _write_memories( # last action it summarises, which on one branch is that node's depth. if memory.depth is None and memory.source_end is not None: memory.depth = memory.source_end + # A v1 memory that summarises nothing — one the player typed — has no + # depth to derive, and leaving it NULL here would rebuild by import the + # exact state migration 62 exists to end: `Path._entry_clause` compares + # `depth <= max_depth`, which a NULL fails, so the memory would vanish + # from every branch the moment the imported adventure was forked. The + # root is the same answer the migration gives, and for the same reason + # — 0 is at or before every fork point, so it is visible from every + # path this adventure can grow. + if memory.depth is None: + memory.depth = lineage.ROOT_DEPTH db.add(memory) diff --git a/backend/app/context/lineage.py b/backend/app/context/lineage.py index e19169d..35cd604 100644 --- a/backend/app/context/lineage.py +++ b/backend/app/context/lineage.py @@ -41,6 +41,14 @@ from .. import models # separately so a read never has to import the write half. NO_DEPTH = -1 +# The opening node of an adventure. Depth 0 exists only on the root branch — a +# fork starts its own nodes after the depth it forked at — so this names one +# node per adventure, not one per branch. It is also where migration 62 parked +# every memory written before memories had coordinates, which is why the two +# places that can retire a memory (`memorybank.forget_node`, and a v1 import +# with no depth to read) both have to say something about it. +ROOT_DEPTH = 0 + def entries_of(branch: models.Branch) -> list[tuple[int, int | None]]: """`branch.lineage` as (branch_id, max_depth) pairs, newest first. diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index ba42df1..221558b 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -171,6 +171,9 @@ def forget_node(db: Session, adventure: models.Adventure, action: models.Action) the node before it, which is a depth whether or not anything still sits there. + The opening node is the one exception, because migration 62 parked the + whole pre-coordinate bank on it — see the comment on `lineage.ROOT_DEPTH`. + Returns how many memories were withdrawn. """ if action.branch_id is None or action.depth is None: @@ -184,6 +187,17 @@ def forget_node(db: Session, adventure: models.Adventure, action: models.Action) ) .all() ) + if action.depth == lineage.ROOT_DEPTH: + # The opening node is special, and only for memories that describe no + # stretch of story. Migration 62 parked every memory written before + # memories had coordinates at depth 0 — that was the choice that took + # nothing away from anybody, but it also collected them all onto one + # node, so withdrawing that node would retire a player's whole bank in + # a single click. A memory with no `source_start` was typed (or + # migrated), describes nothing that can fall off the end, and so has + # nothing to be withdrawn *from*: it stays. A summary that genuinely + # ends here is still withdrawn, because the text it describes is going. + doomed = [m for m in doomed if m.source_start is not None] if not doomed: return 0 starts = [m.source_start for m in doomed if m.source_start is not None] diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 89ac45f..93962b5 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -585,6 +585,17 @@ def _backfill_cursor_anchors(conn) -> None: Guarded on `_depth = -1` so a run that dies halfway resumes: every adventure this has already converted is skipped, and one it has not is indistinguishable from an untouched row. + + **The row numbering is correlated, not ranked-then-filtered.** Numbering + every action in the table and picking one row out of the result reads the + whole of `actions` per adventure — the window function is what stops the + correlation being pushed down, so the planner has no way to make it cheaper + — and this runs inside the one transaction that holds the schema, at boot, + against a database with real stories in it. Restricting the scan to the + adventure being updated makes each pass an index lookup on + `actions.adventure_id` instead, and `PARTITION BY` is then a partition of + one. The two forms give the same answer for the same reason: the rows the + partition would have separated are exactly the rows the filter removes. """ sqlite = conn.dialect.name == "sqlite" story = _story_text_sql("text", sqlite) @@ -594,13 +605,14 @@ def _backfill_cursor_anchors(conn) -> None: SET {name}_cursor_branch_id = {_root_branch_of('adventures.id')}, {name}_cursor_depth = COALESCE( (SELECT ranked.depth FROM ( - SELECT adventure_id, depth, ROW_NUMBER() OVER ( - PARTITION BY adventure_id ORDER BY depth, id + SELECT depth, ROW_NUMBER() OVER ( + ORDER BY depth, id ) AS rn - FROM actions WHERE {story} + FROM actions + WHERE {story} + AND actions.adventure_id = adventures.id ) AS ranked - WHERE ranked.adventure_id = adventures.id - AND ranked.rn = adventures.{name}_cursor), + WHERE ranked.rn = adventures.{name}_cursor), (SELECT MAX(a.depth) FROM actions a WHERE a.adventure_id = adventures.id AND {_story_text_sql('a.text', sqlite)}), diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index da8e661..a42d66f 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -203,6 +203,13 @@ def _latest_narration(db: Session, head_branches: dict[int, int | None]) -> dict models.Action.adventure_id.in_(list(head_branches)), models.Action.branch_id.in_(branch_ids), models.Action.type.in_(NARRATION_TYPES), + # Siblings share a depth and the newest of them has the highest id, + # so without this the snippet quotes whichever attempt was written + # last rather than the one the story tells. Switching back to an + # earlier take would leave the index screen quoting the discarded + # one — the story on the screen and the story in the list would + # disagree, and only the list would be wrong. + models.Action.live.is_(True), ) .subquery() ) @@ -1150,14 +1157,17 @@ def rename_branch( adventure.updated_at = models.utcnow() db.commit() db.refresh(branch) - tip = ( - db.query(func.max(models.Action.depth)) + # Both numbers in the one pass, and counted the way `list_branches` counts + # them — live rows on this branch. A renamed branch is the same branch, so + # the row this hands back has to be the row the panel would have fetched. + tip, own = ( + db.query(func.max(models.Action.depth), func.count(models.Action.id)) .filter( models.Action.adventure_id == adventure.id, models.Action.branch_id == branch.id, models.Action.live.is_(True), ) - .scalar() + .one() ) return schemas.BranchOut( id=branch.id, @@ -1166,7 +1176,7 @@ def rename_branch( depth=tip if tip is not None else ( branch.fork_depth if branch.fork_depth is not None else tree.NO_DEPTH ), - own_actions=0, + own_actions=own, is_head=(branch.id == adventure.head_branch_id), name=branch.name, created_at=branch.created_at, @@ -1342,8 +1352,26 @@ def fork_from_attempt( # attempt alone on its branch: a client that repeats the call — a double # click, a retried request — must get the same answer, not a complaint that # the turn it just forked has nothing to fork to. - if action.live and action.branch_id == adventure.head_branch_id: - return current_window(db, adventure) + if action.live: + # A live node already *is* what its coordinate says, so there is no + # attempt here to take. On the path being read that is simply a no-op, + # and it has to stay one: a client that repeats the call — a double + # click, a retried request — must get the same answer, not a complaint + # that the turn it just forked has nothing to fork to. Off the path it + # is a different line's story, and moving there is a branch switch. + # + # The membership test is the whole lineage, not `head_branch_id`. A + # head borrows its ancestors' turns, so a live node on an ancestor is + # already being read; forking it would move the live row off the parent + # and promote a sibling in its place, rewriting the story on a branch + # nobody asked about *and* on this one, which borrows that depth. + if lineage.path_of(db, adventure).contains(action): + return current_window(db, adventure) + raise HTTPException( + 400, + "That take is already the story on another branch. Switch to that " + "branch to read it.", + ) if len(attempts.group(db, action)) < 2: raise HTTPException( 400, "This turn has only one take, so there is nothing to fork to." @@ -1482,6 +1510,18 @@ def import_adventure( # disagrees with itself is a 400 and not a half-imported adventure holding a # story with a hole in it. story = bundle.plan(payload, version) + # Counted again, on what will actually be written. The check above reads the + # file's own lists, and in a v1 file a turn is one entry carrying its retries + # in a `variants` array — which `plan()` expands into one row per attempt + # (SP4 made every attempt a node). So a file of 5,000 turns with ten takes + # each passes a 5,000-action cap and writes 50,000 rows, comfortably inside + # the 20 MB body limit. `plan()` is pure and the adventure does not exist + # yet, so this still costs nothing but the planning. + limits.check_bundle_lists( + actions=story["nodes"], + memories=story["memories"], + branches=story["branches"], + ) # Raw-dict import bypasses the schemas — clamp strings headed for VARCHAR # columns (Postgres enforces the widths; see schemas.py). diff --git a/backend/app/scripting/pipeline.py b/backend/app/scripting/pipeline.py index 60f0a43..b59e7c9 100644 --- a/backend/app/scripting/pipeline.py +++ b/backend/app/scripting/pipeline.py @@ -28,6 +28,15 @@ class ScriptPipeline: # actions, and this is the documented history API a user script reads. # Handing a script the siblings of the turn it is running on would be # the same bug as building a prompt from them, only user-visible. + # + # `story_actions` also drops blank-text rows, which `adventure.actions` + # kept, so this array is shorter than it used to be for an adventure + # that has any — and `info.actionCount` counts the same way. That is + # deliberate: a row with no text is this app's bookkeeping, it has no + # counterpart in the AI Dungeon history a ported script was written + # against, and the prompt has never included one. A script keyed on + # "every N actions" will land on different turns than it did before + # phase 14; there is no reading of this that is compatible with both. return [ {"text": a.text, "rawText": a.text, "type": a.type} for a in context_history.story_actions(self.adventure) diff --git a/backend/tests/test_attempt_siblings.py b/backend/tests/test_attempt_siblings.py index 41fa3a7..35f1c66 100644 --- a/backend/tests/test_attempt_siblings.py +++ b/backend/tests/test_attempt_siblings.py @@ -404,3 +404,53 @@ def test_export_carries_every_attempt_as_its_own_node(client): ai_rows = [a for a in rows if a.type == "ai"] assert [(a.text, a.live) for a in ai_rows] == [("One.", False), ("Two.", True)] assert len({(a.branch_id, a.depth) for a in ai_rows}) == 1 + + +def test_a_retry_after_switching_back_files_the_new_attempt_last(client): + """The group stays in the order the attempts were made. + + `add_attempt` used to number a new take one past the take it replaced, which + is the end of the group only when the story is standing on the newest one. + Switch a three-take turn back to the first and retry, and the new attempt + collided with take 2 — `renumber` then broke the tie by id and filed it + *between* takes 2 and 3, so the pager walked them in an order nobody played. + """ + ScriptedProvider.replies = ["One.", "Two.", "Three.", "Four."] + _play(client) + _retry(client) + _retry(client) + assert [a.text for a in _rows(client.adv_id) if a.type == "ai"] == [ + "One.", "Two.", "Three."] + + live = _page(client)["actions"][-1] + r = client.post( + f"/api/adventures/{client.adv_id}/actions/{live['id']}/variant", + json={"index": 0}) + assert r.status_code == 200, r.text + + _retry(client) + ai = [a for a in _rows(client.adv_id) if a.type == "ai"] + assert [a.text for a in ai] == ["One.", "Two.", "Three.", "Four."] + assert [a.variant_index for a in ai] == [0, 1, 2, 3] + assert attempts.live_in(ai).text == "Four." + + +def test_the_adventure_list_quotes_the_take_the_story_tells(client): + """The index screen and the story have to agree. + + Siblings share a depth and the newest of them has the highest id, so a + snippet ordered by `(depth, id)` alone quotes whichever attempt was written + last — which, after switching back, is the one the player threw away. + """ + ScriptedProvider.replies = ["One.", "Two."] + _play(client) + _retry(client) + live = _page(client)["actions"][-1] + assert live["text"] == "Two." + client.post(f"/api/adventures/{client.adv_id}/actions/{live['id']}/variant", + json={"index": 0}) + + listed = client.get("/api/adventures").json() + row = [a for a in listed if a["id"] == client.adv_id][0] + assert "One." in row["snippet"] + assert "Two." not in row["snippet"] diff --git a/backend/tests/test_branch_forking.py b/backend/tests/test_branch_forking.py index 2168ac0..ea7a3c6 100644 --- a/backend/tests/test_branch_forking.py +++ b/backend/tests/test_branch_forking.py @@ -321,10 +321,11 @@ def test_forking_a_turn_that_is_already_the_story_is_a_no_op(client): assert len(_branches(client)) == 1 -def test_forking_a_single_take_on_another_branch_is_refused(client): - """The only way to reach the refusal, and it names the wrong tool: a node - with no siblings is not a divergence, so what the caller wants is to switch - to the branch it is on.""" +def test_forking_a_live_node_on_another_branch_is_refused(client): + """A live node off the path is another line's story, not an attempt going + spare — so the refusal names the tool that would actually do it. It used to + answer "only one take", which was true of the group and no help at all: the + caller does not want another take, it wants the branch this one is on.""" discarded = _divergent_story(client) _fork(client, discarded) parent_id = [b for b in _branches(client) if b["parent_branch_id"] is None][0]["id"] @@ -333,7 +334,9 @@ def test_forking_a_single_take_on_another_branch_is_refused(client): r = _fork(client, stranded) assert r.status_code == 400 - assert "one take" in r.json()["detail"] + assert "another branch" in r.json()["detail"] + # And refusing left the tree alone — the bug this guards is a fork that + # promotes a sibling on the branch it was called against. assert len(_branches(client)) == 2 diff --git a/backend/tests/test_branch_management.py b/backend/tests/test_branch_management.py index ff0f6a8..cd428fd 100644 --- a/backend/tests/test_branch_management.py +++ b/backend/tests/test_branch_management.py @@ -222,6 +222,24 @@ def test_a_name_longer_than_the_column_is_refused(client): assert _rename(client, forked, "x" * schemas.BRANCH_NAME_MAX).status_code == 200 +def test_a_rename_hands_back_the_row_the_listing_would_give(client): + """Renaming a branch does not change how many turns are on it. + + `own_actions` was hard-coded to 0 in this response, which only stayed + invisible because the panel throws the body away and refetches. Anything + that trusted the reply would draw a branch that had just lost its turns. + """ + root, forked = _forked(client) + listed = {b["id"]: b for b in _branches(client)} + + renamed = _rename(client, forked, "the cellar").json() + + assert renamed["own_actions"] == listed[forked]["own_actions"] + assert renamed["own_actions"] > 0, "the fixture put turns on this branch" + assert renamed["depth"] == listed[forked]["depth"] + assert renamed["is_head"] == listed[forked]["is_head"] + + def test_naming_a_branch_of_another_adventure_is_a_404(client): root, forked = _forked(client) db = SessionLocal() diff --git a/backend/tests/test_bundle_v2.py b/backend/tests/test_bundle_v2.py index 7b1c276..d151cdd 100644 --- a/backend/tests/test_bundle_v2.py +++ b/backend/tests/test_bundle_v2.py @@ -533,6 +533,75 @@ def test_a_v2_bundle_brings_its_anchors_back(client): db.close() +def test_a_v1_memory_that_summarises_nothing_lands_on_the_root(client): + """The import has to answer the question migration 62 answered. + + A v1 file has no depths, and a memory the player typed has no `sourceEnd` + to derive one from — so it used to come back with a NULL depth, which is the + exact state SP7 removed from the schema. `Path._entry_clause` compares + `depth <= max_depth` and a NULL fails it, so the memory would read fine + until the imported adventure was forked and then vanish from the new branch. + """ + payload = { + "format": bundle.LEGACY_FORMAT, "title": "Old backup", + "memoryCursor": 0, "summaryCursor": 0, + "actions": [ + {"index": 0, "type": "start", "text": OPENING}, + {"index": 1, "type": "story", "text": "A corridor."}, + ], + "memories": [ + {"text": "Kira is the innkeeper's daughter"}, # typed + {"text": "The corridor, summarised", "sourceStart": 1, "sourceEnd": 1}, + ], + } + copy = _imported(client, payload) + + db = SessionLocal() + try: + rows = {m.text: m for m in db.query(models.Memory) + .filter(models.Memory.adventure_id == copy).all()} + assert rows["Kira is the innkeeper's daughter"].depth == 0, ( + "a typed memory anchors at the root, which every branch can see" + ) + assert rows["The corridor, summarised"].depth == 1, "derived from its range" + assert all(m.depth is not None for m in rows.values()) + assert all(m.branch_id is not None for m in rows.values()) + finally: + db.close() + + +def test_the_action_cap_counts_the_rows_a_v1_file_expands_into(client, monkeypatch): + """The cap has to count what gets written, not what the file lists. + + A v1 turn carries its retries in a `variants` array, and SP4 made every + attempt a row — so one entry can become ten. Counting entries lets a file + inside the cap write a multiple of it, and the body limit is no help: the + text is tiny, it is the row count that is the cost. + """ + monkeypatch.setattr(auth, "MULTI_USER", True) + monkeypatch.setattr(limits, "MAX_ACTIONS_PER_ADVENTURE", 6) + monkeypatch.setattr(limits, "_BUNDLE_LIST_CAPS", + {**limits._BUNDLE_LIST_CAPS, "actions": 6}) + payload = { + "format": bundle.LEGACY_FORMAT, "title": "Small file, many rows", + "memoryCursor": 0, "summaryCursor": 0, + "actions": [{"index": 0, "type": "start", "text": OPENING}] + [ + { + "index": i, "type": "ai", "text": "Take four.", + "variants": [{"text": f"Take {n}."} for n in range(4)], + "variantIndex": 3, + } + for i in range(1, 4) + ], + } + assert len(payload["actions"]) <= 6, "the file itself is inside the cap" + before = _adventure_count() + + r = _import(client, payload) + assert r.status_code == 409, r.text + assert _adventure_count() == before, "and nothing was written" + + def test_an_unknown_format_is_refused(client): r = _import(client, {"format": "ai-dnd-adventure-v3", "title": "From the future"}) assert r.status_code == 400, r.text diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py index 0ac9965..7cc029e 100644 --- a/backend/tests/test_memory_nodes.py +++ b/backend/tests/test_memory_nodes.py @@ -452,3 +452,53 @@ def test_retrieving_from_a_deep_fork_costs_what_a_flat_story_costs(deeply_forked f"retrieval on a 20-fork story cost {forked_bytes:,} B against the " f"{flat_bytes:,} B a flat story of the same length cost" ) + + +# ------------------------------------------------------- the opening node + +def test_a_typed_memory_on_the_opening_node_survives_that_node_going(forked): + """The one place a node and its memories part company. + + A memory anchored to a node is withdrawn with the node, which is the rule + and is deliberate: it described that turn, and the turn is leaving. But + migration 62 parked *every* memory written before memories had coordinates + on depth 0 — the only landing spot visible from every branch — so the + opening node carries a whole bank it never produced. Withdrawing it would + retire all of that in one click, for every adventure predating the tree. + + A memory with no `source_start` covers no stretch of story, so nothing about + it can go stale. It stays. + """ + db, adventure, settings, ids = forked + typed = models.Memory( + adventure_id=adventure.id, text="Kira is the innkeeper's daughter", + source_start=None, source_end=None, + ) + typed.branch_id, typed.depth = ids["a"], 0 + db.add(typed) + db.commit() + typed_id = typed.id + + withdrawn = memorybank.forget_node(db, adventure, ids["nodes"]["A0"]) + db.commit() + + assert withdrawn == 0 + assert db.get(models.Memory, typed_id) is not None + + +def test_a_summary_of_the_opening_node_is_still_withdrawn(forked): + """The exception is about memories that describe nothing, not about depth 0. + + A summary that genuinely ends on the opening node describes text that is + going, so it goes too — otherwise the root would collect exactly the + dangling rows `forget_node` replaced `prune_dangling_memories` to prevent. + """ + db, adventure, settings, ids = forked + derived = add_memory(db, adventure, "the opening, summarised", ids["nodes"]["A0"]) + derived_id = derived.id + + withdrawn = memorybank.forget_node(db, adventure, ids["nodes"]["A0"]) + db.commit() + + assert withdrawn == 1 + assert db.get(models.Memory, derived_id) is None diff --git a/frontend/src/pages/Play.jsx b/frontend/src/pages/Play.jsx index e1ec388..d2d1d9b 100644 --- a/frontend/src/pages/Play.jsx +++ b/frontend/src/pages/Play.jsx @@ -1943,6 +1943,15 @@ export default function Play() { // rewriting this one, so the reply carries a new id. setActions((prev) => prev.map( (a) => (a.id === action.id ? updated : a))) + // Switching takes is not only a change of text. The + // server puts back that attempt's script and world + // state, withdraws the memory that hung off the + // coordinate, and rewinds both cursors — none of which + // the panels can see, because they key on + // `actions.length` and the story is the same length it + // was. Same class of bug as a branch switch, which + // `adoptWindow` already bumps this for. + setStateKey((k) => k + 1) }} onError={(message) => setToast({ text: message, isError: true })} /> diff --git a/plan/14-phase-story-tree.md b/plan/14-phase-story-tree.md index 771abc9..0b483ba 100644 --- a/plan/14-phase-story-tree.md +++ b/plan/14-phase-story-tree.md @@ -812,6 +812,17 @@ hand-driving *or* the harness and this took the first. The harness remains the t would catch a prepend regression without a person in the loop, and jsdom's lack of layout means it would not have settled the 5 px question either way. +**One user-visible change nobody asked for, recorded here because a script +author would otherwise find it by being surprised.** Moving `_history` onto +`context_history.story_actions` fixed the branch bug it was there to fix, and +carried a second change with it: `story_actions` drops blank-text rows, which +`adventure.actions` did not. So a user script's `history` array and +`info.actionCount` both got shorter for any adventure holding one. It is the +right shape — a textless row is bookkeeping with no AI Dungeon counterpart, and +the prompt never included one — but a script that fires "every N actions" now +fires on different turns. Not compatible with both readings; this is the one +chosen. + ### SP8 — Drop the legacy columns Only once the tree is proven live. Migration drops `index`, `variants`, `variant_index`,