Skip to content

fix(dm,viewer): cold-open delivers a real 2nd-person opening scene, not the setup brief (Addresses #357) - #388

Merged
100yenadmin merged 1 commit into
mainfrom
fix/357-coldopen-scene
May 30, 2026
Merged

fix(dm,viewer): cold-open delivers a real 2nd-person opening scene, not the setup brief (Addresses #357)#388
100yenadmin merged 1 commit into
mainfrom
fix/357-coldopen-scene

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented May 30, 2026

Copy link
Copy Markdown
Member

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.jsonl final result event: subtype: success, reply text = 0 chars (empty).
  • The DM's last assistant text block was a meta-line — "Logging Vanos and the Sable Court to memory while writing the opening narration." — then two remember calls, then the turn ended. It said it was about to write the opener and never did.
  • The only player-facing prose the DM logged to the engine was a single 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.
  • The fix(play): DM resolver always writes non-empty player-facing narration per beat (Addresses #357) #360 empty-reply fallback (clawdnd_dm_narration_or_fallbackqa/dm_narration_fallback.py) saw the empty reply, recovered the most-recent narration event = that 3rd-person brief, and pushed it to /chat. nb3-b/chat.jsonl is byte-identical to the log_event text.

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)

  1. 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.
  2. 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.
  3. 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).
  4. viewer/openworlds/screen-table.jsx (sanitizer + panel leak) — (a) sanitizeNarration now 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).
  • Local pre-push confidence: 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

    • Internal DM system notation and bookkeeping no longer leak into player-facing narration, chronicle displays, or sidebars.
    • Opening scenes now consistently deliver proper player-facing prose instead of internal notation.
  • Tests

    • Added verification tests for notation filtering and scene recovery.

Review Change Stack

…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
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR hardens the system against returning empty or system-notation text in player chat (issue #357) through coordinated filtering, prompt instruction, and viewer-side sanitization. System-notation rows are now regex-filtered before recovery, DM prompts enforce 2nd-person scene prose output on first turn, and GM-internal advisory content is removed from the live-play panel.

Changes

System-Notation Filtering and DM Prompt Hardening for Issue #357

Layer / File(s) Summary
System-notation detection and fallback filtering
qa/dm_narration_fallback.py, servers/engine/tests/test_dm_narration_fallback.py
Adds _is_system_notation() function using regex to detect ALLCAPS structural headers and character-sheet tokens (PC/NPC/level) in narration rows. Recovery script resets trailing prose and skips matching rows. Three test cases verify setup-brief exclusion, block boundary behavior with real scenes following, and false-positive avoidance on innocent parentheticals.
DM first-turn output instructions
scripts/play.sh, scripts/play_party.sh, skills/dungeon-master/reference/quest-generation.md
Adds "CRITICAL" directives to both initial-DM prompt branches (authored-hero and default paths) and party-play mode, requiring the DM's first-turn final output to be complete 2nd-person opening-scene prose with location/sensory details, companions, quoted dialogue, and a choice—with setup/tool work first, then scene-closing prose, no tool-call endings, and no 3rd-person notation. Documentation clarifies the output contract.
Viewer-side internal content removal
viewer/openworlds/screen-table.jsx, viewer/tests/test_sanitize_narration.py
Extends sanitization regex to recognize scene-debt "kind" labels (e.g., npc_introduced_silent) as GM bookkeeping. Removes advisory React state and Campaign Director fetch logic from ScreenTable. Removes "GM Advisory" panel from live-play sidebar, keeping Quests/Quick Stash/Encounter panels. Test verifies sanitizer strips GM-advisory panel/kind variants while preserving ordinary-language uses of the same keywords.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • electricsheephq/WorldOS#360: Introduces the dm_narration_fallback fallback mechanism itself; this PR adds regex-based system-notation filtering to refine its recovery behavior for #357.
  • electricsheephq/WorldOS#338: Modifies sanitizeNarration and log rendering to remove GM-internal directives; overlaps with this PR's viewer-side sanitization and GM-advisory panel removal changes.

Poem

🐰 No more setup briefs shown to eager players—
The DM now speaks in scenes, not scratchpad layers.
Filters catch the notation, prompts demand the prose,
And viewer hides the GM book so nobody knows!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing the cold-open to deliver a real 2nd-person opening scene instead of a setup brief, directly addressing issue #357.
Description check ✅ Passed The description thoroughly documents the root cause, changes across all affected files, testing strategy, and verification approach, aligning well with the template requirements.
Linked Issues check ✅ Passed All code changes directly address #357 requirements: fallback rejects 3rd-person notation [qa/dm_narration_fallback.py], prompts enforce 2nd-person final output [scripts/play.sh, play_party.sh], sanitizer strips leaked GM-advisory content [viewer/openworlds/screen-table.jsx], and tests verify the fixes.
Out of Scope Changes check ✅ Passed All file modifications are directly scoped to fixing the cold-open narration issue: resolver filtering, prompt updates, documentation, sanitizer improvements, and related tests; no unrelated refactoring or feature creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between bb1870d and 67e958c.

📒 Files selected for processing (7)
  • qa/dm_narration_fallback.py
  • scripts/play.sh
  • scripts/play_party.sh
  • servers/engine/tests/test_dm_narration_fallback.py
  • skills/dungeon-master/reference/quest-generation.md
  • viewer/openworlds/screen-table.jsx
  • viewer/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)")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
_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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[playtest][CRITICAL] DM turn returns empty player-facing narration on engine-heavy beats

1 participant