fix(viewer): cold-open wait affordance + backward-in-time surface guard (input-lock + wall-of-text rollback) - #718
Merged
Merged
Conversation
added 2 commits
June 9, 2026 17:55
…t spinner (input-lock give-up) RRI 2026-06-09 (vm2-newbie 'input locked 9+ min, no feedback' + the veteran give-up): through the minutes-long cold-open the action bar is locked (live but is_live_view desynced) and the chronicle is empty, so the screen reads as a frozen crash. There was no affordance for the cold-open wait — only the player-move pending/stuck beats render a spinner. Fix: a pure computeColdOpenAwaiting selector (live session + nothing landed yet: empty chronicle, empty party, no pending/stuck beat) gates rendering the EXISTING first-beat DmNarratingBeat (its 'the first beat can take a few minutes — hang tight' copy). The empty 'No moves yet' line is suppressed while it shows. Pure read-model; clears the instant a beat or the party arrives, or a player move arms a pending beat. Mirrors the firstBeat=true/streaming=false path already proven for the post-move first beat. Tests: 7 ColdOpenAwaitingTests (the real live+is_live_view=false frame, the negatives: seated party, existing log, pending/stuck precedence, surface-not-ready). recovery-timing 21 passed; full viewer suite green.
…tate rollback) RRI 2026-06-09 (vm2-adversarial 'mid-turn navigation reverts session state + loses the DM beat'): the WORST reliability bug. /session-surface header (day/HP/location) is one snapshot read; the chronicle is client-assembled from /chat+/events. During a long DM beat a transient re-fetch projected an OLDER snapshot, so the header regressed (Day 3->1, HP/location reverted) while the live chronicle held — a header-vs-chronicle desync that reads as a save corruption / party wipe. Fix: build_session_surface now emits the snapshot's monotonic 'updated_at'; loadSurface applies an incoming surface via shouldApplySurface(prev,next) — a strictly-OLDER snapshot of the SAME campaign is rejected (keep the newer one already shown), so the header can never go backward in time. A different campaign (real switch), a newer/equal snapshot, a first surface, or an older save with no clock all apply unchanged. Pure read-model; app-status (live/can_act) still updates each poll. Tests: 6 SurfaceFreshnessTests (older-rejected / newer / equal / cross-campaign / first / no-clock) + build_session_surface updated_at assertion. Full viewer suite green.
📝 WalkthroughWalkthroughThis PR adds two protective mechanisms for cold-open sessions: a monotonic surface freshness guard preventing snapshot regressions, and an awaiting narrative beat affordance when sessions are live but no beats or party have arrived yet. Server provides ChangesCold-Open Awaiting and Surface Freshness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@viewer/tests/test_recovery_timing.py`:
- Around line 419-421: The test method test_isliveview_only_also_fires is
asserting the cold-open affordance but passes live=False, which contradicts the
cold-open contract; update the call to self._awaiting in that test to use
live=True (i.e., self._awaiting(live=True, isLiveView=True)) so the test
explicitly targets a live session with no landed content and aligns the
assertion with the intended contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed374b5c-b79e-4b53-b611-1ad951fccae6
📒 Files selected for processing (4)
viewer/openworlds/screen-table.jsxviewer/server.pyviewer/tests/test_live_view_recovery.pyviewer/tests/test_recovery_timing.py
Comment on lines
+419
to
+421
| def test_isliveview_only_also_fires(self): | ||
| self.assertTrue(self._awaiting(live=False, isLiveView=True)) | ||
|
|
There was a problem hiding this comment.
Cold-open test broadens beyond the stated live-session contract.
Line 420 currently expects the affordance to fire when live=False. That conflicts with the cold-open contract (“live session with nothing landed yet”) and can encode misleading behavior for genuinely non-live sessions.
Suggested fix
- def test_isliveview_only_also_fires(self):
- self.assertTrue(self._awaiting(live=False, isLiveView=True))
+ def test_isliveview_only_does_not_fire(self):
+ self.assertFalse(
+ self._awaiting(live=False, isLiveView=True),
+ "cold-open affordance should require a live session",
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/tests/test_recovery_timing.py` around lines 419 - 421, The test method
test_isliveview_only_also_fires is asserting the cold-open affordance but passes
live=False, which contradicts the cold-open contract; update the call to
self._awaiting in that test to use live=True (i.e., self._awaiting(live=True,
isLiveView=True)) so the test explicitly targets a live session with no landed
content and aligns the assertion with the intended contract.
100yenadmin
added a commit
that referenced
this pull request
Jun 9, 2026
…second one (#719) (#726) The DEFAULT cold-open prompt (scripts/play.sh) instructs start_world + 'if it returns existing_campaigns, start fresh'. When attempt-1 times out (rc=124) AFTER already minting+seeding a campaign, play.sh's one-shot retry re-runs that same prompt -> the DM mints a SECOND, party-less campaign. The viewer auto-follows the newer empty orphan -> party-wipe + a frozen/input-locked screen. This is the adversarially-verified engine root shared by two RRI criticals (hero-bind party-wipe + input-lock); PR #717/#718 mitigated the read-side + added the spinner, but the orphan was still CREATED. Fix (additive, invariant-safe; engine stays sole writer): new shared helper clawdnd_coldopen_retry_msg (qa/lib_beat_driver.sh) returns a RESUME directive (get_state on the existing id, DO NOT start_world, seat a canon PC only if the party is empty) for a DEFAULT cold-open RETRY (first=1, no authored hero) where attempt-1 left a live campaign; otherwise it echoes the base message UNCHANGED (continuing beat / authored hero / fresh first attempt with nothing to resume). Wired into dm_turn's retry in play.sh. Byte-identical on every path except the bug; read-only (asks the engine for the live save) and only on the slow retry path. 3.2-clean (printf, no heredoc-in-$()). The engine seam already exists + is tested (start_world(resume=) -> test_start_world_resume_continues_instead_of_orphaning, test_content.py). Tests: 5 new deterministic shell-helper tests in test_dm_session_remint.py (resume on the bug path; unchanged for no-prior-campaign / continuing-beat / authored-hero; play.sh wiring anti-drift). 15/15 in-file + fast_gate 188 green. Co-authored-by: Eva <arncalso@gmail.com>
100yenadmin
added a commit
that referenced
this pull request
Jun 9, 2026
…at is never perceived as dropped/hung (closes #623) (#743) #623 ("DM beats silently DROP / HANG >10min with no recovery") was a PERCEIVED-latency defect, NOT real DM unreliability. Forensics on the filing sweep_v8 run (Eva, on its own dm.*.jsonl): all four beats ran cleanly at ttft 2-5s / 85-157s wall with ZERO timeouts, retries, empty-fallbacks or provider_errors. The actual defect: /events stayed BLANK for the whole beat -> the OpenWorlds viewer's notePendingProgress streaming-flip never fired -> the player stared at a static "weaving the next beat" spinner and called a healthy 157s beat a "drop" and the cumulative wait a "hang". Root cause: the SOLO scripts/play.sh path (the one that filed #623) had NEITHER the live-progress rule NOR the wrapper-authored heartbeat that scripts/play_party.sh (#623) and scripts/play_codex_dm.sh already carry. So its DM emitted nothing player-facing to /events until the full beat completed. Two-layer fix, both PERCEIVED-latency — neither touches wall-clock (per worldos-latency- forensics, speed levers are REFUTED); the existing bounded timeout + one-retry + #357 empty-narration fallback are unchanged: 1. WRAPPER HEARTBEAT (model-INDEPENDENT, the guarantee): clawdnd_emit_progress_heartbeat writes a short wrapper-authored 2nd-person `narration` row to the engine session log via the shared log_engine_narration BEFORE the DM `claude -p` starts. It lands in /events within ~1s, so the viewer flips its spinner to "the scene is arriving above" no matter how long the model thinks -- and crucially even when the model SKIPS the cooperative early log_event (Eva measured exactly that: rule present, streaming refs = 0). Same proven pattern the Codex DM wrapper uses (OPENING_PROGRESS_TEXT / MOVE_PROGRESS_TEXTS), factored to the shared lib. play.sh emits it on the per-beat loop and the authored-hero cold open (the default cold open mints its campaign inside the turn, so it keeps the #718 cold-open spinner + the live-progress rule). Best-effort + non-fatal (blank id or engine error -> return 0); engine stays the sole writer. 2. CLAWDND_LIVE_PROGRESS_RULE (model-COOPERATIVE): factored verbatim into the shared lib (anti-drift) and applied to play.sh's DM beat prompt -- parity with the party + codex paths. Its absence in the solo path was the bug. Additive; bash 3.2-clean (the only bashism, [[ =~ ]], is byte-identical to the codex path's proven usage). TDD: 6 new tests in test_dm_session_remint.py exercise the real bash helpers under /bin/bash and a real engine campaign -- the heartbeat logs a real narration row, rotates per beat, no-ops on a blank id, and play.sh wires both layers. What remains for the GUI sweep: confirm on the BUILT dist/WorldOS.app that the /events heartbeat row visibly flips the spinner within ~1s of a move (the headless-proxy path that filed #623 could not replicate streaming; this fix makes the signal model-independent so it should hold on the .app too, but the .app render is the gate surface). Co-authored-by: Eva <arncalso@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two read-model reliability fixes from the Phase-2 RRI sweep (adversarially root-caused). Both are pure read-model (engine stays sole writer; viewer stays read-only) and fully unit-tested against the real transpiled
screen-table.jsx.1 — Cold-open wait affordance (input-lock give-up)
RRI 2026-06-09: vm2-newbie filed "input locked 9+ min, no feedback" and the veteran gave up — through the minutes-long cold-open the action bar is locked (live but
is_live_viewdesynced) and the chronicle is empty, so the screen reads as a frozen crash. Only player-move pending/stuck beats rendered a spinner; the cold-open wait had no affordance.Fix: a pure
computeColdOpenAwaitingselector (live session + nothing landed: empty chronicle, empty party, no pending/stuck beat) gates rendering the existing first-beatDmNarratingBeat("the first beat can take a few minutes — hang tight"). The empty "No moves yet" line is suppressed while it shows. Clears the instant a beat or the party arrives, or a player move arms a pending beat. The real frame islive:true, is_live_view:false— exactly when the bar locks.2 — Backward-in-time surface guard (wall-of-text state rollback — the worst critical bug)
RRI 2026-06-09: vm2-adversarial filed "mid-turn navigation reverts session state + loses the DM beat" (Day 3→1, HP/location reverted, narration dropped). Re-diagnosed: the
/session-surfaceheader is one snapshot read while the chronicle is client-assembled from/chat+/events; during a long beat a transient re-fetch projected an older snapshot, regressing the header backward in time while the live chronicle held — a header-vs-chronicle desync that reads as save corruption.Fix:
build_session_surfacenow emits the snapshot's monotonicupdated_at;loadSurfaceapplies an incoming surface viashouldApplySurface(prev, next)— a strictly-older snapshot of the same campaign is rejected (keep the newer one already shown). A different campaign (real switch), a newer/equal snapshot, a first surface, or an older save with no clock all apply unchanged.app-status(live/can_act) still updates each poll.Tests
ColdOpenAwaitingTests(the reallive+is_live_view=falseframe + negatives: seated party, existing log, pending/stuck precedence, surface-not-ready).SurfaceFreshnessTests(older-rejected / newer / equal / cross-campaign / first / no-clock) +build_session_surfaceupdated_atassertion.Part of the Phase-2 reliability sweep (the sat/critical/give-up gates). Companion read-side party-wipe fix: #717. The play.sh cold-open retry double-mint (the engine-root shared by the party-wipe + input-lock) and the dup-narration
record_dm_replyland as follow-up issues.Summary by CodeRabbit
Release Notes
New Features
Tests