feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues#1160
Open
100yenadmin wants to merge 1 commit into
Open
feat(engine): WS3a — DM-unavoidable per-beat progression/closure cues#1160100yenadmin wants to merge 1 commit into
100yenadmin wants to merge 1 commit into
Conversation
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/ #1019) cover the SOFT story-superstructure; these cover travel/combat/XP/clock/quest: - 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.
📝 WalkthroughPriority Level: P1SummaryThis PR successfully implements five progression and closure obligation cues in Implementation QualityStrengths:
Testing Coverage:
Integration
Minor ObservationThe RecommendationReady for merge. Implementation is sound, test coverage is comprehensive, and integration is complete. All five cues follow the "CUE-ONLY" pattern correctly—the engine generates alerts; the DM calls the named verbs. The design properly prevents false positives via precedence rules and anti-spam checks while maintaining backward compatibility through the additive contract. WalkthroughAdds five WS3a "DM-unavoidable" per-beat cue kinds ( ChangesWS3a DM-Unavoidable Progression/Closure Cues
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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
🧪 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)
Comment on lines
+12517
to
+12545
| # WS3a-3. xp_unawarded (med) — leveling_mode=='xp', NOT in combat, a living party member, and a | ||
| # defeated monster still carries xp_value > 0 (the kill-time award never fired / was bypassed). | ||
| # Proactive twin of assert_behavioral's xp_not_orphaned run-end FATAL (qa/assert_behavioral.py:578) | ||
| # — mirrors its guards (xp-mode, party_alive, non-combat) so the cue and the gate agree. Gated | ||
| # NON-combat ONLY: mid-fight a kept xp_value is normal (it's awarded at end_combat); combat_left_ | ||
| # hanging owns the active-combat case, so this fires only once the fight is genuinely closed. | ||
| if not combat_active and getattr(c, "leveling_mode", "xp") == "xp": | ||
| party_alive = any( | ||
| characters.get(pid) is not None and not getattr(characters.get(pid), "dead", False) | ||
| for pid in party_ids | ||
| ) | ||
| orphaned = [ | ||
| ch for ch in characters.values() | ||
| if getattr(ch, "kind", "") == "monster" | ||
| and getattr(ch, "dead", False) | ||
| and (getattr(ch, "xp_value", 0) or 0) > 0 | ||
| ] | ||
| if party_alive and orphaned: | ||
| names = ", ".join((getattr(ch, "name", None) or "?") for ch in orphaned) | ||
| obligations.append({ | ||
| "kind": "xp_unawarded", | ||
| "severity": "med", | ||
| "character_ids": [getattr(ch, "id", None) for ch in orphaned], | ||
| "detail": ( | ||
| f"Defeated monster(s) {names} still carry unawarded XP — progression silently " | ||
| "lost. end_combat (it auto-awards defeated foes' XP) or award_xp(character_id, " | ||
| "amount, reason) for each party member so the kill actually counts." | ||
| ), | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Misleading xp_unawarded remediation can cause duplicate XP awards (confidence: 93%)
The cue is keyed on monster-side state (xp_value > 0), but the detail text offers award_xp(...) as a standalone fix. That mutates character XP, not the monster XP ledger, so the same dead monster can keep re-triggering xp_unawarded on later beats and invite repeated grants.
Proposed minimal fix
- f"Defeated monster(s) {names} still carry unawarded XP — progression silently "
- "lost. end_combat (it auto-awards defeated foes' XP) or award_xp(character_id, "
- "amount, reason) for each party member so the kill actually counts."
+ f"Defeated monster(s) {names} still carry unawarded XP — progression silently "
+ "lost. Resolve through an engine path that consumes defeated foes' xp_value "
+ "(end_combat auto-awards and zeroes the defeated monsters' XP ledger)."If manual post-combat awards are required, add a dedicated engine verb that atomically grants XP and consumes the defeated monsters’ xp_value, then point this cue to that verb.
🤖 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 `@servers/engine/server.py` around lines 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.
| - **The clock advances.** A session still at *morning* in the opening location is frozen. Advance time when the fiction moves forward — `advance_time(phases=N)` / `travel_to(..., advance_time=True)` / `long_rest`. | ||
| - **The party travels to ≥2 locations.** Move along connections (`travel_to`, `advance_time=True` for a real journey) or `add_location(make_current=True)` for somewhere new; narrate each new place's tone yourself before the player acts. | ||
| - **New named faces enter and SPEAK.** The seeded roster is a *starting cast*, not the whole world. `create_character` a named NPC with a voice and at least one quoted line; mark `met=True` when the party meets them on-screen. (Across 57 prior campaigns a brand-new on-screen NPC was NEVER created — this is the obligation most often missed; do not let a session pass without peopling the world.) | ||
| - **Fights CLOSE, XP LANDS, quests RESOLVE — the mechanical loop never stalls.** The engine watches these every beat and surfaces a cue (the `obligations` list) the moment one stalls — act on it: a fight that has no living hostile left must be `end_combat`-ed (don't leave it active into the next beat); a defeated monster's XP must land (`end_combat` auto-awards, or `award_xp`); a quest the party is clearly progressing must record that progress (`complete_objective` / `complete_quest(evolves_to=…)`), not just be narrated. A run that ends with the party stuck in the opening room, the clock frozen at day-1 morning, a fight left hanging, XP never awarded, or every quest still untouched reads as a stall — these are the closure obligations the per-beat cues exist to stop. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tighten the quest-stall wording. Confidence: 92%.
quest_unresolved_late is not "every quest still untouched" in the engine. The cue only fires when no quest has been completed and no objective has ever been recorded done, and it stays silent if a quest-specific cue is already present. As written, this overstates the trigger and can send the DM chasing a stall the engine will never surface.
♻️ Proposed wording fix
- A run that ends with the party stuck in the opening room, the clock frozen at day-1 morning, a fight left hanging, XP never awarded, or every quest still untouched reads as a stall — these are the closure obligations the per-beat cues exist to stop.
+ A run that ends with the party stuck in the opening room, the clock frozen at day-1 morning, a fight left hanging, XP never awarded, or no quest/objective progress ever being recorded reads as a stall — these are the closure obligations the per-beat cues exist to stop.📝 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
| - **Fights CLOSE, XP LANDS, quests RESOLVE — the mechanical loop never stalls.** The engine watches these every beat and surfaces a cue (the `obligations` list) the moment one stalls — act on it: a fight that has no living hostile left must be `end_combat`-ed (don't leave it active into the next beat); a defeated monster's XP must land (`end_combat` auto-awards, or `award_xp`); a quest the party is clearly progressing must record that progress (`complete_objective` / `complete_quest(evolves_to=…)`), not just be narrated. A run that ends with the party stuck in the opening room, the clock frozen at day-1 morning, a fight left hanging, XP never awarded, or every quest still untouched reads as a stall — these are the closure obligations the per-beat cues exist to stop. | |
| - **Fights CLOSE, XP LANDS, quests RESOLVE — the mechanical loop never stalls.** The engine watches these every beat and surfaces a cue (the `obligations` list) the moment one stalls — act on it: a fight that has no living hostile left must be `end_combat`-ed (don't leave it active into the next beat); a defeated monster's XP must land (`end_combat` auto-awards, or `award_xp`); a quest the party is clearly progressing must record that progress (`complete_objective` / `complete_quest(evolves_to=…)`), not just be narrated. A run that ends with the party stuck in the opening room, the clock frozen at day-1 morning, a fight left hanging, XP never awarded, or no quest/objective progress ever being recorded reads as a stall — these are the closure obligations the per-beat cues exist to stop. |
🤖 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 `@skills/dungeon-master/AGENT.md` at 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.
| 6. **Resolve via tools** — checks/attacks/rules through the engine. **For a skill check, call `skill_check(character_id, skill, dc)`** (or `social_check` when it targets an NPC's attitude) — they roll with the character's CORRECT modifier derived from the sheet. **Never hand-compute a bonus into a raw `roll()`** — that's the #1 mechanical error (a wrong ability mod, a missed proficiency). Use bare `roll()` only for dice that aren't a character's skill (a wandering-monster die, a random table). **If the move was an attack or a hostile spell on a foe — or hostiles are attacking — that is COMBAT: do NOT resolve it with a `skill_check` or narration. Call `start_combat` (+ `spawn_monster` for un-stat-blocked foes) FIRST**, then run the engine fight — see the "combat is WARRANTED" non-negotiable below and `reference/combat.md` (companion turns via `companion_suggest_action`, the action economy, the turn loop, damage/saves). Reach the companion only through its tool boundary; never silently skip its turn or fold its lines into your narration. **If a move arrives tagged `[set_seed_param] param=value`** (the player changed a World-Seed dial — tone / narration / GM strictness / chronicle voice / anachronism / chronicler's notes, or a gated rule like difficulty / permadeath / fate dice / item destruction — from the Seed screen), that is **DM-side configuration, not an in-scene action**: apply it with `set_seed_param(campaign_id, param, value)` (add `force=True` only if the player explicitly confirmed a retroactive mid-chronicle change — the tool returns `applied`/`warning`), then honor it going forward (it also surfaces on `get_state.seed_params`) and just briefly acknowledge it out-of-scene. Do **not** roll, advance the clock, or narrate the PC doing something for a seed-param move. | ||
| 6a. **Stream the OUTCOME — emit the felt result via `log_event(kind="narration"/"dialogue", …)` as soon as the dice are in.** Now that the mechanics resolved, write the *felt* result of the roll/attack/spell (never the bare number — see "Dice live inside the tools") as another `log_event`, so the resolution streams onto the player's dashboard too, mid-turn — the scene built in step 2, and now the result lands while you finish bookkeeping. This is the second half of the streaming win: setup-prose first (step 2), outcome-prose here (6a), both live, *before* the turn ends. Between them, the player has watched the whole beat arrive instead of staring at a stalled counter. | ||
| 6b. **Act on engine obligations — MANDATORY before you persist.** `scene_context` (step 1) and `persist_beat` (step 7) return an **`obligations`** list (present only when the engine sees a relationship/quest system going unengaged — it's absent on a healthy beat). For EACH obligation, take the named action THIS beat or the next: an **un-gauged companion** (`companion_gauge_unauthored`) → a freely-recruited / generated companion starts with NO approval vocabulary, so `record_decision(approval_tags=…)` can't move them and their arc is inert; **`author_companion_gauges(companion_id, approval_likes=[…], approval_dislikes=[…])`** with a few cause-keys that fit who they are (add `betrayal_threshold=` to let the bond break if mistreated) — do this the beat they join, before any values-choice; a **companion with no personal quest** (`companion_quest_unauthored`) → a gauged companion still has no engine-tracked personal thread; **`set_companion_quest_arc(companion_id, arc={title, stages:[…]})`** to author one the engine can advance (`advance_companion_quest_arc`) and surface at re-ground, optionally linked to a `personal_quest` arc gate so a deepening bond opens it; a **frozen companion** (`companion_approval_frozen`) → tag the cause on this beat's values-moment with `record_decision(..., approval_tags=[…the companion's likes…])` (or `adjust_attitude` for an off-decision nudge), or play a `camp_scene` at a pause; a **near arc gate** (`companion_arc_gate_near`) → move that companion's regard toward it; an **overdue camp** (`camp_overdue`) → `long_rest` then `camp_scene` to land companion beats; a **resolvable quest** (`quest_resolvable`) → `complete_quest(quest_id, evolves_to=…)` to close AND echo it; a **stalled quest** (`quest_stalled`) → push an objective (`complete_objective`) or `complete_quest` it; a **resolved quest with no echo** (`quest_no_echo`) → set `evolves_to` / `add_consequence`; a **skipped camp** (`camp_scene_skipped`) → the party rested but landed no camp beat, so `camp_scene` now to give each rested companion their social beat (their regard stays frozen otherwise); an **approaching betrayal** (`companion_betrayal_approaching`) → a present companion's bond has curdled past its breaking point: **FORESHADOW the fracture THIS beat** (a cold look, a withheld word, a loyalty openly questioned) so the turn never springs from nowhere — do **NOT** trigger the agenda yourself; when the bond breaks the engine stages it as a **real `attack`** and you dramatize the fallout. **Companions are GAUGED, not just narrated; quests RESOLVE and EVOLVE, not just get mentioned** — a companion whose `attitude_value` never leaves 0 and a quest that ends a multi-location arc still `active` are the exact failures this list exists to stop. | ||
| 6b. **Act on engine obligations — MANDATORY before you persist.** `scene_context` (step 1) and `persist_beat` (step 7) return an **`obligations`** list (present only when the engine sees a relationship/quest system going unengaged — it's absent on a healthy beat). For EACH obligation, take the named action THIS beat or the next: an **un-gauged companion** (`companion_gauge_unauthored`) → a freely-recruited / generated companion starts with NO approval vocabulary, so `record_decision(approval_tags=…)` can't move them and their arc is inert; **`author_companion_gauges(companion_id, approval_likes=[…], approval_dislikes=[…])`** with a few cause-keys that fit who they are (add `betrayal_threshold=` to let the bond break if mistreated) — do this the beat they join, before any values-choice; a **companion with no personal quest** (`companion_quest_unauthored`) → a gauged companion still has no engine-tracked personal thread; **`set_companion_quest_arc(companion_id, arc={title, stages:[…]})`** to author one the engine can advance (`advance_companion_quest_arc`) and surface at re-ground, optionally linked to a `personal_quest` arc gate so a deepening bond opens it; a **frozen companion** (`companion_approval_frozen`) → tag the cause on this beat's values-moment with `record_decision(..., approval_tags=[…the companion's likes…])` (or `adjust_attitude` for an off-decision nudge), or play a `camp_scene` at a pause; a **near arc gate** (`companion_arc_gate_near`) → move that companion's regard toward it; an **overdue camp** (`camp_overdue`) → `long_rest` then `camp_scene` to land companion beats; a **resolvable quest** (`quest_resolvable`) → `complete_quest(quest_id, evolves_to=…)` to close AND echo it; a **stalled quest** (`quest_stalled`) → push an objective (`complete_objective`) or `complete_quest` it; a **resolved quest with no echo** (`quest_no_echo`) → set `evolves_to` / `add_consequence`; a **skipped camp** (`camp_scene_skipped`) → the party rested but landed no camp beat, so `camp_scene` now to give each rested companion their social beat (their regard stays frozen otherwise); an **approaching betrayal** (`companion_betrayal_approaching`) → a present companion's bond has curdled past its breaking point: **FORESHADOW the fracture THIS beat** (a cold look, a withheld word, a loyalty openly questioned) so the turn never springs from nowhere — do **NOT** trigger the agenda yourself; when the bond breaks the engine stages it as a **real `attack`** and you dramatize the fallout. The list also names the HARD **progression / closure** stalls (WS3a) — keep the mechanical loop moving, not just the relationships: a **party stuck in one scene** (`party_stuck_one_location`) → the run has gone 8+ beats without leaving the opening place; move the story with `travel_to(destination_id, advance_time=True)` (or `add_location(make_current=True)` if there's no edge yet), then open the new place's tone in prose; a **fight left hanging** (`combat_left_hanging`) → combat reads active but no living hostile remains; `end_combat(resolution='…')` now so initiative/HP reset and (xp mode) the foes' XP auto-awards; **unawarded XP** (`xp_unawarded`) → a defeated monster still carries `xp_value` out of combat; `end_combat` (it auto-awards) or `award_xp(character_id, amount, reason)` so the kill counts; a **frozen clock** (`clock_dm_frozen`) → the party has moved on but the clock still reads day 1, morning; `advance_time(phases=N)` as scenes pass, `long_rest` at a safe stop, or `downtime` for a longer skip (the clock feeds companion regard / camp / every day-gated system); a **late-unresolved quest** (`quest_unresolved_late`) → 8+ beats in and not one quest objective has been recorded done; `complete_objective(quest_id, objective)` as the party clears a step, or `complete_quest(quest_id, evolves_to='…')` when a thread resolves. **Companions are GAUGED, not just narrated; quests RESOLVE and EVOLVE, not just get mentioned; the party TRAVELS, fights CLOSE, XP LANDS, and the clock MOVES** — a companion whose `attitude_value` never leaves 0, a quest that ends a multi-location arc still `active`, a party that never left the opening room, and a clock frozen at day-1 morning are the exact failures this list exists to stop. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the missing quest_unresolved_late guard conditions. Confidence: 89%.
This checklist says the cue fires when "not one quest objective has been recorded done," but the engine also requires zero completed quests and suppresses the cue if quest_resolvable or quest_stalled is already present. Without that, the DM contract here drifts from the actual precedence rules and encourages redundant handling.
♻️ Proposed wording fix
- a **late-unresolved quest** (`quest_unresolved_late`) → 8+ beats in and not one quest objective has been recorded done; `complete_objective(quest_id, objective)` as the party clears a step, or `complete_quest(quest_id, evolves_to='…')` when a thread resolves.
+ a **late-unresolved quest** (`quest_unresolved_late`) → 8+ beats in, no quest has been completed, and not one quest objective has been recorded done (and only when no `quest_resolvable` / `quest_stalled` cue already applies); `complete_objective(quest_id, objective)` as the party clears a step, or `complete_quest(quest_id, evolves_to='…')` when a thread resolves.🤖 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 `@skills/dungeon-master/SKILL.md` at 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.
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.
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.