Polish Chronicle rows, Forge art, and table readiness - #548
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughParses player routing tags into replayable beats, deduplicates replayed player rows against optimistic echoes, adds app-status play gating and a blocks-play banner, expands camp deep-linking/campMode, updates merchant surface and journal filtering, adjusts item-art alias resolution, and updates related tests and narration markup. ChangesPlayer Routing, Chronicle Dedup, and UI Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
100yenadmin
force-pushed
the
codex/worldos-chronicle-row-polish
branch
5 times, most recently
from
June 2, 2026 00:46
445b7ca to
b73ec38
Compare
100yenadmin
force-pushed
the
codex/worldos-chronicle-row-polish
branch
from
June 2, 2026 00:50
b73ec38 to
e481055
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
viewer/tests/test_openworlds_static.py (2)
699-700: ⚡ Quick winMake the inline-Chronicle regression check whitespace-agnostic.
Line 700 matches one exact newline/indent layout, so the test can miss the regression if
<span>Chronicle</span>comes back in the narration row with different formatting. Prefer a regex scoped todata-worldos-testid="chronicle-narration"instead of a fixed multiline literal.Suggested assertion hardening
self.assertIn("sanitizeNarration(entry.text)", source) self.assertIn('data-worldos-testid="chronicle-narration"', source) - self.assertNotIn('>Chronicle</span>\n {text}', source) + self.assertNotRegex( + source, + r'data-worldos-testid="chronicle-narration"[\s\S]*?>Chronicle</span>', + )🤖 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_openworlds_static.py` around lines 699 - 700, The current assertion uses a literal multiline string ('>Chronicle</span>\n {text}') which is whitespace-sensitive; update the test around data-worldos-testid="chronicle-narration" to use a regex match instead (e.g., use re.search with re.DOTALL or collapse whitespace) to assert that a <span>Chronicle</span> is not present in the narration row; modify the assertNotIn line to perform a regex search on the source variable scoped to the data-worldos-testid="chronicle-narration" fragment (and add an import for re if missing).
189-202: ⚡ Quick winAssert the merchant error-feedback branch too.
Line 201 only proves the non-OK fetch throws. If the
.catch(...toast(...))path regresses, this test still passes while the user loses the “Move not sent” feedback promised byviewer/openworlds/screen-merchant.jsx:329-365. Add an assertion for the catch/toast strings here so the static contract covers the full failure UX.Suggested assertion additions
self.assertIn("if (surfaceLoading) return;", source) self.assertIn("disabled={cart.length === 0 || surfaceLoading", source) self.assertIn("Checking the counter", source) self.assertIn("if (!response.ok) throw new Error", source) + self.assertIn(".catch((e) => toast({ kind: \"danger\"", source) + self.assertIn('title: "Move not sent"', source)🤖 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_openworlds_static.py` around lines 189 - 202, The test test_merchant_waits_for_live_action_lane_before_purchase currently only asserts the non-OK fetch throws; add assertions to cover the error-feedback branch by checking the rendered source contains the fetch .catch handler and the toast call plus the UX string used for failures (e.g., assertIn('.catch', source), assertIn('toast(', source) and assertIn('Move not sent', source)) so the static test ensures the catch/toast "Move not sent" feedback from the merchant screen is present.viewer/tests/test_live_narration_stream.py (1)
740-756: ⚡ Quick winAdd a regression case for
[say] continuevs[do] continue.This test validates routed speech/action, but not the quick-label overlap case. Adding
[say] continueasdialogwould guard against accidental speech→action coercion.🤖 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_live_narration_stream.py` around lines 740 - 756, Update the test_routed_player_replay_preserves_action_vs_speech_after_reload test to include a regression case for the quick-label overlap by adding a player item with text '[say] continue' (distinct timestamp) to the h.enqueue payload so the run exercises both '[do] continue' and '[say] continue'; then assert that h.chronicle() contains two separate entries (one action row for the '[do]' continue and one dialog row for the '[say]' continue) preserving routing tags—use the existing _run call, h.enqueue invocation, and the returned out["chronicle"] comparison to add this extra input and expected chronicle entry.
🤖 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/openworlds/app.jsx`:
- Around line 801-814: fromHash/onHash currently only call setCampMode when
route.campMode is a boolean, so moving away from `#camp/`#rest leaves the old
campMode set; change both the initial handling and the onHash handler to always
call setCampMode(route.campMode) (i.e. remove the typeof boolean guard) so that
undefined from fromHash clears the previous campMode state; update references in
the initial block and the onHash function that use fromHash, setCampMode, and
setScreen accordingly.
In `@viewer/openworlds/screen-table.jsx`:
- Around line 283-290: The projectPlayerReplay projection is converting "You"
dialog entries into actions for quick-label words and needs to skip that
conversion when the replay explicitly routed the text as speech; update
projectPlayerReplay to detect explicit speech-routing indicators (e.g.
entry.route === 'say', entry.mode === 'say', or entry.routing?.type === 'say' —
check whichever routing field your model uses) and return the original entry if
any such indicator is present before applying
playerEchoKeys/QUICK_ACTION_REPLAY_LABELS; apply the same guarding logic to the
duplicate projection at the other location mentioned (around the second
occurrence at line ~315).
---
Nitpick comments:
In `@viewer/tests/test_live_narration_stream.py`:
- Around line 740-756: Update the
test_routed_player_replay_preserves_action_vs_speech_after_reload test to
include a regression case for the quick-label overlap by adding a player item
with text '[say] continue' (distinct timestamp) to the h.enqueue payload so the
run exercises both '[do] continue' and '[say] continue'; then assert that
h.chronicle() contains two separate entries (one action row for the '[do]'
continue and one dialog row for the '[say]' continue) preserving routing
tags—use the existing _run call, h.enqueue invocation, and the returned
out["chronicle"] comparison to add this extra input and expected chronicle
entry.
In `@viewer/tests/test_openworlds_static.py`:
- Around line 699-700: The current assertion uses a literal multiline string
('>Chronicle</span>\n {text}') which is whitespace-sensitive; update
the test around data-worldos-testid="chronicle-narration" to use a regex match
instead (e.g., use re.search with re.DOTALL or collapse whitespace) to assert
that a <span>Chronicle</span> is not present in the narration row; modify the
assertNotIn line to perform a regex search on the source variable scoped to the
data-worldos-testid="chronicle-narration" fragment (and add an import for re if
missing).
- Around line 189-202: The test
test_merchant_waits_for_live_action_lane_before_purchase currently only asserts
the non-OK fetch throws; add assertions to cover the error-feedback branch by
checking the rendered source contains the fetch .catch handler and the toast
call plus the UX string used for failures (e.g., assertIn('.catch', source),
assertIn('toast(', source) and assertIn('Move not sent', source)) so the static
test ensures the catch/toast "Move not sent" feedback from the merchant screen
is present.
🪄 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: 175c75d0-81c1-44d9-b0f2-dc52240ece49
📒 Files selected for processing (6)
viewer/openworlds/app.jsxviewer/openworlds/screen-journal.jsxviewer/openworlds/screen-merchant.jsxviewer/openworlds/screen-table.jsxviewer/tests/test_live_narration_stream.pyviewer/tests/test_openworlds_static.py
100yenadmin
changed the title
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.
Summary
/chatplayer replay rows against matching optimistic player echoes in the OpenWorlds Chronicle.Chroniclelabel from each DM narration row; the section title androle=loglabel still name the region./moveremains the intent lane.Product Evidence
/Volumes/LEXAR/Codex/worldos-product-slices/browser-scout-5012d02-20260602T064900/./Volumes/LEXAR/Codex/worldos-product-slices/chronicle-polish-5012d02-20260602T065420/.ready_for_play=true,can_act=true, six enabled actions, private art present, zero console errors,inlineChronicleLabels=0, and one clean player echo plus one DM reply after Continue.Test Plan
python3 -m pytest viewer/tests/test_live_narration_stream.py viewer/tests/test_openworlds_static.py -qgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Changes
Tests