From 94259e4076cb23968311e971e3a5a79a74f0f5c7 Mon Sep 17 00:00:00 2001 From: MoonlightByte <70819495+MoonlightByte@users.noreply.github.com> Date: Wed, 9 Sep 2026 10:44:58 -0700 Subject: [PATCH 1/2] fix(combat): #279 allegiance comes from the scene, not the participant bucket Bandit Captain Gorvek fought for the party in his own set-piece. Root cause is not a mis-typed creature: it is an authority boundary in the wrong place. createEncounter buckets participants as `npcs` (has a character sheet) or `monsters` (is a stat block). combat_builder maps that bucket to `type`, and ensure_combatant_ids maps `type` to `faction` ("npc" -> "party"). Every named villain with a sheet therefore became a party member. resolver.validate_intent then made that invented table a hard targeting gate, so the boss was rejected for "cannot attack ally cmb-player-eirik-vane-1" three times and handed a legalTargets whitelist containing only his own men. He took it, and the narrator truthfully reported him killing them. Meanwhile the model had already authored the correct answer. sceneFacts.relations said Gorvek was hostile to the player, reconciled to exact combatant IDs and persisted by combat_builder. Nothing in the repository read it, and the T096 payload never showed it back to the actor agent -- which was shown the invented faction table instead. The agent was punished for not knowing a fact that was deliberately withheld from it. `faction` is assigned once at creation and never mutated anywhere, so the gate also could not express any mid-combat change of side: charm, domination, surrender or betrayal all invert under it. This change: - sends sceneFacts.relations and objectives in the T096 payload for typed encounters, and tells the agent that sceneFacts wins over type/faction; - deletes the same-faction "cannot attack ally" rejection; - makes the remaining legalTargets hints truthful (living roster minus self). The eight factual gates are untouched: unknown actor, actor cannot act, stale stateVersion, outside the turn window, unknown target, target already down, ability not on sheet, no ammunition. Code keeps identity, canonical state, ordering and arithmetic; who counts as a friend goes back to the agent. Absence-safe. Legacy encounters never reach this resolver (combat_orchestrator gates on pipelineMode == "agentic"), and pre_typed encounters carry no sceneFacts, so the payload is unchanged for them. Evidence, real captured encounter TW05-E1 plus live OpenAI T096 calls: - deterministic replay of the incident, 24/24 checks, including all five observed ally-rejections now allowed and all eight factual gates still firing; - live A/B on the same encounter and turn: baseline 3/4 samples had the boss attacking his own men (matching the observed 3/6); with this change 0/4; - charm and turncoat scenarios, impossible under the old gate, both resolve correctly; with relations and objectives both emptied the agent still does not fall back to type/faction. Known and NOT fixed here: is_hostile() still derives from type/faction, so the boss is excluded from all_hostiles_resolved and a set-piece can complete with him alive. Pre-existing, unchanged by this commit, reported on the issue. Refs #279. Doctrine: #193 NEQ-COMBAT-02, AP-6, NEQ-CORE-06, leanness tests 1-3. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CKDbsAQBJqRUp8QjeqtfvM --- core/ai/combat_agent.py | 31 ++++++++++++++++++++++++++++++- core/combat/resolver.py | 31 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/core/ai/combat_agent.py b/core/ai/combat_agent.py index 12061db6..f6ab594f 100644 --- a/core/ai/combat_agent.py +++ b/core/ai/combat_agent.py @@ -26,7 +26,11 @@ load_srd_reference_index, normalize_rule_name, ) -from core.managers.combat_state import combatant_by_id, resolve_creature_controller +from core.managers.combat_state import ( + combat_provenance, + combatant_by_id, + resolve_creature_controller, +) from utils.capture.multi_model_capture import capture_and_fanout, register_callsite from utils.capture.live_provider_call import LiveProviderSuperseded from utils.character_sheet_contract import extract_json_object @@ -289,6 +293,18 @@ def _intent_system_prompt(): keys. If no suitable listed spell remains, choose a listed weapon/action or a defensive action instead of guessing spell mechanics. encounterContext and ruleReferences are authoritative scene/rule guidance when present. + +sceneFacts, when present, is the authoritative allegiance and motive record for +this encounter. sceneFacts.relations gives directed subject/object dispositions +and sceneFacts.objectives gives who is trying to do what to whom, both keyed by +exact combatantId. Use them to decide who each actor fights. A creature's 'type' +and 'faction' fields describe how that combatant is stored and controlled, NOT +which side it is on: a named villain with a character sheet is stored as +type 'npc', and a charmed, dominated, surrendered or turncoat combatant keeps +whatever type and faction it was created with. When type/faction and sceneFacts +disagree about a side, sceneFacts wins. When sceneFacts is silent about a pair, +use the scene as narrated in encounterContext and the actor's own objectives; +do not fall back to type or faction to infer an alliance. An adjudicated intent may contain: - description: mechanical ruling - save: {type, dc, halfOnSave} when targets roll a save @@ -395,6 +411,19 @@ def request_intent_batch( else spell_references or {} ), } + # Issue #279: the scene relations and objectives are authored by the model at + # createEncounter and reconciled to exact combatant IDs, but were persisted and + # never shown back to the actor agent. Without them the only allegiance signal in + # this payload was the creature `type`/`faction` pair, which is derived from the + # createEncounter participant bucket and says "npc" for every named villain. + # Absence-safe: legacy and pre_typed encounters carry no sceneFacts and are + # byte-identical to before. + if combat_provenance(encounter) == "typed": + scene = encounter.get("sceneFacts") or {} + payload["sceneFacts"] = { + "relations": scene.get("relations", []), + "objectives": scene.get("objectives", []), + } pending_ids = list(pending_turn.get("actorIds", [])) if isinstance(npc_voice_intents, Mapping): selected_voice = {} diff --git a/core/combat/resolver.py b/core/combat/resolver.py index 80419cbe..f49a9c1c 100644 --- a/core/combat/resolver.py +++ b/core/combat/resolver.py @@ -166,11 +166,21 @@ def _living_target_ids(encounter): def _living_opponent_ids(encounter, actor): + """Living combatants this actor may target. + + Issue #279: this used to subtract everyone sharing the actor's `faction`. + `faction` is assigned once at encounter creation from the createEncounter + participant bucket (combat_state.ensure_combatant_ids) and is never mutated + afterwards, so it cannot express a named villain stored as an `npc`, nor any + mid-combat change of side (charm, domination, surrender, betrayal). Legality + of a target is now identity plus canonical state only; who is worth attacking + is a semantic judgment the actor agent makes from sceneFacts. The actor is + excluded because these lists are offered as attack targets in a rejection. + """ return [ target_id for target_id in _living_target_ids(encounter) - if (combatant_by_id(encounter, target_id) or {}).get("faction") - != actor.get("faction") + if target_id != actor.get("combatantId") ] @@ -219,12 +229,7 @@ def validate_intent(encounter, characters, intent, strict=None): if strict and not target_id: return False, Rejection( reason="attack requires a targetId", - legalTargets=[ - target - for target in _living_target_ids(encounter) - if (combatant_by_id(encounter, target) or {}).get("faction") - != actor.get("faction") - ], + legalTargets=_living_opponent_ids(encounter, actor), retryable=True, ) if target_id is not None and combatant_by_id(encounter, target_id) is None: @@ -236,11 +241,11 @@ def validate_intent(encounter, characters, intent, strict=None): return False, Rejection( reason="target %s is already down" % target_id, legalTargets=_living_opponent_ids(encounter, actor), retryable=True) - if strict and target is not None and target.get("faction") == actor.get("faction"): - return False, Rejection( - reason="%s cannot attack ally %s" % (actor_id, target_id), - legalTargets=_living_opponent_ids(encounter, actor), - retryable=True) + # Issue #279: the same-faction "cannot attack ally" rejection lived here. + # It read a table that code invents from the participant bucket, so it + # blocked a named villain from fighting the party and handed him his own + # men as the only legal targets. Allegiance is semantic and mutable; + # code owns identity, state and arithmetic, not who counts as a friend. if strict: entry = _find_action(sheet, intent.get("ability")) if entry is None or not is_executable_attack(entry): From 3219dddcdf54bfffc12acaba02f34dc04e26650d Mon Sep 17 00:00:00 2001 From: MoonlightByte <70819495+MoonlightByte@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:59:46 -0700 Subject: [PATCH 2/2] fix(combat): #279 correct the side at creation, and count XP by hostility Follow-on to 8bb9dc7b, which fixed targeting but left three downstream readers of the same bucket-derived guess. combat_builder seeds `faction` via ensure_combatant_ids BEFORE the authored scene is reconciled, then stores the scene and never applies it. The boss was therefore still seeded "party" even though sceneFacts said he was hostile to the player, so is_hostile() excluded him from all_hostiles_resolved and a set-piece could auto-complete at combat_manager.py:4869 with the villain untouched at full HP; voice_context omitted him from the threat list shown to companions; and utils/xp.py, keyed on `type == 'enemy'`, awarded nothing for killing him. apply_scene_declared_sides() promotes a participant to "hostile" only when the scene explicitly declares it hostile toward somebody on the party side. It is deliberately one-directional: it can promote, never demote. Demoting an adversary would end a fight early; promoting an ally would make all_hostiles_resolved unsatisfiable and hang the encounter, so the only move available is the one the model stated outright. The player is never promoted. Dispositions are compared as EXACT values against a small vocabulary, never scored or pattern-matched, and an unrecognised disposition is a no-op. utils/xp.py now counts defeated hostiles rather than defeated buckets. A hostile carrying a character sheet has no stat block, so its CR cannot be resolved; that awards nothing and warns, rather than inventing a CR from level or HP. The guard change alone would have made creature['monsterType'] raise KeyError inside the completion path, so that access is now safe. Absence-safe throughout: no relations, an unrecognised disposition, a legacy or pre_typed encounter, or malformed relations all leave the roster untouched, and encounters already on disk are never migrated. Evidence, real captured encounter TW05-E1: - 61/61 deterministic checks across three harnesses (24 targeting, 28 sides, 9 XP), including one-directional safety, idempotence, dangling-ID and garbage relation handling, and legacy/pre_typed/unknown-contract no-ops; - the fight no longer auto-completes with the boss standing, and does complete once he is down; - mook XP identical before and after (150, unchanged breakdown); the boss is now counted as a defeated hostile; a hostile with a resolvable CR awards 200; - live OpenAI T096, 4 further samples with both fixes: 0/4 friendly fire (running total 0/8 with fixes, against 3/4 on unmodified origin/main); - charm and turncoat scenarios still resolve correctly, and with relations and objectives both emptied the agent still does not fall back to type/faction. KNOWN DATA GAP, not fixed here: The_Thornwood_Watch authored Bandit Captain Gorvek with no challengeRating (level 1, 12 HP), so defeating him still awards 0 XP. That is a module-generation gap, not a combat gap -- char_schema.json already allows challengeRating. Reported on the issue for separate triage. Also unchanged: all_party_resolved at combat_state.py:314 still counts the boss as party via `type in ("player","npc")`, which faction cannot override. Its observed effect in this incident was nil and changing that disjunct has legacy blast radius; left for the #191 epic. Refs #279. Doctrine: #193 NEQ-COMBAT-02, AP-6, AP-7 (values not prose), NEQ-CORE-01 (no hang), leanness tests 1-3. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CKDbsAQBJqRUp8QjeqtfvM --- core/combat/scene.py | 75 +++++++++++++++++++++++++++++++ core/generators/combat_builder.py | 6 ++- utils/xp.py | 56 ++++++++++++++++------- 3 files changed, 119 insertions(+), 18 deletions(-) diff --git a/core/combat/scene.py b/core/combat/scene.py index 371ff133..6fbd107b 100644 --- a/core/combat/scene.py +++ b/core/combat/scene.py @@ -16,10 +16,85 @@ OBJECTIVE_STATUS_VALUES = {"active", "satisfied", "failed", "abandoned"} +# Dispositions that unambiguously declare opposition to the party. These are +# compared as EXACT values against the typed relation field, never matched +# against prose or scored against a vocabulary: an unrecognised disposition is +# deliberately a no-op that leaves the existing side exactly as it was. The +# field is free-form by contract, so this reads only what it can be certain of. +ADVERSARIAL_DISPOSITIONS = frozenset({"hostile", "enemy", "adversarial", "opposed"}) + + class SceneReconciliationError(ValueError): """A scene proposal cannot be reconciled to exact canonical identities.""" +def apply_scene_declared_sides(encounter): + """Correct `faction` from the sides the model actually authored (issue #279). + + `ensure_combatant_ids` seeds `faction` from `type`, which is itself derived + from the createEncounter participant bucket: anything carrying a character + sheet lands in `npcs` and is seeded "party". That makes every named villain + a party member, which is how Bandit Captain Gorvek came to fight for the + party in his own set-piece. + + The scene manifest already carries the answer. This promotes a participant + to "hostile" only when the scene explicitly says it is hostile toward + somebody on the party side. + + Deliberately one-directional. It can promote a combatant to hostile; it can + never demote one to party. Wrongly demoting an adversary would end a fight + early, and wrongly promoting an ally would make `all_hostiles_resolved` + unsatisfiable and hang the encounter, so the only move available here is the + one the model stated outright. The player character is never promoted. + + Absence-safe and creation-time only: no relations, an unrecognised + disposition, or a non-typed encounter all leave the roster untouched, and + encounters already on disk are never migrated. + + Returns the set of combatant IDs whose faction this changed. + """ + if not isinstance(encounter, dict): + return set() + creatures = encounter.get("creatures") + if not isinstance(creatures, list): + return set() + scene = encounter.get("sceneFacts") + if not isinstance(scene, dict) or scene.get("contractVersion") != CONTRACT_VERSION: + return set() + + by_id = { + c.get("combatantId"): c + for c in creatures + if isinstance(c, dict) and c.get("combatantId") + } + player_ids = { + cid for cid, c in by_id.items() if c.get("type") == "player" + } + party_ids = player_ids | { + cid for cid, c in by_id.items() if c.get("faction") == "party" + } + + changed = set() + for relation in scene.get("relations") or []: + if not isinstance(relation, dict): + continue + disposition = relation.get("disposition") + if not isinstance(disposition, str): + continue + if disposition.strip().lower() not in ADVERSARIAL_DISPOSITIONS: + continue + subject_id = relation.get("subjectId") + if relation.get("objectId") not in party_ids: + continue + if subject_id in player_ids or subject_id not in by_id: + continue + subject = by_id[subject_id] + if subject.get("faction") != "hostile": + subject["faction"] = "hostile" + changed.add(subject_id) + return changed + + def _require_object(value, label): if not isinstance(value, dict): raise SceneReconciliationError(f"{label} must be an object") diff --git a/core/generators/combat_builder.py b/core/generators/combat_builder.py index d9494f45..31a96d52 100644 --- a/core/generators/combat_builder.py +++ b/core/generators/combat_builder.py @@ -570,7 +570,7 @@ def generate_encounter(encounter_data): # Every genuinely new encounter is typed-agentic. Persisted provenance, # never a hidden runtime selector, keeps old encounters on their existing # compatibility route. - from core.combat.scene import reconcile_scene_manifest + from core.combat.scene import apply_scene_declared_sides, reconcile_scene_manifest from core.managers.combat_state import ensure_combatant_ids, ensure_combat_state ensure_combatant_ids(encounter) @@ -586,6 +586,10 @@ def generate_encounter(encounter_data): ] reconciled = reconcile_scene_manifest(encounter_data, canonical_snapshot) encounter["sceneFacts"] = reconciled["sceneFacts"] + # Issue #279: ensure_combatant_ids above seeded `faction` from the participant + # bucket before the authored scene was available. Now that it is, let the scene + # correct the sides it explicitly declared. + apply_scene_declared_sides(encounter) ensure_combat_state( encounter, new_encounter=True, diff --git a/utils/xp.py b/utils/xp.py index 8ef05bfc..6cf0dcb0 100644 --- a/utils/xp.py +++ b/utils/xp.py @@ -6,6 +6,7 @@ import json import os from .module_path_manager import ModulePathManager +from core.managers.combat_state import is_hostile # CR to XP mapping (updated to include fractional CRs) cr_to_xp = { @@ -42,23 +43,44 @@ def calculate_xp(encounter, party_tracker): path_manager = ModulePathManager(current_module) for creature in encounter['creatures']: - if creature['type'] == 'enemy' and is_defeated(creature['status']): - defeated_count += 1 - monster_type = creature['monsterType'].lower() - if monster_type not in monster_cache: - monster_file = path_manager.get_monster_path(monster_type) - if os.path.exists(monster_file): - monster = load_json_file(monster_file) - monster_cache[monster_type] = monster - else: - print(f"Warning: Monster file {monster_file} not found.") - continue - - monster = monster_cache[monster_type] - cr = monster['challengeRating'] - xp = get_xp_for_cr(cr) - total_xp += xp - xp_breakdown.append((monster['name'], cr, xp)) + # Issue #279: this counted `type == 'enemy'`, which is derived from the + # createEncounter participant bucket, not from which side a combatant is + # on. A named villain carrying a character sheet is stored as type 'npc', + # so defeating the module's boss awarded nothing and he was not even + # counted. Hostility is the correct question, and is_hostile() reads the + # side the scene declared. + if not is_hostile(creature) or not is_defeated(creature.get('status', '')): + continue + defeated_count += 1 + + # Encounter monsters carry a monsterType pointing at a stat block whose + # challengeRating prices the kill. A sheet-based hostile has no stat + # block; its value must come from an authored challengeRating, and there + # is none to invent here. Award nothing rather than guess a CR, and say + # so loudly enough to be diagnosable. + monster_type = creature.get('monsterType') + if not monster_type: + print( + f"Warning: no challengeRating available for defeated hostile " + f"{creature.get('name')!r} (no monsterType); no XP awarded for it." + ) + continue + + monster_type = monster_type.lower() + if monster_type not in monster_cache: + monster_file = path_manager.get_monster_path(monster_type) + if os.path.exists(monster_file): + monster = load_json_file(monster_file) + monster_cache[monster_type] = monster + else: + print(f"Warning: Monster file {monster_file} not found.") + continue + + monster = monster_cache[monster_type] + cr = monster['challengeRating'] + xp = get_xp_for_cr(cr) + total_xp += xp + xp_breakdown.append((monster['name'], cr, xp)) # Count player characters and party NPCs num_players = len(party_tracker['partyMembers'])