fix(combat): #279 allegiance comes from the scene, not the participant bucket

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 = {}

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):

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")

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,

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'])