fix(qa+wrappers): SYN-01 dead-beat masking — error-class result classification, visible failure beats, single chatlog impl (Refs #757 #745) - #841
Merged
Conversation
…results, visible failure beats, single chatlog impl (Refs #757 #745) Root cause (audit SYN-01 = F12-7 + F12-14 + F13-5, docs/audits/ENGINE-AUDIT-2026-06-11.md): a 401/auth-failed `claude -p` result carries NON-empty error text (subtype:"success", is_error:true, api_error_status:401), bypassing the empty-only retry (run_duo turn_retry) AND the #357 narration-fallback gate — so the error string was chatlogged AS DM PROSE; a fully-dead beat either recycled the previous beat's prose into a client-hidden row or wrote an unflagged EMPTY dm row; and 3 QA runners' local chatlog() overrides (run_duo:135, ui_playtest:138, run_party:169) shadowed the shared lib, killing the fallback_recovered stamp (which also had zero consumers). Fix (qa/lib_beat_driver.sh, shared by all 5 DM wrappers): - clawdnd_dm_final_text — the ONE extraction front door: notes the final attempt's stream-json in $STATE_DIR/.dm_last_result and echoes NOTHING on an error-class result (is_error / api_error_status), surfacing the real failure + 401/403 re-auth hint via the existing clawdnd_report_attempt_failure pattern. Empty echo makes the empty-only retries fire on error results too. - clawdnd_resolve_dm_reply — parses the FINAL result event FIRST: error-class => beat FAILED (never chat the error text, never fallback-recycle). A fallback recovery is GENUINE only if the DM logged NEW prose past the pre-beat log-tail mark (clawdnd_dm_prebeat_mark + qa/dm_beat_mark.py — same prose filters as the #357 fallback; fail-open without a mark); recycled-only prose => beat FAILED. - record_dm_reply blank guard + clawdnd_chatlog_dm_failed — a failed/blank beat records ONE wrapper-authored VISIBLE failure row stamped {"beat_failed":true} (chat-only by design: engine-logging it would dedup-hide repeats and pollute recap/FTS/lean-tail/fallback memory). - Deleted the 3 runner chatlog overrides — the lib chatlog (drop-in superset) is the single implementation, so the fallback_recovered/beat_failed stamps land in every runner. - qa/assert_behavioral.py dm_beat_honesty — counts + reports beats_failed / fallback_recovered per run (WARN only; the gate-discount policy stays #757's call). #828's worldos_timeout / retry-escalation / re-mint behavior is preserved exactly (only the final jq extraction is swapped for the front door; the pre-beat mark is taken once per beat, before attempt 1). Tests: servers/engine/tests/test_dead_beat_classification.py (25 new, red-first) — 401 fixture never reaches chat; recycled recovery fails the beat; genuine #357 win preserved; heartbeat rows never count as prose; blank record => exactly one visible, never-hidden failure row; override deletion + wiring shape locks; assert_behavioral counts without gating; bash -n on every touched script. Full engine suite 1830 passed; qa/fast_gate.sh PASS. Finding-ids: SYN-01 (F12-7, F12-14, F13-5). Refs #757, Refs #745.
📝 WalkthroughWalkthroughThis PR implements a "dead-beat failure honesty" system for DM turns that detects error-class results and empty narration, validates recovered prose against pre-beat log marks to reject recycled content, records visible failure rows in chat logs, and provides QA metrics to measure compliance without gating test runs. ChangesDead-Beat Failure Honesty System
Sequence DiagramsequenceDiagram
participant Beat as DM Beat Flow
participant Mark as clawdnd_dm_prebeat_mark
participant Final as clawdnd_dm_final_text
participant Resolve as clawdnd_resolve_dm_reply
participant Fallback as Narration Fallback
participant Check as clawdnd_dm_logged_new_prose
participant Record as clawdnd_chatlog_dm_failed
participant Chat as Chat Log
Beat->>Mark: log current tail
Mark->>Beat: mark written
Beat->>Final: invoke LLM, get result
Final->>Final: classify error-class?
Final-->>Resolve: empty on error
Resolve->>Resolve: empty reply from LLM?
Resolve->>Fallback: recover from engine log
Fallback-->>Resolve: recovered prose
Resolve->>Check: is prose NEW after mark?
alt Recycled (no NEW)
Check-->>Resolve: recycled
Resolve->>Record: reject, mark failed
else Genuine (NEW prose after mark)
Check-->>Resolve: genuine
Resolve-->>Resolve: keep recovered prose
end
Resolve->>Chat: write beat (with stamps)
Chat-->>Chat: logged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![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
🤖 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_beat_mark.py`:
- Around line 57-74: _snapshot_path currently updates best by encountering a
strictly larger size, which makes ties depend on os.listdir() order; change it
to be deterministic like the shell helper by first computing the maximum
snapshot size (>1 byte) across all campaigns and then selecting the
lexicographically smallest campaign name among those whose snapshot.json has
that max size. In practice inside _snapshot_path (use symbols root, names, size,
best_size, p): iterate names to build a mapping of name->size for existing
snapshot.json files, compute max_size, return "" if max_size <= 1, otherwise
pick the smallest name from [name for name,size in mapping.items() if size ==
max_size] and return os.path.join(root, chosen_name, "snapshot.json").
In `@qa/lib_beat_driver.sh`:
- Around line 399-406: The final result extractor clawdnd_dm_final_text
currently uses jq 'map(select(.type=="result"))[-1].result // ""' which
preserves whitespace-only payloads; change the jq expression in
clawdnd_dm_final_text to trim surrounding whitespace and collapse
whitespace-only strings to "" (e.g., append a jq gsub trim like |
gsub("^\\s+|\\s+$";"") and then coalesce to ""), so any result that's only
whitespace becomes an empty string and triggers the retry/failure behavior.
🪄 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: 84469811-34d4-4f26-8ac2-c685a74ad85f
📒 Files selected for processing (9)
qa/assert_behavioral.pyqa/dm_beat_mark.pyqa/lib_beat_driver.shqa/run_duo.shqa/run_party.shqa/ui_playtest.shscripts/play.shscripts/play_party.shservers/engine/tests/test_dead_beat_classification.py
Comment on lines
+57
to
+74
| def _snapshot_path(state_dir): | ||
| """The LARGEST non-empty snapshot under <state_dir>/campaigns — mirrors the shell-side | ||
| clawdnd_snapshot_path (find -size +1c | ls -S | head -1).""" | ||
| best, best_size = "", 1 # >1 byte, matching find's -size +1c | ||
| root = os.path.join(state_dir, "campaigns") | ||
| try: | ||
| names = os.listdir(root) | ||
| except OSError: | ||
| return "" | ||
| for name in names: | ||
| p = os.path.join(root, name, "snapshot.json") | ||
| try: | ||
| size = os.path.getsize(p) | ||
| except OSError: | ||
| continue | ||
| if size > best_size: | ||
| best, best_size = p, size | ||
| return best |
There was a problem hiding this comment.
Make _snapshot_path() deterministic with the shell helper.
This is supposed to mirror clawdnd_snapshot_path, but on equal-size snapshot.json files it currently depends on os.listdir() order. The shell side uses a sorted ls -S pick, so mark/check can inspect a different campaign than clawdnd_dm_narration_or_fallback on tie cases and misclassify genuine recovery as recycled (or vice versa).
Proposed fix
def _snapshot_path(state_dir):
"""The LARGEST non-empty snapshot under <state_dir>/campaigns — mirrors the shell-side
clawdnd_snapshot_path (find -size +1c | ls -S | head -1)."""
- best, best_size = "", 1 # >1 byte, matching find's -size +1c
+ candidates: list[tuple[int, str]] = []
root = os.path.join(state_dir, "campaigns")
try:
names = os.listdir(root)
except OSError:
return ""
@@
try:
size = os.path.getsize(p)
except OSError:
continue
- if size > best_size:
- best, best_size = p, size
- return best
+ if size > 1:
+ candidates.append((size, p))
+ if not candidates:
+ return ""
+ return sorted(candidates, key=lambda item: (-item[0], item[1]))[0][1]🤖 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_beat_mark.py` around lines 57 - 74, _snapshot_path currently updates
best by encountering a strictly larger size, which makes ties depend on
os.listdir() order; change it to be deterministic like the shell helper by first
computing the maximum snapshot size (>1 byte) across all campaigns and then
selecting the lexicographically smallest campaign name among those whose
snapshot.json has that max size. In practice inside _snapshot_path (use symbols
root, names, size, best_size, p): iterate names to build a mapping of name->size
for existing snapshot.json files, compute max_size, return "" if max_size <= 1,
otherwise pick the smallest name from [name for name,size in mapping.items() if
size == max_size] and return os.path.join(root, chosen_name, "snapshot.json").
Comment on lines
+399
to
+406
| clawdnd_dm_final_text() { | ||
| local out="$1" state_dir="$2" rc="${3:-0}" | ||
| printf '%s\n' "$out" > "$state_dir/.dm_last_result" 2>/dev/null || true | ||
| if clawdnd_dm_result_is_error "$out"; then | ||
| clawdnd_report_attempt_failure "$out" "$rc" | ||
| return 0 | ||
| fi | ||
| jq -rs 'map(select(.type=="result"))[-1].result // ""' "$out" 2>/dev/null |
There was a problem hiding this comment.
Collapse whitespace-only final results to "".
jq ... .result // "" preserves payloads like " " or "\n". Every wrapper still treats only zero-length output as a dead beat, so a whitespace-only final result skips the retry path and can still land as an effectively invisible DM row instead of a visible failure beat.
Proposed fix
clawdnd_dm_final_text() {
local out="$1" state_dir="$2" rc="${3:-0}"
+ local text=""
printf '%s\n' "$out" > "$state_dir/.dm_last_result" 2>/dev/null || true
if clawdnd_dm_result_is_error "$out"; then
clawdnd_report_attempt_failure "$out" "$rc"
return 0
fi
- jq -rs 'map(select(.type=="result"))[-1].result // ""' "$out" 2>/dev/null
+ text="$(jq -rs 'map(select(.type=="result"))[-1].result // ""' "$out" 2>/dev/null)"
+ [ -n "${text//[[:space:]]/}" ] && printf '%s' "$text"
}📝 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
| clawdnd_dm_final_text() { | |
| local out="$1" state_dir="$2" rc="${3:-0}" | |
| printf '%s\n' "$out" > "$state_dir/.dm_last_result" 2>/dev/null || true | |
| if clawdnd_dm_result_is_error "$out"; then | |
| clawdnd_report_attempt_failure "$out" "$rc" | |
| return 0 | |
| fi | |
| jq -rs 'map(select(.type=="result"))[-1].result // ""' "$out" 2>/dev/null | |
| clawdnd_dm_final_text() { | |
| local out="$1" state_dir="$2" rc="${3:-0}" | |
| local text="" | |
| printf '%s\n' "$out" > "$state_dir/.dm_last_result" 2>/dev/null || true | |
| if clawdnd_dm_result_is_error "$out"; then | |
| clawdnd_report_attempt_failure "$out" "$rc" | |
| return 0 | |
| fi | |
| text="$(jq -rs 'map(select(.type=="result"))[-1].result // ""' "$out" 2>/dev/null)" | |
| [ -n "${text//[[:space:]]/}" ] && printf '%s' "$text" | |
| } |
🤖 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/lib_beat_driver.sh` around lines 399 - 406, The final result extractor
clawdnd_dm_final_text currently uses jq 'map(select(.type=="result"))[-1].result
// ""' which preserves whitespace-only payloads; change the jq expression in
clawdnd_dm_final_text to trim surrounding whitespace and collapse
whitespace-only strings to "" (e.g., append a jq gsub trim like |
gsub("^\\s+|\\s+$";"") and then coalesce to ""), so any result that's only
whitespace becomes an empty string and triggers the retry/failure behavior.
100yenadmin
added a commit
that referenced
this pull request
Jun 14, 2026
…ta-aborted); fix sweep header parallel→sequential (#846) - OPERATING-GOAL state block → current truth: main ~347b6c2, the audit fix-wave + the sibling full-engine-audit fork's fixes (mech #832/#834, image_render #829, dead-beat #841, seat #833, spells #830, economy #831, beat-reliability #828) all landed; rc3 QUOTA-ABORTED + invalid (#845, newbie-canary clean); NEXT ACTION = re-measure with the hardened sequential sweep at current main, blocked on claude quota. - sweep_v2.sh header: the top comment + start-line still said PARALLEL after #844 made the persona batch sequential — corrected so the file is internally consistent. Co-authored-by: Eva <arncalso@gmail.com>
100yenadmin
added a commit
that referenced
this pull request
Jun 14, 2026
…meout/stale-status/budget, claude provider_status, run_duo deadline, party actor timeout, play.sh idle+lock, sonnet cold-open margin (Refs #811) (#863) Brings the codex/openclaw/claude lanes up to the play.sh protections per the audited P2 cluster. All changes additive + bash 3.2-clean; preserves #828's worldos_timeout shim + #841's failure-classification (both confirmed intact). F12-2 sonnet cold-open deadline 400 had thin (band-top) margin → non-opus default 400→550 in clawdnd_dm_timeout (opus unchanged 500). Updated the opus-tuning + wrapper-reliability guards; rewrote the (already-stale, routine=200) cold-open timeout proof to 360 + added opus/sonnet margin scenarios. F12-9 codex DM wrapper (play_codex_dm.sh): EXIT/INT/TERM trap now stamps provider_status "failed" on abnormal exit; codex exec wrapped in an inline worldos_timeout (WORLDOS_CODEX_TURN_TIMEOUT) + ONE session-safe retry; session budget ENFORCED via token-count spend accounting (WORLDOS_CODEX_USD_PER_MTOK) → "exhausted" stop (the budget envs were validated then never used). F12-10 claude lanes never wrote provider_status.json → factored clawdnd_write_provider_status (atomic, v1 schema) into the lib; play.sh + play_party.sh write running/stopped(turn_cap|budget|idle)/failed so the viewer buckets a dead/stopped session as no_provider instead of "unknown". F12-11 run_duo DM turn was unbounded + swallowed the real cause → wrap in worldos_timeout(clawdnd_dm_timeout); report on rc!=0 (dedup-guarded); replace the inline cold-open remint with the shared clawdnd_dm_remint_session_on_retry; keep empty-output retry as 2nd trigger. F12-12 play_party companion (actor) turn was unbounded → worldos_timeout ${WORLDOS_ACTOR_TIMEOUT:-120}; empty on failure (companion_moves skip-safe). F12-13 play.sh had no idle ceiling and no launch lock → port play_party's MAX_IDLE block + acquire/release the single-flight launch lock. Skipped as already-done (confirmed on main): F12-1/3/4/5/8 (#828), SYN-01 (#841). Tests: fast_gate Tier-0 191 passed; codex wrapper 34 (incl 6 new F12-9, 3 runtime fake-codex); wrapper-reliability/opus/adversarial 43; new shell tests test_play_party_actor_timeout.sh, test_run_duo_dm_timeout.sh, test_provider_status_claude_lanes.sh + extended single-flight + cold-open proof. Source: docs/audits/ENGINE-AUDIT-2026-06-11.md 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.
Root cause (audit SYN-01 = F12-7 + F12-14 + F13-5)
Spec:
docs/audits/ENGINE-AUDIT-2026-06-11.md§ SYN-01 + deep-spec comments on #757/#745. Verified against current main (post-#828).claude -presult carries NON-empty result text (verified verbatim:subtype:"success", is_error:true, api_error_status:401) → it bypassed the empty-only retry (qa/run_duo.shturn_retry) and the empty-only [playtest][CRITICAL] DM turn returns empty player-facing narration on engine-heavy beats #357 fallback gate (qa/lib_beat_driver.sh) → the auth-error string was chatlogged as DM prose.engine_loggeddedup → app.jsx drops it) or wrote an unflagged EMPTY dm row (post-fix(qa+engine): make the beat heartbeat real — flip progress at ingest, decontaminate recap/FTS/lean-tail/fallback (#749) #763 heartbeat-lane mode) — masked as "resolved" either way.chatlogAFTER sourcing the lib (run_duo.sh:135,ui_playtest.sh:138,run_party.sh:169), silently discardingclawdnd_chatlog_dm's{"fallback_recovered":true}stamp — which also had zero consumers in gate code.Fix (shared, in
qa/lib_beat_driver.sh— all 5 DM wrappers wired)clawdnd_dm_final_text(the ONE extraction front door, replacing the barejq … .resultin every wrapper) notes the final attempt's stream-json in$STATE_DIR/.dm_last_resultand echoes nothing on an error-class result, surfacing the failure + 401/403 re-auth hint via the existingclawdnd_report_attempt_failurepattern.clawdnd_resolve_dm_replythen parses the FINAL result event FIRST:is_error/api_error_status⇒ beat FAILED — never chat the error text, never fallback-recycle.clawdnd_dm_prebeat_mark+qa/dm_beat_mark.py(new; same prose filters as the [playtest][CRITICAL] DM turn returns empty player-facing narration on engine-heavy beats #357 fallback; fail-open) snapshot the session-log position before attempt 1. A fallback recovery counts as genuine (fallback_recovered:true) only if the DM logged NEW prose this beat; recycled-only prose fails the beat. The genuine [playtest][CRITICAL] DM turn returns empty player-facing narration on engine-heavy beats #357 win is preserved and pinned by test.record_dm_replyblank guard +clawdnd_chatlog_dm_failed: a failed/blank beat records ONE wrapper-authored, player-visible failure row stamped{"beat_failed":true}(chat-only by design: engine-logging it would dedup-hide repeats and pollute recap/FTS/lean-tail/fallback memory — documented inline).chatlog(verified drop-in superset, byte-identical 2-arg rows) is the only impl, so the honesty stamps land in every runner.qa/assert_behavioral.pydm_beat_honestycounts + reportsbeats_failed/fallback_recoveredper run (WARN only; the gate-discount policy stays [qa] dm_turn fallback masks a fully-dead DM beat as a 'resolved' turn with recycled prose #757's call).#828 preserved exactly:
worldos_timeout, retry deadline escalation, session re-mint, cold-open guards untouched — only the final extraction line is swapped and the pre-beat mark added (once per beat, before attempt 1).Tests
servers/engine/tests/test_dead_beat_classification.py— 25 new, red-first (16 failed pre-fix): 401 fixture → beat failed, error text never reaches chat, re-auth hint surfaced; recycled recovery → failed beat (not the previous beat's prose); NEW-prose-after-mark → genuine recovery; heartbeat rows never count as prose; heartbeat-lane dead beat → exactly one visible, never-hidden failure row; no-mark callers keep legacy behavior; override-deletion + wiring shape locks; assert_behavioral counts without gating;/bin/bash -non every touched script (macOS bash 3.2).qa/test_assert_behavioral.py+ qa static tests green.qa/fast_gate.shTier-0: PASS.Deviations / residuals
beats_failedis counted at the chat-row + gate-report layer (stamps +dm_beat_honesty+ a running stderr counter); the scores_db column is F13-4's (not yet landed) — wire the column there.engine_logged, and contaminate recap/FTS/lean-tail/[playtest][CRITICAL] DM turn returns empty player-facing narration on engine-heavy beats #357-fallback memory. Rationale inline atCLAWDND_DM_FAILED_BEAT_TEXT.qa/play_human.sh:33,qa/run_duo_openclaw.sh:72,scripts/play_codex_dm.sh:522(the latter two don't use the stamp path).Finding-ids: SYN-01 (F12-7, F12-14, F13-5). Refs #757, Refs #745. Milestone v1.0.5. DO NOT MERGE without review.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests