feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues - #1160
Conversation
📝 WalkthroughPriority Level: P3 This PR adds five new WS3a beat-obligation cues to
It keeps the change cue-only: the engine emits obligations but does not auto-resolve them, and uses defensive The change set also:
No additional issues were identified from the provided summary. WalkthroughAdds five WS3a per-beat cue kinds to ChangesWS3a Progression and Closure Cues
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant server.py as _compute_beat_obligations
participant campaign as Campaign snapshot
participant obligations as beat obligations
server.py->>campaign: read beats, combat, quests, locations, time
server.py->>obligations: append quest_unresolved_late or WS3a stall cues
Possibly related PRs
Confidence: 96% 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 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 `@servers/engine/server.py`:
- Around line 12517-12545: The detail text in the xp_unawarded obligation (in
the "detail" field around line 12534-12537) suggests using award_xp() as a
remediation, but this is misleading because it only mutates character XP without
clearing the monster's xp_value ledger, allowing the same dead monster to
re-trigger the cue on later beats. Either remove the misleading award_xp()
suggestion from the detail text, or update it to point to a dedicated engine
verb that atomically grants XP to all party members AND clears/consumes the
defeated monster's xp_value in a single operation to prevent re-triggering.
In `@skills/dungeon-master/AGENT.md`:
- Line 26: The wording describing the quest-stall closure obligation
inaccurately characterizes when the `quest_unresolved_late` cue fires. The
phrase "every quest still untouched" overstates the trigger condition. Replace
this imprecise description with language that accurately reflects the actual cue
logic: the `quest_unresolved_late` cue only fires when no quest has been
completed AND no objective has ever been recorded as done, and it remains silent
if a quest-specific cue is already surfaced. This ensures the DM has the correct
understanding of when the engine will actually surface this stall signal rather
than expecting a broader condition.
In `@skills/dungeon-master/SKILL.md`:
- Line 50: In the documentation for the `quest_unresolved_late` obligation, the
current description only states that the cue fires when "not one quest objective
has been recorded done" at 8+ beats. Update this description to include the
missing guard conditions: the cue requires that zero completed quests also exist
AND it is suppressed (does not fire) if `quest_resolvable` or `quest_stalled` is
already present in the obligations list. This alignment will ensure the
documented contract matches the actual engine precedence rules and prevents the
DM from attempting redundant handling.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c8bff268-a6b2-48cc-a091-ea9e33a92e82
📒 Files selected for processing (6)
qa/fast_gate.shqa/test_ws3a_progression_invariants.pyservers/engine/server.pyservers/engine/tests/test_beat_obligations.pyskills/dungeon-master/AGENT.mdskills/dungeon-master/SKILL.md
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Maintain a Baldur's-Gate-3-prestige voice: generous, brisk, fair storyteller who spotlights the player and their companion, says 'yes, and' to clever ideas, and keeps danger honest using dice and rules from the engine
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Never flatter the player with unearned wins; the world pushes back, NPCs have their own wants, and unearned concessions are worthless
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Narrate the world and adjudicate outcomes in the present, in-scene; never speak or decide for the player's character
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Use act/beat vocabulary (cold open, act, beat, midpoint, reversal, inciting incident, spine hook, payoff) as private craft language only — never label these in player-facing narration
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Never narrate dice/check tallies or stage-direction status summaries (e.g., 'three failed social checks', 'meeting beat complete') in player-facing narration
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Act 1 — open a grounded, personal scene (4-beat cold open for brand-new campaigns), establish tone and real inciting incident, and hook that matters to a person, not the world yet
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Act 2 — escalate with friction that sticks (a real attempt fails, a choice exacts a price), and deliver a genuine midpoint reversal where the ally is the informant, the prize is gone, the safe path was a trap, or the cost lands on the hero personally
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Do not smooth over or re-roll away the midpoint reversal in Act 2; let it land as a genuine turn to absorb
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Act 3 — converge threads into a decisive, dramatized confrontation and pay off what Act 1 set up and what the midpoint cost
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Co-author the climax: hand the player the discovery and let them react — confrontations come as interruptible exchanges, never a single block of villain monologue or DM-narrated revelation
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: In Act 3 denouement, signal every live named thread (a foe's fate, an NPC's stance) so nothing important vanishes; no new sub-plots in final beats
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Advance the clock when fiction moves forward using `advance_time(phases=N)`, `travel_to(..., advance_time=True)`, or `long_rest`
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Party must travel to ≥2 locations per session using `travel_to` or `add_location(make_current=True)` for new places; narrate each location's tone before the player acts
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Create and introduce ≥1 new named NPC per session using `create_character` with a voice and at least one quoted line; mark `met=True` when party meets them on-screen
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Close fights with `end_combat` (don't leave active into next beat), award XP on defeated monsters, resolve quests with `complete_objective` or `complete_quest(evolves_to=…)` — never leave the mechanical loop stalled
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: All time and state changes must go through engine tools — the engine is the single source of truth; never assert state in prose without matching engine calls
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-06-23T04:11:27.154Z
Learning: Read state with `get_state` to re-ground each beat; write state only through engine tools, never through prose assertion
🪛 Ruff (0.15.18)
qa/test_ws3a_progression_invariants.py
[warning] 47-47: Use @pytest.fixture over @pytest.fixture()
Remove parentheses
(PT001)
[warning] 63-63: Missing type annotation for **arc_kw
(ANN003)
[warning] 72-72: Unused function argument: state_dir
(ARG001)
[warning] 93-93: Unused function argument: state_dir
(ARG001)
[warning] 128-128: Unused function argument: state_dir
(ARG001)
[warning] 136-136: Unused function argument: state_dir
(ARG001)
[warning] 146-146: Unused function argument: state_dir
(ARG001)
[warning] 159-159: Unused function argument: state_dir
(ARG001)
[warning] 171-171: Unused function argument: state_dir
(ARG001)
[warning] 183-183: Unused function argument: state_dir
(ARG001)
[warning] 208-208: Unused function argument: state_dir
(ARG001)
[warning] 222-222: Unused function argument: state_dir
(ARG001)
[warning] 242-242: Unused function argument: state_dir
(ARG001)
[warning] 258-258: Unused function argument: state_dir
(ARG001)
servers/engine/tests/test_beat_obligations.py
[warning] 730-730: Missing return type annotation for private function _location
(ANN202)
[warning] 730-730: Boolean default positional argument in function definition
(FBT002)
|
triage 2026-07-02: RESCUE candidate — needs rebase onto current main; see /tmp/triage/prs_human.md |
Add 5 progression/closure obligations to _compute_beat_obligations (the SOLE every-beat cue seam ridden by persist_beat + scene_context.durable), keeping the HARD mechanical loop from quietly stalling — the relationship cues (#1017/#1018/ - party_stuck_one_location (med): 8+ act-local beats, <2 visited locations, no in-place-progression (byte-identical to assert_behavioral's party_traveled exception) -> travel_to / add_location. - combat_left_hanging (med): combat active but no living hostile (mirrors end_combat's order-based detection) -> end_combat. Owns the beat over xp_unawarded while combat is active. - xp_unawarded (med): xp-mode, NON-combat, living party member, a defeated monster still carrying xp_value>0 (proactive twin of the xp_not_orphaned FATAL) -> end_combat / award_xp. - clock_dm_frozen (low): substantial beats, day==1 & morning, not in combat, HONEST snapshot proxy; fires only when visited>=2 (party_stuck owns the clock otherwise) -> advance_time / long_rest / downtime. - quest_unresolved_late (med): substantial beats, a quest exists, zero quests completed AND no objective ever recorded done, anti-spam vs quest_resolvable/ quest_stalled -> complete_objective / complete_quest. All CUE-ONLY (Option A — no engine auto-action); pure reads of engine-mutated gauges with defensive getattr (older/partial snapshot degrades a cue to skipped); precedence gates collapse the worst case to ~2-3 cues. ADDITIVE: a fully-progressed snapshot still yields [] (no obligations key). Pin _PARTY_STUCK_BEATS=8 to assert_behavioral's SINGLE_SCENE_MIN_BEATS. Tests (TDD): per-kind FIRE+CLEAR + a fully-progressed-snapshot empty-digest case in test_beat_obligations.py; new deterministic qa/test_ws3a_progression_invariants.py (NO LLM) proves each named verb MOVES its gauge on a real persisted campaign + the cue fires-then-clears, wired into qa/fast_gate.sh. Skill docs (SKILL.md step-6b + AGENT.md closure obligations) name the 5 cues. fast_gate: GREEN (253 passed). focused suite: 83 passed.
5702092 to
0f56013
Compare
evaOS review status: stale headPR: #1160 - feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #1160 Details: live=8923c474aa3b83d7fa0dfd90cd5507ed12271540 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…(end_combat) — bare award_xp invites duplicate awards (CodeRabbit Major, verified vs _award_kill_xp idempotent-zero at server.py:6919); tighten AGENT.md quest-stall wording
…Rabbit); drop unused imports (code-quality)
|
Review findings addressed (architect-verified against source): (1) xp_unawarded duplicate-award risk CONFIRMED and fixed — |
evaOS review status: completedPR: #1160 - feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1160 Review URL: #1160 (review) |
Findings addressed and verified in commits after this review (xp_unawarded remediation fixed, wording tightened, imports dropped); threads resolved with evidence.
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #1160 - feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues
Head: 263beb982bfa7a1861b74452a6fa75209299b835 into main. Review event: COMMENT.
Estimated review effort: 5/5 (~58 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/fast_gate.sh |
modified | +6/-1 | Changed file | Low |
qa/test_ws3a_progression_invariants.py |
added | +267/-0 | Test coverage | Elevated: large change |
servers/engine/server.py |
modified | +177/-0 | Changed file | Moderate: validated P2 finding |
servers/engine/tests/test_beat_obligations.py |
modified | +319/-0 | Test coverage | Elevated: large change |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 1, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
- Docs only: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1017, #1018, #1019.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
…_combat inapplicable, evaos P2); fix combat_left_hanging mirror-site citation (evaos P3, 6467→~7009); SKILL.md 6b match; follow-up #1312 for engine reconcile of stranded kill-XP
|
Both evaos findings verified against source and fixed in dc7bc55: P2 (xp_unawarded → end_combat inapplicable): CONFIRMED, fixed. The cue gates on `not combat_active`, and `end_combat` requires an active fight — so end_combat is structurally inapplicable here (you're right). `_award_kill_xp` (server.py:6919) is the kill-time hook that zeroes `xp_value`; an out-of-combat dead monster with `xp_value>0` means that hook was missed on the death path. Reworded the remediation to `award_xp(character_id, amount, reason)` (the only grant path out of combat), with an honest note that it doesn't consume `xp_value` so the advisory may re-surface — award once. The real fix (a tool that grants AND consumes stranded kill-XP) is tracked as #1312. This intentionally supersedes the earlier CodeRabbit "don't use award_xp" steer, which assumed a cleaner path exists — for this out-of-combat cue there isn't one. P3 (combat_left_hanging cites server.py:6467): CONFIRMED, fixed. end_combat's live-hostile detection is the `live_hostiles` comprehension at ~server.py:7009, not 6467. Comment corrected. (The combat_left_hanging cue itself is correct — it fires when combat IS active, so its end_combat recommendation is right.) 95 focused tests + fast_gate green post-fix. |
evaOS review status: in progressPR: #1160 - feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues evaOS review is running for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1160 |
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/test_ws3a_progression_invariants.py`:
- Around line 181-200: The xp_unawarded test currently only verifies that the
cue disappears after directly zeroing Character.xp_value through the store, not
that any engine-facing action clears it. In
test_xp_unawarded_cue_fires_out_of_combat_then_clears_when_xp_value_zeroed,
either rename/mark the test to clearly state it checks gauge-read behavior only,
or add a companion assertion around award_xp showing that the out-of-combat
grant path does not clear xp_unawarded yet. Use the existing
store.load_campaign/store.save_campaign flow and the award_xp / _award_kill_xp /
end_combat symbols to keep the intent explicit.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a0d678f1-03dd-4e19-b2a8-7bf219074f29
📒 Files selected for processing (6)
qa/fast_gate.shqa/test_ws3a_progression_invariants.pyservers/engine/server.pyservers/engine/tests/test_beat_obligations.pyskills/dungeon-master/AGENT.mdskills/dungeon-master/SKILL.md
💤 Files with no reviewable changes (4)
- skills/dungeon-master/AGENT.md
- skills/dungeon-master/SKILL.md
- servers/engine/server.py
- servers/engine/tests/test_beat_obligations.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: test
- GitHub Check: viewer-tests
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (csharp)
- GitHub Check: test
- GitHub Check: viewer-tests
⚠️ CI failures not shown inline (2)
GitHub Actions: LLM Quality Gate (advisory) / quality-gate: feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "[selfcheck] bash -n on the duo runner + scorer"�[0m
�[36;1mbash -n qa/run_duo.sh�[0m
�[36;1mbash -n qa/score.sh�[0m
�[36;1mtest -x qa/run_duo.sh�[0m
�[36;1mtest -x qa/score.sh�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] regression detector imports + answers --help (pure reader, no DB write)"�[0m
�[36;1muv run --directory servers/engine python "${GITHUB_WORKSPACE}/qa/detect_regression.py" --help >/dev/null�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] scorer guard path emits a hashed artifact into a TEMP dir (gateway-free)"�[0m
�[36;1mtmp="$(mktemp -d)"�[0m
�[36;1mprintf '# transcript\n' > "$tmp/t.md"�[0m
�[36;1mprintf '{}\n' > "$tmp/state.json"�[0m
�[36;1mWORLDOS_SCORE_GUARD_ONLY=1 bash qa/score.sh \�[0m
�[36;1m "$tmp/t.md" "$tmp/state.json" qa/rubric.md qa/score_schema.json "$tmp/out.json" 0.01�[0m
�[36;1mtest -s "$tmp/out.json"�[0m
�[36;1mrm -rf "$tmp"�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] jq present (scorecard tool)"�[0m
�[36;1mcommand -v jq >/dev/null || { echo "::error title=LLM Quality Gate::jq missing on runner"; exit 1; }�[0m
GitHub Actions: LLM Quality Gate (advisory) / 0_quality-gate.txt: feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "[selfcheck] bash -n on the duo runner + scorer"�[0m
�[36;1mbash -n qa/run_duo.sh�[0m
�[36;1mbash -n qa/score.sh�[0m
�[36;1mtest -x qa/run_duo.sh�[0m
�[36;1mtest -x qa/score.sh�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] regression detector imports + answers --help (pure reader, no DB write)"�[0m
�[36;1muv run --directory servers/engine python "${GITHUB_WORKSPACE}/qa/detect_regression.py" --help >/dev/null�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] scorer guard path emits a hashed artifact into a TEMP dir (gateway-free)"�[0m
�[36;1mtmp="$(mktemp -d)"�[0m
�[36;1mprintf '# transcript\n' > "$tmp/t.md"�[0m
�[36;1mprintf '{}\n' > "$tmp/state.json"�[0m
�[36;1mWORLDOS_SCORE_GUARD_ONLY=1 bash qa/score.sh \�[0m
�[36;1m "$tmp/t.md" "$tmp/state.json" qa/rubric.md qa/score_schema.json "$tmp/out.json" 0.01�[0m
�[36;1mtest -s "$tmp/out.json"�[0m
�[36;1mrm -rf "$tmp"�[0m
�[36;1m�[0m
�[36;1mecho "[selfcheck] jq present (scorecard tool)"�[0m
�[36;1mcommand -v jq >/dev/null || { echo "::error title=LLM Quality Gate::jq missing on runner"; exit 1; }�[0m
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: As the Dungeon Master for a WorldOS campaign, maintain the stable DM identity and apply the companion `dungeon-master` skill contract as the full craft ruleset.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Use a generous, brisk, fair storyteller voice with a Baldur's Gate 3-style prestige tone; spotlight the player and companion, say 'yes, and' to clever ideas, and keep danger and rules honest.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Never speak or decide for the player's character; narrate the world and adjudicate outcomes only, and keep prose evocative, controlled, present-tense, and fully in-scene rather than as an after-action summary.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Think and run sessions using the private act/beat vocabulary internally, but never expose that scaffolding to player-facing narration; player text must remain in-world prose and quoted dialogue only.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Do not narrate dice/check tallies or stage-direction status summaries; never leak system scaffolding or meta commentary into player-facing text.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Run every session as a three-act arc: Act 1 opens with a grounded personal inciting incident and human-scale hook; Act 2 escalates with friction, a cost, and a mandatory midpoint reversal; Act 3 delivers a decisive climax, payoff, and clean denouement.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: Make the climax co-authored: hand the player the discovery and let them react; confrontations should be interruptible exchanges, not a single block of villain monologue or DM-narrated revelation.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: By the end of a substantial session, ensure the clock advances, the party travels to at least two locations, and new named NPCs enter, speak, and are marked met when encountered on-screen.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: By the end of a substantial session, fights must be closed, XP must be awarded, and quests/objectives that are clearly progressing must record progress or completion.
Learnt from: CR
Repo: electricsheephq/WorldOS
Timestamp: 2026-07-02T22:51:13.195Z
Learning: All time and state changes must go through engine tools; never assert state in prose without the matching tool call, and re-ground each beat with `get_state` before writing changes.
🪛 Ruff (0.15.20)
qa/test_ws3a_progression_invariants.py
[warning] 45-45: Use @pytest.fixture over @pytest.fixture()
Remove parentheses
(PT001)
[warning] 61-61: Missing type annotation for **arc_kw
(ANN003)
[warning] 70-70: Unused function argument: state_dir
(ARG001)
[warning] 91-91: Unused function argument: state_dir
(ARG001)
[warning] 126-126: Unused function argument: state_dir
(ARG001)
[warning] 134-134: Unused function argument: state_dir
(ARG001)
[warning] 144-144: Unused function argument: state_dir
(ARG001)
[warning] 157-157: Unused function argument: state_dir
(ARG001)
[warning] 169-169: Unused function argument: state_dir
(ARG001)
[warning] 181-181: Unused function argument: state_dir
(ARG001)
[warning] 206-206: Unused function argument: state_dir
(ARG001)
[warning] 220-220: Unused function argument: state_dir
(ARG001)
[warning] 240-240: Unused function argument: state_dir
(ARG001)
[warning] 256-256: Unused function argument: state_dir
(ARG001)
🔇 Additional comments (2)
qa/test_ws3a_progression_invariants.py (1)
70-267: LGTM!qa/fast_gate.sh (1)
66-71: LGTM!
There was a problem hiding this comment.
Walkthrough
PR: #1160 - feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues
Head: dc7bc55b300f022ca5e4cf687a80caa7cc173553 into main. Review event: COMMENT.
Estimated review effort: 5/5 (~58 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/fast_gate.sh |
modified | +6/-1 | Changed file | Low |
qa/test_ws3a_progression_invariants.py |
added | +267/-0 | Test coverage | Elevated: large change |
servers/engine/server.py |
modified | +177/-0 | Changed file | Moderate: validated P2 finding |
servers/engine/tests/test_beat_obligations.py |
modified | +319/-0 | Test coverage | Elevated: large change |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 1, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
- Auth: 1
- Docs only: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1017, #1018, #1019.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
Latest-round threads triaged (all verified vs source; none are must-fix defects — per AGENTS.md "warning-only bot suggestions are non-blocking unless they identify a real defect"):
Functionally correct + tested (95 focused + fast_gate green). Resolving these as non-blocking. |
All findings verified vs source and addressed: by-design w/ tracked follow-up (#1312) or cosmetic (non-defect per AGENTS.md). Threads resolved with evidence.
…s-closing verb; bare add_consequence leaves the quest active (codex P2, same class as the #1160 xp_unawarded advice fix)
#1317) * feat(engine): #1313 endgame quest-resolution cue (cue-half iteration 2) The Option-3 next_action nudge (#1314) raised general engagement but the measured residual behavioral RED (rri-a1-opt3) was one thing: a quest left ACTIVE at session end. Escalate the existing cue machinery in the WRAP WINDOW. Wrap window = arc.act==3 AND arc.climax_landed (set by the DM's own mark_climax) — the one honest engine gauge for 'the story has passed its peak and is closing', a positive engine flag (not a fiction read of harness prose) and mutually exclusive with act_climax_owed by construction (that fires only while climax_landed is False), so the endgame cue lives strictly in the denouement. In that window an active quest emits a single HIGH quest_endgame_unresolved cue whose detail is the resolution imperative (complete_quest(evolves_to=…) / complete_objective / add_consequence hand-off). Being HIGH + severity-sorted, it becomes the next_action imperative in the final beats. Precedence: it REPLACES the generic quest_resolvable/quest_stalled (section 3) and quest_unresolved_late (3b) for the wrap window — one cue, not three, no double-fire. Additive: pure derivation, no new state; empty-digest byte-identity preserved (arc-less/older/pre-climax snapshots read the window False). NO teeth (advisory). - server.py: _in_wrap_window derivation + quest_endgame_unresolved cue + precedence - test_beat_obligations.py: 9 new tests (fires in-window, absent out-of-window/ resolved/arcless, precedence, next_action lift, byte-identity); fully-progressed fixture updated (a thread active at wrap is now correctly owed) - SKILL.md: step-6b obligations paragraph extended with the new kind * fix(#1313): endgame-cue hand-off advice — complete_quest is the status-closing verb; bare add_consequence leaves the quest active (codex P2, same class as the #1160 xp_unawarded advice fix) * test(#1313): pin the per-quest fan-out of quest_endgame_unresolved (evaos P3 — guards against a hoisted/deduped append refactor) --------- Co-authored-by: Eva <arncalso@gmail.com>
1 participant
What
Add 5 progression/closure obligations to
_compute_beat_obligations()(the SOLE every-beat cue seam ridden bypersist_beat+scene_context.durable, already used by #1017/#1018/#1019). The relationship cues cover the SOFT story-superstructure; WS3a covers the HARD mechanical loop so it can't quietly stall:party_stuck_one_locationbeats_in_act>=8AND<2visited locations AND not the in-place-progression exceptiontravel_to(advance_time=True)/add_location(make_current=True)combat_left_hangingcombat.activeAND no living hostile in the orderend_combat(resolution=…)xp_unawardedxp_value>0end_combat(auto-awards) /award_xpclock_dm_frozenday==1AND morning AND not in combat ANDvisited>=2advance_time/long_rest/downtimequest_unresolved_latecomplete_objective/complete_quest(evolves_to=…)Invariants honored
locations[].visited,combat.active/order,characters[].dead/current_hp/xp_value,day/time_of_day,quests[].status/completed_objectives) — never tool-counts, beat-history, or Decision prose.combat_left_hangingowns the beat overxp_unawardedwhile combat is active;party_stuckowns the clock overclock_dm_frozen(clock fires only oncevisited>=2);quest_unresolved_lateis suppressed when any quest is alreadyquest_resolvable/quest_stalledthis beat.party_stuck's exception is BYTE-IDENTICAL toassert_behavioral'sparty_traveledexception (qa/assert_behavioral.py:677):visited>=1 AND clock_advanced AND a completed quest AND beats>=8. New constant_PARTY_STUCK_BEATS = 8pinned to that gate'sSINGLE_SCENE_MIN_BEATS.[](noobligationskey) — the additive contract is proven by a newtest_fully_progressed_snapshot_yields_no_obligationsand the existing healthy-fixture persist_beat/scene_context omit-key tests still pass.Tests (TDD)
servers/engine/tests/test_beat_obligations.py: per-kind FIRE + CLEAR tests for all 5 + precedence tests + the fully-progressed empty-digest case. (71 passed.)qa/test_ws3a_progression_invariants.py(NEW, deterministic, NO LLM): for each cue, calls the named engine verb DIRECTLY on a real persisted campaign and asserts the gauge MOVED, PLUS the cue fires-then-clears. (12 passed.) Wired intoqa/fast_gate.sh's inner TESTS list.skills/dungeon-master/SKILL.md(step-6b obligations) +skills/dungeon-master/AGENT.md(closure obligations) name the 5 cues.Validation
uv run --directory servers/engine python -m pytest -q -p no:xdist ../../qa/test_ws3a_progression_invariants.py tests/test_beat_obligations.py→ 83 passed.bash qa/fast_gate.sh→ GREEN, 253 passed.