# TODO Backlog of bugs + long-term items, from user. Milestones live in REWORK_PLAN.md. ## Feature backlog ### FEAT-M6: Transactional undo (moved from REWORK_PLAN) - Every mutating action writes event: `(type, payload, undo_payload, undone, ts)`. - Undo = apply `undo_payload` in same SQLite tx, flip `undone`. Transactional, no stale clobber. - Replaces fragile `/logs` snapshot-write undo. - Migration: keep old undo working for existing entries until cleared; new format for new entries. - Related: BUG-7 (reorder no undo). ## Architecture: 1-list turn order model (DONE) - Single source: turnOrderIds === participants.map(id). No re-sort after startEncounter. nextTurn skips inactive (predicate), inactive stay in slot. - Drag (reorder) overrides initiative — cross-init allowed, DM choice. - startEncounter sorts ALL participants by init once, then frozen. - addParticipant splices by init pos. remove/toggle/reorder sync list. - Display renders participants[] directly (no sortParticipantsByInitiative). - BUG-6 (reorder divergence) fixed structurally. BUG-5 (rotation) held (500 rounds CLEAN). ### FEAT-3: initiative first-class entry (add + edit) - Current: only initMod at char-build. No initiative field at add-participant or edit. 3-step to set after other steps. - Need: initiative field at add-char, add-monster, AND edit participant. - Separate design + RED. Own work item. - Related: tie-break = drag order (current, works). Expose clearly. ### FEAT-1: Dead participants stay in turn order — DONE - Fixed: `applyHpChange` no longer flips `isActive` or touches `turnOrderIds` on death/revive. Dead stay in rotation, `nextTurn` visits them, PCs get death-save turn. `isActive` = DM toggle only. - Tests: `shared/tests/turn.dead-skip.test.js` (4 green). Char tests updated to new behavior. ### FEAT-2: upgrade app internal logs to be parseable - Goal: combat logs in Firestore store enough structured state to run skip/rotation analysis on ANY historic round — not just replay stdout. - Current logs: `{timestamp, message, encounterName, undo}`. Parser must guess roster from message strings. Brittle. - Upgrade: add structured fields at turn-state mutation log sites in App.js (startEncounter, toggleActive, addParticipant, removeParticipant, applyHpChange death/revive, togglePause, nextTurn): ``` turnSnapshot: { round, currentTurnParticipantId, turnOrderIds, activeIds } ``` - Then `scripts/analyze-turns.js` ingests app logs directly (adapter fetch). Works on real game sessions, any round, deterministic. - Parser scaffold NOW ingests replay stdout only (stopgap until FEAT-2). ## Confirmed bugs (tests written, NOT fixed) ### BUG-1: addParticipant + pause/resume corrupts turn rotation - **RESOLVED** as side effect of BUG-2 fix (dup-id rejection broke chain). - Audit: 0 violations / 100 rounds after BUG-2 fix. - Replay: 10 rounds clean, no skip/dupe. - Audit: 128 violations / 100 rounds, 4 symptom faces. - Symptom chain (one bug family): 1. pause blocks nextTurn advance → totalTurns stays frozen (e.g. 120) 2. addParticipant re-adds same `r${totalTurns}` id (BUG-2: no dedup) 3. togglePause resume rebuilds turnOrderIds → dup id appears x2 4. nextTurn gets stuck on dup id → rotation breaks 5. eventually nextTurn throws 'Encounter not running' - Symptom counts (audit-state.js, 100 rounds): 62x turnOrder-no-dup, 52x rotation-dupes, 14x nextTurn-throws - Repro in replay round 10+: current stuck on one participant forever, nextTurn returns same id, round never advances. - Clean minimal repro (shared/tests/turn.pause-add.test.js) PASSES = combo needs more state than single add+pause. Audit authoritative repro. - Clean subsystems (zero violations): HP bounds, isActive, deathSave range, conditions, removeParticipant orphans. - Real repro = `node scripts/audit-state.js` (or audit-rotation.js). ### BUG-2: addParticipant allows duplicate id - **FIXED** (commit: addParticipant throws on dup id). - Test: `shared/tests/turn.characterization.test.js` 'addParticipant rejects duplicate id' --- GREEN. ### bug-3 was a halucination has been removed ### BUG-4: hide-player-HP breaks display view (preexisting) - **Broader than hide-HP**: ALL 5 `storage.setDoc(getPath.activeDisplay(), ...)` calls use `{merge:true}` which is IGNORED (setDoc = replace per contract). Each write clobbers other fields on activeDisplay/status doc. - line 1619: hide-HP toggle → clobbers campaignId+encounterId (display paused) - line 1648: start combat → clobbers hidePlayerHp - line 1779: end combat → clobbers hidePlayerHp - line 1997: deactivate → clobbers hidePlayerHp - line 2002: activate → clobbers hidePlayerHp - Toggle "hide player HP" in admin → display view flips to "Game Session Paused". - Toggling back does NOT recover. Must re-activate encounter in encounters panel to restore display. - Expected: hide-HP toggle updates one field on activeDisplay/status doc, display stays live on current encounter. - Likely cause: toggle writes to wrong path, or clobbers activeCampaignId/ activeEncounterId with null (setDoc replace vs updateDoc patch). - Fix: use updateDoc (patch) not setDoc (replace); or include all existing fields when writing. - Test: render App + DisplayView, toggle hide-HP, assert display still shows encounter (not paused). ### BUG-5: mid-round addParticipant/revive corrupts rotation — FIXED - Fixed (commit `494327f`). Slot-array turn order + DRY advance core `nextActiveAfter`. Both nextTurn + computeTurnOrderAfterRemoval delegate. - 500-round replay: 0 skips, 0 double-acts. ### BUG-6: reorderParticipants doesn't update turnOrderIds — FIXED - Fixed structurally by 1-list model (commit 5d3a060). turnOrderIds = participants.map(id) always. reorder cross-init allowed (DM override). Display === rotation by construction. - Test: `shared/tests/turn.reorder.test.js` 'reorder updates turnOrderIds' (RED). - `reorderParticipants(enc, draggedId, targetId)` swaps two same-initiative participants in `participants[]` array but leaves `turnOrderIds` unchanged. - nextTurn rotates via `turnOrderIds` only → reorder has NO effect on combat rotation. Mid-encounter drag-drop = pointless. - replay-combat.js calls reorderParticipants with WRONG signature `(enc, reorderedArray)` --- swallowed by try/catch, silent no-op. So replay never exercised real path either. - Fix: reorder must also update turnOrderIds to match new participant order (within same-initiative tie). ### BUG-7: reorderParticipants has no undo - Test: `shared/tests/turn.undo.test.js` 'reorderParticipants has no undo' (GREEN doc). - `reorderParticipants` returns `log: null`. Other ops return `log.undo`. - Cannot undo drag-drop. Candidate for undo system (M6). ### BUG-8: ws adapter has no reconnect - Test: `server/tests/ws-reconnect.test.js` (RED). - WS dies (idle/error/close) → `wsReady=null`, subscribers dead forever. - Display frozen until full reload. - Fix: `onclose` → reconnect + re-subscribe existing paths. ### BUG-10: deact+reactivate same round double-acts participant - Discovered in 500-round replay (3 occurrences). DISTINCT from BUG-5. - Pattern: participant acts → DM deactivates them → DM reactivates them same round → `computeTurnOrderAfterAddition` re-inserts by initiative (front) → acts AGAIN before round ends. - No "acted-this-round" guard. Slot-array model has no per-round-acted set. - Edge case (DM deact+reactivate same participant same round). - Fix candidate: track actedThisRound set, skip re-acted; OR insertion places reactivate AFTER current position (not by initiative). - Parser now discounts deact-current advances, so this surfaced real. ### BUG-11: FE Combat.scenario test crashes (pre-existing) - `src/tests/Combat.scenario.test.js:254` deathSave query helper throws (button not found). - Baseline (my changes removed) also exit=1. Pre-existing, not regression. - Crashes whole FE test run (process dies). ### BUG-13: reorderParticipants crossing current pointer = ambiguous acted-semantics - Discovered 7/1 replay. `reorderParticipants` (shared/turn.js:522) = pure drag, no pointer logic. Swapping two actors across current pointer mid-round = ambiguous who-acted-this-round. Earlier replay arbitrary swaps showed skip/double (R9 Summon3 2x, R11 Goblin1 2x) before fix restricted swaps to upcoming-only. - Replay now avoids crossing (adjacent upcoming pair only, commit af165f4). Real app untested: if DM drags actor past current pointer mid-round, skip/ double behavior undefined. - Decide: block cross-pointer reorder, or define acted-semantics. RED needed. ### BUG-14: addParticipant init-insertion breaks after drag-reorder - Discovered 7/1 replay. `computeTurnOrderAfterAddition` scans for first id with init < addedInit, assumes list init-sorted. After drag, list NOT sorted → scan hits wrong slot. - Trace turn 30→31: list `[Goblin1:20,Goblin2:22,...]` (drag moved Goblin1 before Goblin2). Add Reinforce3 init 21 → scan hits Goblin1:20 (idx 0, <21) first → insert at 0. Should slot after Goblin2:22. WRONG. - Root conflict: 1-list model = drag source of truth (no re-sort); addParticipant = init-based insertion (needs sorted list). After ANY drag, add-insertion meaningless. - Proposed fix: append to end always (option A). DM drags to position. Matches drag = source of truth. Makes `computeTurnOrderAfterAddition` trivial. - Related: FEAT-3 (initiative first-class field). ## Pipeline (bugs only --- milestones live in REWORK_PLAN.md) - [ ] BUG-4: fix setDoc→updateDoc for all 5 activeDisplay sites - [x] BUG-5: fixed (1-list model, 500 rounds clean) - [x] BUG-6: fixed structurally (1-list model) - [x] BUG-12: fixed — campaign selection follows activeDisplay - [x] BUG-15: fixed — DisplayView no longer re-sorts (drag order preserved) - [x] BUG-8: ws adapter reconnect (implemented + GREEN) - [ ] BUG-10: deact+reactivate double-act - [ ] BUG-11: FE Combat.scenario crash - [ ] BUG-13: reorder cross-pointer semantics (RED + decide block/allow) - [ ] BUG-14: addParticipant init-insert breaks post-drag (append? + RED)