Group a turn's takes by their parent, not by where they sit
SP7 shipped a tree nobody could use. Driving it by hand found three things, and the schema is what the third one needs. A take forked onto its own branch leaves the coordinate its siblings are still at. `attempts.group` filtered on (branch_id, depth), so that take read as the only one of its turn -- 1/1 where the player is owed 1/3, with the other takes unreachable from the line they were taken on. Nesting has the same shape from the other side: takes under C1 and takes under C2 share a depth, and only the parent says a pager under C2 reads 2/2 rather than counting C1's three too. So `actions.parent_id`, and `group` keys on it. The alternative -- pointing a branch's fork at a node instead of a depth, so a promoted take never moves -- was rejected: `lineage` exists so a read is an OR-clause per branch rather than a walk up parent pointers, and moving the fork point changes path resolution itself, which drags in cursors, memory depths and both bundle formats. This column is read to group takes and for nothing else. One indexed lookup, never a walk, and no read of the story changes. Two writers had to learn it. `add_attempt` places a sibling by hand rather than through `place_action`, and copies the parent, because a take belongs to the turn it is a take *of*. `place_action` resolves it from the path for a genuinely new node, which is the honest default. And one reader had to be kept out of it. `delete_turn` meant "every attempt at this coordinate"; the group spans branches now, so undoing a turn would have deleted a take that another branch is telling. `attempts.on_branch` scopes it back. 409 tests, unchanged from main and green -- a linear story has one take per parent, so none of this is reachable until a second one exists. The migration adds one column, backfills the linear case, and leaves NULL where it cannot honestly place a row; `group` falls back to the coordinate there, which is the rule those rows were written under. One `VACUUM FULL actions;` owed after deploy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dvvqn9ZDR4ixeFPHNbww7
This commit is contained in:
committed by
Parth
co-authored by
Claude Opus 5
parent
e692780f08
commit
6fa213f6db
@@ -266,6 +266,16 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [
|
||||
# visible from today. Anchoring them at the tip instead would have emptied
|
||||
# them out of every branch forked earlier than they were typed.
|
||||
(62, "UPDATE memories SET depth = 0 WHERE depth IS NULL"),
|
||||
# Phase 14, SP9 — the node a node was played after, so a turn's takes can be
|
||||
# grouped by parent instead of by coordinate. A coordinate stops answering
|
||||
# "which takes belong together" the moment one of them is forked onto its
|
||||
# own branch: it leaves its siblings behind and reads 1/1 beside their 1/3.
|
||||
#
|
||||
# Nullable, and left NULL wherever the backfill cannot honestly place a row
|
||||
# (see _backfill_parents). `attempts.group` falls back to the coordinate for
|
||||
# those, which is the rule they were written under.
|
||||
(63, "ALTER TABLE actions ADD COLUMN parent_id INTEGER REFERENCES actions(id) ON DELETE SET NULL"),
|
||||
(64, "CREATE INDEX IF NOT EXISTS ix_actions_parent ON actions (parent_id)"),
|
||||
]
|
||||
|
||||
LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1)
|
||||
@@ -278,6 +288,7 @@ SNAPSHOT_COMPRESS_VERSION = 43
|
||||
TREE_BACKFILL_VERSION = 52
|
||||
CURSOR_ANCHOR_VERSION = 56
|
||||
SIBLING_SPLIT_VERSION = 60
|
||||
PARENT_BACKFILL_VERSION = 64
|
||||
|
||||
# An adventure with no actions has no tip. -1 keeps "the next node goes at
|
||||
# head_depth + 1" true without a special case (mirrors tree.NO_DEPTH).
|
||||
@@ -324,6 +335,45 @@ def _backfill_world_delta(conn) -> None:
|
||||
conn.execute(text(sql))
|
||||
|
||||
|
||||
def _backfill_parents(conn) -> None:
|
||||
"""Point every node at the take it was played after.
|
||||
|
||||
One pass, and deliberately only one: the live node one depth back on the
|
||||
same branch. That is the whole of a linear story, which is the whole of
|
||||
every adventure written before SP9 — forking reached the screen in SP7 and
|
||||
the tree was found unusable before anyone forked with it.
|
||||
|
||||
The rows left NULL are the first node of a forked branch, whose parent sits
|
||||
on an ancestor branch and cannot be found without walking `lineage` per row.
|
||||
`attempts.group` falls back to the coordinate for a NULL parent, which is
|
||||
exactly the rule those rows were written under, so the fallback is not a
|
||||
degraded answer for them — it is the original one. Every fork made from SP9
|
||||
on sets `parent_id` at write time and never relies on this.
|
||||
|
||||
Correlated to the row being updated rather than numbering the table, so the
|
||||
planner drives it off ix_actions_branch_depth. That is the lesson of
|
||||
_backfill_cursor_anchors, which numbered every action once per adventure.
|
||||
"""
|
||||
conn.execute(
|
||||
text(
|
||||
"""
|
||||
UPDATE actions SET parent_id = (
|
||||
SELECT prev.id FROM actions AS prev
|
||||
WHERE prev.adventure_id = actions.adventure_id
|
||||
AND prev.branch_id = actions.branch_id
|
||||
AND prev.depth = actions.depth - 1
|
||||
AND prev.live = TRUE
|
||||
ORDER BY prev.id
|
||||
LIMIT 1
|
||||
)
|
||||
WHERE parent_id IS NULL
|
||||
AND branch_id IS NOT NULL
|
||||
AND depth IS NOT NULL
|
||||
"""
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _backfill_variant_count(conn) -> None:
|
||||
"""Populate actions.variant_count from the existing variants list.
|
||||
|
||||
@@ -890,6 +940,10 @@ def bootstrap(engine: Engine) -> None:
|
||||
if version == SIBLING_SPLIT_VERSION:
|
||||
_backfill_state_after(conn)
|
||||
_split_variants_into_siblings(conn)
|
||||
# After 63 adds the column and 64 indexes it: the UPDATE is
|
||||
# what the index is for, so it runs once both are in place.
|
||||
if version == PARENT_BACKFILL_VERSION:
|
||||
_backfill_parents(conn)
|
||||
current = version
|
||||
_set_version(conn, current)
|
||||
_encrypt_plaintext_api_keys(conn)
|
||||
|
||||
Reference in New Issue
Block a user