fix(dm,viewer): cold-open delivers a real 2nd-person opening scene, not the setup brief (Addresses #357) - #388
Conversation
…ot the setup brief Root cause (nb3, bb1870d): the DM never wrote the 2nd-person opening scene as its reply text (final result text = 0 chars). It logged only a 3rd-person game-system setup brief via log_event(kind=narration) ('COLD OPEN — ARRIVAL: Rolan (tiefling wizard, PC) walks toward...'), then ended its turn. The #360 empty-reply fallback recovered that 3rd-person brief and pushed it to /chat, so a first-timer saw developer/GM notation instead of an opening scene. A GM Advisory bookkeeping panel also leaked to the player's live-play sidebar. Fixes both ends + the leak (engine stays sole writer — prompt/skill/resolver/ sanitizer only): - qa/dm_narration_fallback.py: never recover a 3rd-person setup brief / game-system notation (high-confidence shapes: a leading ALLCAPS label header or a (... PC)/(... NPC)/(level N ...) sheet tag) — blank beats notation. - scripts/play.sh (both opener branches) + scripts/play_party.sh: the cold-open opener prompt now carries the same 'FINAL reply text MUST be the 2nd-person scene, never a tool call / 3rd-person brief' contract the beat loop has. - skills/dungeon-master/reference/quest-generation.md: explicit cold-open reply-text contract. - viewer/openworlds/screen-table.jsx: sanitizeNarration strips the leaked scene-debt KIND label line ('npc introduced silent ...'); the GM Advisory panel is removed from the player live-play table screen (still on the journal). - tests: +4 resolver tests, +1 sanitizer test. Addresses #357
📝 WalkthroughWalkthroughThe PR hardens the system against returning empty or system-notation text in player chat (issue ChangesSystem-Notation Filtering and DM Prompt Hardening for Issue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
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 `@qa/dm_narration_fallback.py`:
- Line 60: The _SETUP_LABEL regex contains Unicode RIGHT SINGLE QUOTATION MARK
and EN DASH/EN DASH characters which can fail on ASCII inputs; update the
pattern in the _SETUP_LABEL constant to use ASCII equivalents (apostrophe ' and
hyphen/minus -) or include both ASCII and Unicode variants in the character
classes (e.g., replace the Unicode ’ with ' and replace —/– with - or a class
like [\-\u2013\u2014]) so matching is robust across encodings.
🪄 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: 4de45e1a-24b2-408b-bdcc-3fa484b98908
📒 Files selected for processing (7)
qa/dm_narration_fallback.pyscripts/play.shscripts/play_party.shservers/engine/tests/test_dm_narration_fallback.pyskills/dungeon-master/reference/quest-generation.mdviewer/openworlds/screen-table.jsxviewer/tests/test_sanitize_narration.py
| # A leading ALLCAPS structural LABEL followed by ':' or ' — ' — the chronicle/brief header the | ||
| # DM writes for itself ("COLD OPEN — ARRIVAL:", "SETUP:", "BRIEF —", "CHRONICLE:"). Two+ caps | ||
| # words so an in-fiction shout ("HELP!") or a single proper noun never trips it. | ||
| _SETUP_LABEL = re.compile(r"^\s*[A-Z][A-Z'’]+(?:[ \-—–][A-Z][A-Z'’]+){0,5}\s*(?::|—|–|-\s)") |
There was a problem hiding this comment.
Replace ambiguous Unicode characters with ASCII equivalents.
The regex pattern uses Unicode RIGHT SINGLE QUOTATION MARK (') and EN DASH (—, –) characters. These can cause encoding issues and matching failures when the input text uses standard ASCII apostrophes and hyphens. Use ASCII equivalents for robustness.
🔧 Proposed fix
-_SETUP_LABEL = re.compile(r"^\s*[A-Z][A-Z'']+(?:[ \-—–][A-Z][A-Z'']+){0,5}\s*(?::|—|–|-\s)")
+_SETUP_LABEL = re.compile(r"^\s*[A-Z][A-Z']+(?:[ \-][A-Z][A-Z']+){0,5}\s*(?::|-\s)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _SETUP_LABEL = re.compile(r"^\s*[A-Z][A-Z'’]+(?:[ \-—–][A-Z][A-Z'’]+){0,5}\s*(?::|—|–|-\s)") | |
| _SETUP_LABEL = re.compile(r"^\s*[A-Z][A-Z']+(?:[ \-][A-Z][A-Z']+){0,5}\s*(?::|-\s)") |
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 60-60: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
[warning] 60-60: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
[warning] 60-60: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
[warning] 60-60: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
🤖 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 `@qa/dm_narration_fallback.py` at line 60, The _SETUP_LABEL regex contains
Unicode RIGHT SINGLE QUOTATION MARK and EN DASH/EN DASH characters which can
fail on ASCII inputs; update the pattern in the _SETUP_LABEL constant to use
ASCII equivalents (apostrophe ' and hyphen/minus -) or include both ASCII and
Unicode variants in the character classes (e.g., replace the Unicode ’ with '
and replace —/– with - or a class like [\-\u2013\u2014]) so matching is robust
across encodings.
Root cause (diagnosed from the nb3 logs, build bb1870d)
The DM NEVER wrote the 2nd-person opening scene as its reply text — it is not "wrote prose that got dropped". Evidence from the nb3-b run (
/tmp/wos-verify/play-state/nb3-b/):dm.combined.jsonlfinalresultevent:subtype: success, reply text = 0 chars (empty).remembercalls, then the turn ended. It said it was about to write the opener and never did.log_event(kind="narration", text="COLD OPEN — ARRIVAL: Rolan (tiefling wizard, PC) walks toward Sorcerous Sundries…")— a 3rd-person setup brief in game-system notation.clawdnd_dm_narration_or_fallback→qa/dm_narration_fallback.py) saw the empty reply, recovered the most-recentnarrationevent = that 3rd-person brief, and pushed it to/chat.nb3-b/chat.jsonlis byte-identical to thelog_eventtext.So the player saw a 3rd-person setup brief and no opening scene (satisfaction 3/10, give-up). Two contributing causes, fixed at both ends; a GM-advisory panel also leaked.
Of the two hypotheses in the issue: it is the second — the DM did silent setup via tools and ended its turn WITHOUT writing the opening prose as its reply, so the #360 fallback grabbed the setup-log event.
What changed (engine stays the SOLE WRITER — prompt / skill / resolver / sanitizer only; all additive)
qa/dm_narration_fallback.py(resolver/fallback) — the fallback now never recovers a 3rd-person setup brief / game-system notation. New_is_system_notation()rejects only HIGH-CONFIDENCE shapes: a leading ALLCAPS label header (COLD OPEN — ARRIVAL:,SETUP:, …) or a parenthetical character-sheet tag ((tiefling wizard, PC),(NPC),(level 3 …)). Such a narration row breaks the trailing block and is treated like bookkeeping — recovering nothing (the viewer then shows the DM-stuck indicator) is strictly better than showing the player developer notation. A real 2nd-person scene ("You step into the Heapside warren…") always survives.scripts/play.sh(both opener branches) +scripts/play_party.sh— the cold-open opener prompt now carries the same contract the beat loop already had: your FINAL reply text MUST be the 2nd-person opening scene (where you are, what you see/hear/smell, who's present + a quoted line, an open moment + choice); do tool setup FIRST then CLOSE on the scene; NEVER end on a tool call or a 3rd-person setup brief. This was the asymmetry — the beat loop enforced it, the opener didn't.skills/dungeon-master/reference/quest-generation.md— added an explicit cold-open reply-text contract (the skill's per-beat rule existed in SKILL.md but the cold-open reference never stated it; the cold open is where it's most often dropped).viewer/openworlds/screen-table.jsx(sanitizer + panel leak) — (a)sanitizeNarrationnow strips a narration line led by a scene-debt KIND label (npc introduced silent …+ the raw underscore kinds), extending the [playtest][P1] DM-internal "GM Advisory" note leaks into the player-facing story panel #335/[playtest][P1-critical] DM scaffolding leaks into player Chronicle — dice tallies + plot-structure jargon ('spine hook'/'cold open') + 'beat complete' stage-directions #347 guard; (b) the GM Advisory panel (Campaign Director debts — "what the campaign owes the story" + a tool-naming nudge like "…record their first memory with remember") is removed from the player's live-play table screen — it was the actual nb3 leak the newbie flagged. It still renders on the journal/Director surface (screen-journal.jsx).Touched: DM skill (quest-generation.md), cold-open prompt (play.sh + play_party.sh), resolver/fallback (dm_narration_fallback.py), sanitizer + player view (screen-table.jsx).
Tests
servers/engine/tests/test_dm_narration_fallback.py: +4 (the exact nb3 brief is NOT recovered; a brief-then-real-scene recovers only the scene; real 2nd-person prose with an innocent parenthetical survives; the existing trailing-block recovery still passes).viewer/tests/test_sanitize_narration.py: +1 (the verbatim panel leak + raw debt-kind labels are stripped; "silent"/"consequence"/"npc" as ordinary fiction survive).bash -n(both scripts),py_compile,scripts/license_check.py, Node+Babel transpile of the.jsx, and standalone logic runs of the new helpers all pass. Full pytest/vitest run in CI.Do NOT close on merge
Do NOT close #357 on merge — verify on the next newbie playtest: an opening 2nd-person scene reaches
/chat(not the setup brief), no GM-advisory leak in the player view, and satisfaction ≥7.Summary by CodeRabbit
Bug Fixes
Tests