fix(viewer): #826 nav-away mid-narration resilience + #825 full chronicle narration - #869
Conversation
…icle narration Two v1.0.4 viewer orphans from the engine-audit Section-4 coverage check (rc1+rc2 persona evidence with no prior issue). Viewer-scope only; the engine stays sole writer — viewer/server.py is the read-only bridge projecting state the engine already wrote. #826 — Navigating away mid-narration corrupts session state + permanently freezes the scene image (rc2 adversarial). A user-initiated nav DURING an in-flight beat (distinct from the #745/#648 stalled-beat trigger). Two legs: * State corruption (double-fire): postMove armed the one-move gate only AFTER the /move POST resolved, leaving a window where a nav-away/nav-back remounted ScreenTable (fresh submittingRef=false, pending=null) and re-opened the bar — a second move could double-fire the one-move-at-a-time lane. Fix: arm OPTIMISTICALLY before the await (the App-level useLiveSession pending state survives the unmount), and on a POST rejection roll it back via a new, surgical abandonPending that bypasses the #648 arm-grace (an authoritative server rejection is not a spurious clear; it only clears the move we armed, never a newer live turn). Preserves the #745/#746 recovery contract and the #399/#406 first-beat window (armPending/notePendingProgress untouched). * Scene-image freeze (dead handle): chrome.jsx Img latched `failed` on the first onError and only ever cleared it on a SCOPE CHANGE — so a #399 fire-and-forget scene that 404s while its art is still being generated stayed frozen on the placeholder forever, even once the same-scope image became servable. Fix: bounded, backed-off, cache-busted retries so the component RECOVERS when the pending art lands (and stops after the budget for a genuinely-missing image). No dead latch. #825 — Chronicle truncates DM narration mid-word at a fixed ceiling (3 personas: rc2 adversarial+narrative, rc1 veteran). The chronicle's leading history band (_session_recent_events) hard-cut every row to text[:1000], slicing a long DM beat mid-word with no ellipsis/expand. Fix: render the FULL beat (_bounded_chronicle_text) — the render region is already a scrollable role="log" and the #752 a11y bound is the ROW cap (CHRONICLE_RENDER_CAP/MAX_LIVE_BEATS), not a per-row char ceiling, so removing the cut does not reintroduce the #752 a11y flood. Only a generous DoS guard remains (64 KB, far above any real beat), and even it cuts on a word boundary. TDD via the JSX behaviour harness (test_recovery_timing.py pattern): viewer/tests/test_nav_chronicle_resilience.py — a nav-away-during-pending / optimistic-arm-survival + authoritative-abandon test, an Img dead-handle-recovery test, and a long-narration no-mid-word-truncation test (7 tests, all green). Full viewer suite 505 passed / 1 skipped; fast_gate Tier-0 195 passed. Source: docs/audits/ENGINE-AUDIT-2026-06-11.md (Section-4 orphans #1 + #5) Refs #826, #825
|
Warning Review limit reached
More reviews will be available in 1 hour, 26 minutes, and 8 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a4d2d9d21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // with a fresh submittingRef=false AND pending=null — the bar re-opened and a SECOND move could | ||
| // double-fire the one-move-at-a-time lane (the #826 state corruption). recordPlayerEcho is #399- | ||
| // idempotent, so re-POST/retry paths don't duplicate the echo. | ||
| recordPlayerEcho(hero.name, text, cleanMove); |
There was a problem hiding this comment.
This now appends the player's action to the Chronicle before the /move POST is accepted, but the rejection path only clears the optimistic pending state and never removes that echo. When the server rejects a move (for example a stale action, dead move sink, or payload.ok === false), the player gets a "Move not sent" toast while the Chronicle still shows the action as if it happened, and a later retry can leave duplicate/false history in the visible session.
Useful? React with 👍 / 👎.
1 participant
Fixes the two v1.0.4 viewer orphans flagged by the engine-audit Section-4 coverage check (rc1+rc2 persona complaints with no prior issue). Both are viewer-scope only — the engine stays sole writer (
viewer/server.pyis the read-only bridge projecting state the engine already wrote; no campaign-state writes, no wire-contract changes).Source:
docs/audits/ENGINE-AUDIT-2026-06-11.md(Section-4 orphans #1 + #5). Refs #826, #825.#826 — Navigating away mid-narration corrupts session state + freezes the scene image (rc2 adversarial)
A user-initiated nav during an in-flight beat — distinct from the #745/#648 stalled-beat trigger. Two independent legs:
1. State corruption (double-fire).
postMovearmed the one-move gate only after the/movePOST resolved. If the player navigated away (Table→Map) during the in-flight POST and back, ScreenTable remounted with a freshsubmittingRef=falseandpending=null→ the action bar re-opened → a second move could double-fire the one-move-at-a-time lane.useLiveSessionhook, so they survive ScreenTable unmounting. On a POST rejection, roll back via a new, surgicalabandonPending(text)that bypasses the play-loop: Enter-submitted Do doesn't arm the narrating spinner — player perceives no DM (move-sink/pending-state) #648 arm-grace (an authoritative server rejection is not a spurious clear) and only clears the move we armed — never a newer live turn.armPending/notePendingProgressuntouched).2. Scene-image freeze (dead handle).
chrome.jsxImglatchedfailedon the firstonErrorand only ever cleared it on a scope change — so a #399 fire-and-forget scene that 404s while its art is still being generated stayed frozen on the placeholder forever, even once the same-scope image became servable.#825 — Chronicle truncates DM narration mid-word at a fixed ceiling (3 personas: rc2 adversarial+narrative, rc1 veteran)
The chronicle's leading history band (
_session_recent_events) hard-cut every row totext[:1000], slicing a long DM beat mid-word with no ellipsis/expand — the remainder unreadable._bounded_chronicle_text). The render region is already a scrollablerole="log", and the [a11y][major] Chronicle floods the accessibility tree by beat 2 — Do/Declare unreachable for AT users #752 a11y bound is the ROW cap (CHRONICLE_RENDER_CAP/MAX_LIVE_BEATS), not a per-row char ceiling — so removing the cut does not reintroduce the [a11y][major] Chronicle floods the accessibility tree by beat 2 — Do/Declare unreachable for AT users #752 a11y-tree flood. Only a generous DoS guard remains (64 KB, far above any real beat), and even it cuts on a word boundary.text[:1000](terse round summaries, not DM prose) is intentionally out of scope.Tests (TDD via the JSX behaviour harness —
test_recovery_timing.pypattern)New
viewer/tests/test_nav_chronicle_resilience.py(7 tests, all green):abandonPendingrollback (bypasses the play-loop: Enter-submitted Do doesn't arm the narrating spinner — player perceives no DM (move-sink/pending-state) #648 grace) + abandon-is-surgical (won't clobber a newer live turn).Imgrecovers (re-mounts a cache-busted<img>) after a recoverableonError, instead of freezing on a dead handle._session_recent_events— no mid-word truncation; the exact 1000-char regression is pinned.Verification:
7 passed.505 passed, 1 skipped.qa/fast_gate.shTier-0:195 passed✅.Invariants held: engine sole-writer untouched; additive-by-default (
abandonPendingis a new exported callback; oldclearPendingfallback kept for older bundles;ImgURL keeps the/image?scope=shape on the happy path); wire contracts frozen.Final GUI confirmation comes from the re-measure sweep — this PR is the sound fix + the deterministic JSX-harness proof. Do not merge pending that sweep.