From 31e3d69d8d75ba1cfbc2fce52f353554a29fe6ba Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:36:24 -0700 Subject: [PATCH] R1: split "where am I" from "how far have I walked" Backtracking (revisit R2) breaks the assumption every zone-run caller quietly relied on: that CurrentRoom == len(VisitedNodes)-1. Position and progress have been the same number only because navigation was forward-only. Split them. CurrentRoom is now the first-entry index of CurrentNode in VisitedNodes -- the room number the player was shown on the way in, and the salt that enemy/trap/harvest/encounter keys hash. A revisited room therefore resolves to the same room. RoomsTraversed is a new monotonic step counter, persisted, backfilled from visited_nodes for in-flight rows. The audit found only two progress-shaped reads. narrationCadence moves to RoomsTraversed so a backtracking player draws fresh flavor rather than replaying the entry-room lines. The other was a latent bug: zoneCmdGo labelled the room it moved into as CurrentRoom+1, which is only correct at the frontier; advanceZoneRunNode now returns the true path index. VisitedNodes becomes an ordered set and RoomsCleared becomes idempotent -- both no-ops while navigation is forward-only, both load-bearing after R2. No player-visible behavior change. Nothing to route off RoomsTraversed for threat/SU: verified at HEAD that movement charges neither. Threat comes from combat, supplies burn per day. The revisit plan's cost model claimed otherwise and has been corrected -- R2's "discount" is really a net-new cost decision. --- internal/db/db.go | 7 + internal/plugin/adventure.go | 1 + internal/plugin/bootstrap_rooms_traversed.go | 35 +++ internal/plugin/dnd_expedition_cmd.go | 2 +- internal/plugin/dnd_zone_cmd_graph.go | 6 +- internal/plugin/dnd_zone_narration.go | 22 +- internal/plugin/dnd_zone_run.go | 94 +++++-- internal/plugin/zone_graph_nav.go | 30 ++- internal/plugin/zone_revisit_r1_test.go | 257 +++++++++++++++++++ 9 files changed, 418 insertions(+), 36 deletions(-) create mode 100644 internal/plugin/bootstrap_rooms_traversed.go create mode 100644 internal/plugin/zone_revisit_r1_test.go diff --git a/internal/db/db.go b/internal/db/db.go index 6749d0c..b37c8c8 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -345,6 +345,13 @@ func runMigrations(d *sql.DB) error { // needs no bootstrap backfill. `ALTER TABLE adventure_inventory ADD COLUMN temper INTEGER NOT NULL DEFAULT 0`, `ALTER TABLE magic_item_equipped ADD COLUMN temper INTEGER NOT NULL DEFAULT 0`, + // Revisit R1 (gogobee_revisit_plan.md §R1). Until now "how far along + // is this run" and "which room am I standing in" were the same number, + // both read off len(visited_nodes). Backtracking splits them: the + // path index stops being monotonic, so effort gets its own counter. + // DEFAULT 0 is wrong for in-flight rows — bootstrapRoomsTraversed + // backfills them from visited_nodes. + `ALTER TABLE dnd_zone_run ADD COLUMN rooms_traversed INTEGER NOT NULL DEFAULT 0`, } for _, stmt := range columnMigrations { if _, err := d.Exec(stmt); err != nil { diff --git a/internal/plugin/adventure.go b/internal/plugin/adventure.go index a149b50..da45697 100644 --- a/internal/plugin/adventure.go +++ b/internal/plugin/adventure.go @@ -177,6 +177,7 @@ func (p *AdventurePlugin) Init() error { // through the L4-L5h dual-write soak (fresh deploys, restored backups). bootstrapPlayerMetaFromLegacy() bootstrapRestoreExpeditionStreakDecay() + bootstrapRoomsTraversed() // Rehydrate DM room mappings for existing characters chars, err := loadAllAdvCharacters() diff --git a/internal/plugin/bootstrap_rooms_traversed.go b/internal/plugin/bootstrap_rooms_traversed.go new file mode 100644 index 0000000..a7a7043 --- /dev/null +++ b/internal/plugin/bootstrap_rooms_traversed.go @@ -0,0 +1,35 @@ +package plugin + +import ( + "log/slog" + + "gogobee/internal/db" +) + +// bootstrapRoomsTraversed backfills dnd_zone_run.rooms_traversed for rows +// that predate the revisit R1 column. Before R1 the step counter was implicit +// — len(visited_nodes) — because navigation was forward-only, so replaying +// that identity is an exact reconstruction, not an estimate. +// +// The ALTER lands DEFAULT 0, which would tell an in-flight run it has walked +// nowhere: ambient narration cadence would reset to the entry-room line and +// R2's revisit preflight would read a fresh run. Hence the backfill. +// +// Idempotent, and safe to run against live rows: every row written after R1 +// inserts rooms_traversed >= 1 (the entry node counts as one traversal), so +// `rooms_traversed = 0` uniquely identifies a row this has not yet touched. +// Kept in-tree permanently — a fresh deploy restoring an old backup needs it +// (feedback_loader_rewire_needs_bootstrap). +func bootstrapRoomsTraversed() { + res, err := db.Get().Exec(` + UPDATE dnd_zone_run + SET rooms_traversed = MAX(json_array_length(visited_nodes), 1) + WHERE rooms_traversed = 0`) + if err != nil { + slog.Error("bootstrap: rooms_traversed backfill failed", "err", err) + return + } + if n, _ := res.RowsAffected(); n > 0 { + slog.Warn("bootstrap: rooms_traversed backfilled from visited_nodes", "rows", n) + } +} diff --git a/internal/plugin/dnd_expedition_cmd.go b/internal/plugin/dnd_expedition_cmd.go index b01ce9f..e9f405c 100644 --- a/internal/plugin/dnd_expedition_cmd.go +++ b/internal/plugin/dnd_expedition_cmd.go @@ -692,7 +692,7 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf if chosen == nil { return false // nothing unlocked — leave it for the player / reaper } - if err := advanceZoneRunNode(run.RunID, chosen.To); err != nil { + if _, err := advanceZoneRunNode(run.RunID, chosen.To); err != nil { slog.Warn("expedition: auto-pick stale fork", "user", run.UserID, "run", run.RunID, "err", err) return false diff --git a/internal/plugin/dnd_zone_cmd_graph.go b/internal/plugin/dnd_zone_cmd_graph.go index 3b99dd1..48eb8c2 100644 --- a/internal/plugin/dnd_zone_cmd_graph.go +++ b/internal/plugin/dnd_zone_cmd_graph.go @@ -62,7 +62,7 @@ func (p *AdventurePlugin) advanceTransitionGraph(run *DungeonRun, zone ZoneDefin if len(choices) == 1 && len(unlocked) == 1 { // Plain auto-advance — no fork to prompt for. chosen := choices[0] - if aerr := advanceZoneRunNode(run.RunID, chosen.To); aerr != nil { + if _, aerr := advanceZoneRunNode(run.RunID, chosen.To); aerr != nil { err = aerr return } @@ -184,7 +184,8 @@ func (p *AdventurePlugin) zoneCmdGo(ctx MessageContext, rest string) error { if cerr != nil { return p.SendDM(ctx.Sender, cerr.Error()) } - if aerr := advanceZoneRunNode(run.RunID, chosen.To); aerr != nil { + nextIdx, aerr := advanceZoneRunNode(run.RunID, chosen.To) + if aerr != nil { return p.SendDM(ctx.Sender, "Couldn't advance: "+aerr.Error()) } markActedToday(ctx.Sender) @@ -194,7 +195,6 @@ func (p *AdventurePlugin) zoneCmdGo(ctx MessageContext, rest string) error { nextNode := g.Nodes[chosen.To] fireGraphRegionTransition(run.UserID, fromNode, nextNode) nextRoom := nodeKindToRoomType(nextNode.Kind) - nextIdx := run.CurrentRoom + 1 var b strings.Builder if kind := autoBreakCampOnMove(ctx.Sender); kind != "" { b.WriteString(fmt.Sprintf("⛺ Camp struck (**%s**) — the party moved on.\n\n", kind)) diff --git a/internal/plugin/dnd_zone_narration.go b/internal/plugin/dnd_zone_narration.go index b09e40c..8602a85 100644 --- a/internal/plugin/dnd_zone_narration.go +++ b/internal/plugin/dnd_zone_narration.go @@ -330,16 +330,26 @@ func eliteRoomEntryLine(zoneID ZoneID, runID string, roomIdx int) string { return "🎭 **TwinBee:** " + line } -// narrationCadence returns the salt index used to seed narration -// pickers — the count of nodes visited so far (0-based, matching the -// pre-G6 CurrentRoom semantics). Both linear-mode and graph-mode -// advance bump len(VisitedNodes) in lockstep with CurrentRoom, so this -// preserves existing pick determinism while letting G9 drop the legacy -// current_room column without re-keying every flavor pool seed. +// narrationCadence returns the salt index used to seed narration pickers — +// the number of rooms walked so far, 0-based. This is a *progress* read, not +// a position read (revisit R1's audit split): flavor should keep moving +// forward as the player does, so a backtrack draws fresh lines rather than +// replaying the ones they already read on the way in. +// +// Forward-only runs have RoomsTraversed == len(VisitedNodes), so this stays +// numerically identical to the pre-R1 derivation and every existing pick +// keeps landing on the same line. It is stable while the player stands +// still, which is what lets a re-read of `!status` print the same prose. +// +// The len(VisitedNodes) fallback covers rows that predate the R1 column and +// have yet to be swept by bootstrapRoomsTraversed. func narrationCadence(run *DungeonRun) int { if run == nil { return 0 } + if run.RoomsTraversed > 0 { + return run.RoomsTraversed - 1 + } if n := len(run.VisitedNodes); n > 0 { return n - 1 } diff --git a/internal/plugin/dnd_zone_run.go b/internal/plugin/dnd_zone_run.go index 6e32dee..d19c061 100644 --- a/internal/plugin/dnd_zone_run.go +++ b/internal/plugin/dnd_zone_run.go @@ -76,6 +76,14 @@ type DungeonRun struct { CurrentNode string VisitedNodes []string NodeChoices map[string]any + + // Revisit R1 — monotonic count of node entries, including the entry + // room. Distinct from CurrentRoom: CurrentRoom answers "where am I on + // the path" and moves backwards when the player backtracks, while + // RoomsTraversed answers "how much walking has this run cost" and only + // ever climbs. Forward-only navigation keeps them locked together at + // RoomsTraversed == CurrentRoom+1; revisit is what pulls them apart. + RoomsTraversed int } // IsActive reports whether this run is still ongoing (not boss-defeated, @@ -244,12 +252,16 @@ func startZoneRun(userID id.UserID, zoneID ZoneID, dndLevel int, rng *rand.Rand) run.CurrentNode = entryNode run.VisitedNodes = []string{entryNode} run.NodeChoices = map[string]any{} + // Standing in the entry room is one traversal — the player walked in. + // Starting at 1 also keeps `rooms_traversed = 0` free as the + // "never backfilled" sentinel bootstrapRoomsTraversed keys on. + run.RoomsTraversed = 1 if _, err := db.Get().Exec(` INSERT INTO dnd_zone_run (run_id, user_id, zone_id, total_rooms, rooms_cleared, gm_mood, - current_node, visited_nodes, node_choices) - VALUES (?, ?, ?, ?, '[]', ?, ?, ?, '{}')`, + current_node, visited_nodes, node_choices, rooms_traversed) + VALUES (?, ?, ?, ?, '[]', ?, ?, ?, '{}', 1)`, run.RunID, run.UserID, string(zoneID), run.TotalRooms, startMood, entryNode, string(visitedJSON), ); err != nil { @@ -272,7 +284,7 @@ func getActiveZoneRun(userID id.UserID) (*DungeonRun, error) { SELECT run_id, user_id, zone_id, total_rooms, rooms_cleared, boss_defeated, abandoned, loot_collected, gm_mood, started_at, last_action_at, completed_at, - current_node, visited_nodes, node_choices + current_node, visited_nodes, node_choices, rooms_traversed FROM dnd_zone_run WHERE user_id = ? AND completed_at IS NULL @@ -312,7 +324,7 @@ func getZoneRun(runID string) (*DungeonRun, error) { SELECT run_id, user_id, zone_id, total_rooms, rooms_cleared, boss_defeated, abandoned, loot_collected, gm_mood, started_at, last_action_at, completed_at, - current_node, visited_nodes, node_choices + current_node, visited_nodes, node_choices, rooms_traversed FROM dnd_zone_run WHERE run_id = ?`, runID) r, err := scanZoneRun(row) if errors.Is(err, sql.ErrNoRows) { @@ -343,7 +355,7 @@ func scanZoneRun(row scanner) (*DungeonRun, error) { &r.RunID, &r.UserID, &zoneID, &r.TotalRooms, &clearedJSON, &bossDefeatedI, &abandonedI, &lootJSON, &r.DMMood, &r.StartedAt, &r.LastActionAt, &completedAtRaw, - ¤tNode, &visitedJSON, &choicesJSON, + ¤tNode, &visitedJSON, &choicesJSON, &r.RoomsTraversed, ); err != nil { return nil, err } @@ -390,14 +402,16 @@ func scanZoneRun(row scanner) (*DungeonRun, error) { r.NodeChoices = map[string]any{} } r.CurrentNode = currentNode - // G9b: CurrentRoom is now derived from VisitedNodes — no longer - // persisted in current_room. The downstream callers that key on - // CurrentRoom (combat enemy/trap salts, status renders, harvest - // keys) stay numerically identical to the dual-write era because - // VisitedNodes was bumped in lockstep with current_room. - if n := len(r.VisitedNodes); n > 0 { - r.CurrentRoom = n - 1 - } + // G9b: CurrentRoom is derived from VisitedNodes — no longer persisted + // in current_room. Revisit R1 re-derives it as the *first-entry index + // of CurrentNode* rather than len(VisitedNodes)-1. The two agree for + // every forward-only run (each advance appends the node it moves to, + // so the newest node is always the current one), but they diverge the + // moment a player backtracks. Keying on the node — not the tail — is + // what makes a revisited room resolve to its original identity: the + // enemy/trap salts, harvest keys and encounter IDs downstream all + // hash CurrentRoom, so room 3 must stay room 3 on the way back. + r.CurrentRoom = pathIndexOf(r.VisitedNodes, r.CurrentNode) if r.CurrentNode == "" { // Defensive: a row from before the G4 dual-write deploy that // somehow survived 24h inactivity timeouts. Pin to the linear @@ -407,6 +421,53 @@ func scanZoneRun(row scanner) (*DungeonRun, error) { return &r, nil } +// appendVisited records a node entry in first-entry order. A node already +// in the path is not re-appended: VisitedNodes is an ordered *set*, and the +// player's room numbers must not renumber themselves when they walk back +// through a room they've already seen. The step cost of that walk is +// carried by rooms_traversed, not by this slice. +func appendVisited(visited []string, node string) []string { + for _, n := range visited { + if n == node { + return visited + } + } + return append(visited, node) +} + +// appendClearedRoom marks a room index resolved. Idempotent: walking out of +// a room a second time doesn't re-append. "Cleared" is a sticky property of +// the room, not a count of exits — re-appending would let a backtracking +// player inflate RoomsCleared past TotalRooms and skew every "N/M rooms" +// render that reads its length. +func appendClearedRoom(cleared []int, room int) []int { + for _, r := range cleared { + if r == room { + return cleared + } + } + return append(cleared, room) +} + +// pathIndexOf returns the first-entry index of node within visited — the +// 0-based room number the player sees in `!map`'s Path strip. Revisits do +// not append, so a node's index is stable for the life of the run. +// +// A node missing from visited means a row whose current_node was hot-swapped +// in without a matching visit record (pre-G4 rows, defensive only). Fall back +// to the tail, which is what the pre-R1 derivation would have produced. +func pathIndexOf(visited []string, node string) int { + for i, n := range visited { + if n == node { + return i + } + } + if n := len(visited); n > 0 { + return n - 1 + } + return 0 +} + // deriveLegacyNodeID returns the node id a linear graph would assign // to position roomIdx (0-based) for the given zone. Mirrors // BuildLinearGraph's ".r" scheme so hot-swapped rows align @@ -439,7 +500,7 @@ func markRoomCleared(runID string) (RoomType, error) { if !ok { return "", fmt.Errorf("no graph for zone %q", r.ZoneID) } - cleared := append(r.RoomsCleared, r.CurrentRoom) + cleared := appendClearedRoom(r.RoomsCleared, r.CurrentRoom) clearedJSON, _ := json.Marshal(cleared) edges := g.outgoingEdges(r.CurrentNode) @@ -461,13 +522,14 @@ func markRoomCleared(runID string) (RoomType, error) { return "", nil } nextNode := edges[0].To - visited := append(r.VisitedNodes, nextNode) + visited := appendVisited(r.VisitedNodes, nextNode) visitedJSON, _ := json.Marshal(visited) if _, err := db.Get().Exec(` UPDATE dnd_zone_run SET rooms_cleared = ?, current_node = ?, visited_nodes = ?, + rooms_traversed = rooms_traversed + 1, last_action_at = CURRENT_TIMESTAMP WHERE run_id = ?`, string(clearedJSON), nextNode, string(visitedJSON), runID, @@ -476,6 +538,8 @@ func markRoomCleared(runID string) (RoomType, error) { } r.CurrentNode = nextNode r.VisitedNodes = visited + r.RoomsTraversed++ + r.CurrentRoom = pathIndexOf(visited, nextNode) return nodeKindToRoomType(g.Nodes[nextNode].Kind), nil } diff --git a/internal/plugin/zone_graph_nav.go b/internal/plugin/zone_graph_nav.go index 9015436..e58f513 100644 --- a/internal/plugin/zone_graph_nav.go +++ b/internal/plugin/zone_graph_nav.go @@ -315,7 +315,7 @@ func recordRoomCleared(runID string) (*DungeonRun, error) { if !r.IsActive() { return nil, ErrNoActiveRun } - cleared := append(r.RoomsCleared, r.CurrentRoom) + cleared := appendClearedRoom(r.RoomsCleared, r.CurrentRoom) clearedJSON, _ := json.Marshal(cleared) if _, err := db.Get().Exec(` UPDATE dnd_zone_run @@ -376,28 +376,36 @@ func completeRunAtNode(runID string, boss bool) error { return err } -// advanceZoneRunNode moves a run to nextNode: appends to visited_nodes, -// sets current_node, and clears any pending fork prompt. Caller is -// expected to have already called recordRoomCleared for the prior node. -func advanceZoneRunNode(runID, nextNode string) error { +// advanceZoneRunNode moves a run to nextNode: records the node entry in +// visited_nodes, sets current_node, bumps the traversal counter, and clears +// any pending fork prompt. Caller is expected to have already called +// recordRoomCleared for the prior node. +// +// Returns the moved-to room's path index so callers can label it without +// assuming CurrentRoom+1 — an assumption that only holds while the player +// is walking the frontier. +func advanceZoneRunNode(runID, nextNode string) (int, error) { r, err := getZoneRun(runID) if err != nil { - return err + return 0, err } if r == nil { - return ErrNoActiveRun + return 0, ErrNoActiveRun } - visited := append(r.VisitedNodes, nextNode) + visited := appendVisited(r.VisitedNodes, nextNode) visitedJSON, _ := json.Marshal(visited) - _, err = db.Get().Exec(` + if _, err := db.Get().Exec(` UPDATE dnd_zone_run SET current_node = ?, visited_nodes = ?, node_choices = '{}', + rooms_traversed = rooms_traversed + 1, last_action_at = CURRENT_TIMESTAMP WHERE run_id = ?`, - nextNode, string(visitedJSON), runID) - return err + nextNode, string(visitedJSON), runID); err != nil { + return 0, err + } + return pathIndexOf(visited, nextNode), nil } // resolveForkChoice takes a 1-based choice index against a pending diff --git a/internal/plugin/zone_revisit_r1_test.go b/internal/plugin/zone_revisit_r1_test.go new file mode 100644 index 0000000..d848a87 --- /dev/null +++ b/internal/plugin/zone_revisit_r1_test.go @@ -0,0 +1,257 @@ +package plugin + +import ( + "encoding/json" + "testing" + + "gogobee/internal/db" + "maunium.net/go/mautrix/id" +) + +// Revisit R1 (gogobee_revisit_plan.md §R1) — the structural split between +// "where am I on the path" (CurrentRoom) and "how far have I walked" +// (RoomsTraversed). No player-visible behavior changes in R1; these tests +// pin the invariants R2's `!revisit` will lean on. + +// setupRevisitTestDB builds a schema-only database in a temp dir. Unlike +// setupZoneRunTestDB it does not copy the operator's data/gogobee.db, so it +// runs everywhere instead of skipping on any machine that lacks one — these +// tests assert the R1 invariants, which no prod row is needed to exercise. +func setupRevisitTestDB(t *testing.T) { + t.Helper() + db.Close() + if err := db.Init(t.TempDir()); err != nil { + t.Fatalf("db.Init: %v", err) + } + t.Cleanup(db.Close) +} + +// ── pure helpers ──────────────────────────────────────────────────────────── + +func TestPathIndexOf_ReturnsFirstEntryIndex(t *testing.T) { + // A backtracked path: the player walked r1→r2→r3, then back to r2. + // r2's room number must stay 1 — the number they were shown on the way + // in, and the salt every enemy/trap/harvest key downstream hashes. + visited := []string{"z.r1", "z.r2", "z.r3"} + if got := pathIndexOf(visited, "z.r2"); got != 1 { + t.Errorf("pathIndexOf(r2) = %d, want 1", got) + } + if got := pathIndexOf(visited, "z.r3"); got != 2 { + t.Errorf("pathIndexOf(r3) = %d, want 2", got) + } +} + +func TestPathIndexOf_FallsBackToTailWhenAbsent(t *testing.T) { + // Pre-G4 rows carry a current_node that was never recorded as visited. + // The fallback must reproduce the pre-R1 derivation (len-1), not 0 — + // pinning such a row to the entry room would re-fire the entry encounter. + visited := []string{"z.r1", "z.r2", "z.r3"} + if got := pathIndexOf(visited, "z.nowhere"); got != 2 { + t.Errorf("pathIndexOf(absent) = %d, want tail index 2", got) + } + if got := pathIndexOf(nil, "z.r1"); got != 0 { + t.Errorf("pathIndexOf(empty) = %d, want 0", got) + } +} + +func TestAppendVisited_DoesNotRenumberOnRevisit(t *testing.T) { + visited := []string{"z.r1", "z.r2"} + again := appendVisited(visited, "z.r1") + if len(again) != 2 { + t.Fatalf("re-entering a visited node appended: %v", again) + } + fresh := appendVisited(again, "z.r3") + if len(fresh) != 3 || fresh[2] != "z.r3" { + t.Errorf("new node not appended in order: %v", fresh) + } +} + +func TestAppendClearedRoom_IsIdempotent(t *testing.T) { + // Walking out of room 1 twice must not inflate RoomsCleared — its + // length feeds "N/M rooms" renders and would eventually exceed TotalRooms. + cleared := appendClearedRoom([]int{0, 1}, 1) + if len(cleared) != 2 { + t.Fatalf("re-clearing room 1 appended: %v", cleared) + } + if got := appendClearedRoom(cleared, 2); len(got) != 3 { + t.Errorf("fresh room not appended: %v", got) + } +} + +// ── the load-bearing property: CurrentRoom tracks the node, not the tail ──── + +func TestCurrentRoom_DerivesFromCurrentNodeNotVisitedTail(t *testing.T) { + setupRevisitTestDB(t) + uid := id.UserID("@r1-backtrack:example") + + run, err := startZoneRun(uid, ZoneGoblinWarrens, 1, nil) + if err != nil { + t.Fatalf("startZoneRun: %v", err) + } + // Walk forward two rooms so VisitedNodes has a tail distinct from + // the node we're about to stand on. + for i := 0; i < 2; i++ { + if _, err := markRoomCleared(run.RunID); err != nil { + t.Fatalf("markRoomCleared %d: %v", i, err) + } + } + fwd, err := getZoneRun(run.RunID) + if err != nil || fwd == nil { + t.Fatalf("reload: %v", err) + } + if len(fwd.VisitedNodes) != 3 { + t.Fatalf("expected 3 visited nodes, got %v", fwd.VisitedNodes) + } + if fwd.CurrentRoom != 2 { + t.Errorf("forward CurrentRoom = %d, want 2", fwd.CurrentRoom) + } + + // Simulate the backtrack R2 will perform: current_node moves back to + // the first room, visited_nodes is untouched. Pre-R1 this read as + // room 2 (the tail); it must now read as room 0. + backTo := fwd.VisitedNodes[0] + if _, err := db.Get().Exec( + `UPDATE dnd_zone_run SET current_node = ? WHERE run_id = ?`, + backTo, run.RunID); err != nil { + t.Fatalf("backtrack write: %v", err) + } + back, err := getZoneRun(run.RunID) + if err != nil || back == nil { + t.Fatalf("reload after backtrack: %v", err) + } + if back.CurrentRoom != 0 { + t.Errorf("after backtrack CurrentRoom = %d, want 0 (the node's first-entry index)", back.CurrentRoom) + } + if len(back.VisitedNodes) != 3 { + t.Errorf("backtrack must not shrink VisitedNodes: %v", back.VisitedNodes) + } + // The step counter does not rewind with the player. + if back.RoomsTraversed != 3 { + t.Errorf("RoomsTraversed = %d, want 3 (monotonic)", back.RoomsTraversed) + } +} + +// ── R1's stated acceptance: forward-only trajectories are unchanged ───────── + +func TestForwardOnlyRun_KeepsTraversalLockedToPathIndex(t *testing.T) { + setupRevisitTestDB(t) + uid := id.UserID("@r1-forward:example") + + run, err := startZoneRun(uid, ZoneGoblinWarrens, 1, nil) + if err != nil { + t.Fatalf("startZoneRun: %v", err) + } + if run.RoomsTraversed != 1 { + t.Fatalf("fresh run RoomsTraversed = %d, want 1 (entry room walked into)", run.RoomsTraversed) + } + + for step := 0; step < 3; step++ { + if _, err := markRoomCleared(run.RunID); err != nil { + t.Fatalf("markRoomCleared %d: %v", step, err) + } + cur, err := getZoneRun(run.RunID) + if err != nil || cur == nil { + t.Fatalf("reload %d: %v", step, err) + } + // This is the identity every pre-R1 caller silently assumed. As long + // as navigation is forward-only it must keep holding, or R1 has + // changed behavior it promised not to. + if cur.RoomsTraversed != cur.CurrentRoom+1 { + t.Errorf("step %d: RoomsTraversed=%d, CurrentRoom=%d — want traversed == room+1", + step, cur.RoomsTraversed, cur.CurrentRoom) + } + if cur.RoomsTraversed != len(cur.VisitedNodes) { + t.Errorf("step %d: RoomsTraversed=%d, len(VisitedNodes)=%d — want equal on a forward-only run", + step, cur.RoomsTraversed, len(cur.VisitedNodes)) + } + // narrationCadence is the pre-R1 salt: len(VisitedNodes)-1. Any drift + // here silently re-keys every flavor pool for in-flight runs. + if got, want := narrationCadence(cur), len(cur.VisitedNodes)-1; got != want { + t.Errorf("step %d: narrationCadence = %d, want %d (pre-R1 salt)", step, got, want) + } + } +} + +func TestNarrationCadence_TracksTraversalNotPosition(t *testing.T) { + // A backtracked run: standing in room 0, but three rooms walked. Flavor + // should keep advancing, so a player who doubles back reads new lines + // instead of the entry-room narration a second time. + run := &DungeonRun{ + VisitedNodes: []string{"z.r1", "z.r2", "z.r3"}, + CurrentNode: "z.r1", + CurrentRoom: 0, + RoomsTraversed: 4, + } + if got := narrationCadence(run); got != 3 { + t.Errorf("narrationCadence = %d, want 3 (traversals-1), not the room index", got) + } + // Rows not yet swept by the bootstrap fall back to the visited count. + stale := &DungeonRun{VisitedNodes: []string{"z.r1", "z.r2"}} + if got := narrationCadence(stale); got != 1 { + t.Errorf("un-backfilled row cadence = %d, want 1", got) + } +} + +// ── bootstrap ─────────────────────────────────────────────────────────────── + +func TestBootstrapRoomsTraversed_BackfillsFromVisitedNodes(t *testing.T) { + setupRevisitTestDB(t) + uid := id.UserID("@r1-bootstrap:example") + + run, err := startZoneRun(uid, ZoneGoblinWarrens, 1, nil) + if err != nil { + t.Fatalf("startZoneRun: %v", err) + } + visited := []string{"gw.r1", "gw.r2", "gw.r3", "gw.r4"} + visitedJSON, _ := json.Marshal(visited) + // Reproduce a row as the ALTER leaves it: a real path, counter at 0. + if _, err := db.Get().Exec( + `UPDATE dnd_zone_run SET visited_nodes = ?, rooms_traversed = 0 WHERE run_id = ?`, + string(visitedJSON), run.RunID); err != nil { + t.Fatalf("seed pre-R1 row: %v", err) + } + + bootstrapRoomsTraversed() + + got, err := getZoneRun(run.RunID) + if err != nil || got == nil { + t.Fatalf("reload: %v", err) + } + if got.RoomsTraversed != 4 { + t.Errorf("RoomsTraversed = %d, want 4 (len(visited_nodes))", got.RoomsTraversed) + } + + // Idempotent: a second sweep must not touch the row it already fixed, + // nor the counter of a run that has since walked on. + if _, err := db.Get().Exec( + `UPDATE dnd_zone_run SET rooms_traversed = 9 WHERE run_id = ?`, run.RunID); err != nil { + t.Fatalf("bump: %v", err) + } + bootstrapRoomsTraversed() + again, _ := getZoneRun(run.RunID) + if again.RoomsTraversed != 9 { + t.Errorf("re-run clobbered a live counter: got %d, want 9", again.RoomsTraversed) + } +} + +// An empty visited_nodes must not backfill to 0 and re-arm the sentinel — +// the row would be swept on every single boot. +func TestBootstrapRoomsTraversed_FloorsAtOne(t *testing.T) { + setupRevisitTestDB(t) + uid := id.UserID("@r1-empty-path:example") + + run, err := startZoneRun(uid, ZoneGoblinWarrens, 1, nil) + if err != nil { + t.Fatalf("startZoneRun: %v", err) + } + if _, err := db.Get().Exec( + `UPDATE dnd_zone_run SET visited_nodes = '[]', rooms_traversed = 0 WHERE run_id = ?`, + run.RunID); err != nil { + t.Fatalf("seed: %v", err) + } + bootstrapRoomsTraversed() + got, _ := getZoneRun(run.RunID) + if got.RoomsTraversed != 1 { + t.Errorf("RoomsTraversed = %d, want 1 (floor)", got.RoomsTraversed) + } +}