diff --git a/README.md b/README.md index 2d52153..a176d8c 100644 --- a/README.md +++ b/README.md @@ -1951,8 +1951,11 @@ Règles modélisées : retour : la politique peut les muter sans affecter le combat ; l'objet est un dataclass figé au premier niveau, les dictionnaires internes sont des copies mutables, non deep-freezés). Une politique LLM - se branche via `CombatEngine(..., llm_policy=...)` (`LLMPolicy(observation, - state) -> tuple[Action, ...]`). Schéma enrichi (E1-E5) : + se branche via `CombatEngine(..., llm_policy=...)` (`LLMPolicy(observation) + -> tuple[Action, ...]` : la politique ne reçoit QUE le snapshot, jamais + l'état vivant ; un callable hérité à deux arguments `(observation, state)` + reste accepté via l'adaptateur de compatibilité D1, signature inspectée + une fois à la construction). Schéma enrichi (E1-E5) : - Par combattant : id, side, pos, occupied_cells, hp/hp_max, active/dead, dying/stabilized, conditions (noms + rounds_left), resources_left (+ `spell_slot_{level}`), weapon_broken/weapon_destroyed/weapon_stolen/weapon_dropped, carried_items @@ -2050,7 +2053,10 @@ Règles modélisées : charge_reason (str | None — None si `can_charge` est vrai ; sinon la raison de `ChargeResult` verbatim (`no_line_of_effect` / `no_charge_path`) ou `no_weapon` si aucune arme de mêlée utilisable). - - Top-level : round, acting_combatant, combatants, pairs, terrain, elevation, + - Top-level : schema_version (P0.1 : version épinglée du schéma d'observation, + constante `OBSERVATION_SCHEMA_VERSION` = 1 ; incrémentée seulement si une + clé est retirée ou change de sens), round, acting_combatant, combatants, + pairs, terrain, elevation, threatened (grille bool des cases menacées au contact par les ennemis — AoO), ranged_threatened (U6d : grille bool des cases sous tir ennemi à distance — champ de tir des armes à distance utilisables des ennemis diff --git a/src/pf1e_simulator/combat.py b/src/pf1e_simulator/combat.py index 6c71621..c226a15 100644 --- a/src/pf1e_simulator/combat.py +++ b/src/pf1e_simulator/combat.py @@ -121,6 +121,7 @@ Phase 0 documented deviations from PF1e (conventions): from __future__ import annotations +import inspect from collections.abc import Callable, Iterable from dataclasses import asdict, dataclass, field, replace from itertools import pairwise @@ -1259,6 +1260,12 @@ class ChargeResult: reason: str | None +# Observation schema version (P0.1, docs/phase-2-llm-architecture.md section +# 10). Bump only when a top-level key is removed or its meaning changes; +# additive keys keep the version. +OBSERVATION_SCHEMA_VERSION = 1 + + @dataclass(frozen=True) class Observation: """Serializable snapshot of combat state for a policy/LLM. @@ -1269,7 +1276,9 @@ class Observation: collections are copies (tuples/dicts), never read back by the engine, so a policy may mutate them freely without touching combat; the snapshot is JSON-serializable (the top level is a frozen dataclass; the inner dicts are - plain mutable copies, not deep-frozen). + plain mutable copies, not deep-frozen). ``to_dict()`` carries + ``schema_version`` (``OBSERVATION_SCHEMA_VERSION``) as its first key so a + consumer can detect breaking schema changes. ``combatants`` holds one dict per combatant in ``self._states`` order; ``pairs`` holds one dict per (acting combatant x each other combatant) with @@ -1310,6 +1319,7 @@ class Observation: def to_dict(self) -> dict[str, object]: """Return a JSON-serializable dict with the pinned top-level keys.""" return { + "schema_version": OBSERVATION_SCHEMA_VERSION, "round": self.round, "acting_combatant": self.acting_combatant, "combatants": self.combatants, @@ -1613,7 +1623,7 @@ class CombatEngine: *, round_cap: int = 100, policy: Policy | None = None, - llm_policy: LLMPolicy | None = None, + llm_policy: LLMPolicy | LegacyLLMPolicy | None = None, spell_registry: dict[str, SpellSpec] | None = None, reaction_policy: ReactionPolicy | None = None, monsters_die_at_zero: bool = False, @@ -1685,18 +1695,24 @@ class CombatEngine: if self._monsters_die_at_zero and not _is_pc_combatant(state.combatant): state.dies_at_zero = True - def _llm_adapter(self, llm_policy: LLMPolicy) -> Policy: - """Wrap an ``LLMPolicy`` into a ``Policy``. + def _llm_adapter(self, llm_policy: LLMPolicy | LegacyLLMPolicy) -> Policy: + """Wrap an ``LLMPolicy`` (or a pre-D1 two-argument callable) into a ``Policy``. At the point of invocation (``actions = self._policy(self, state)`` in ``_take_turn``), the Observation is built for the current state and - delegated to ``llm_policy(observation, state)``. The adapter consumes - no RNG and mutates no state. + delegated. New-style policies get ``llm_policy(observation)`` only; + a legacy ``llm_policy(observation, state)`` callable is detected ONCE + here (signature inspection, not per turn) and keeps receiving the live + state. The adapter consumes no RNG and mutates no state. """ + two_arg = _accepts_state(llm_policy) + call: Callable[..., tuple[Action, ...]] = llm_policy def adapter(_engine: CombatEngine, state: CombatantState) -> tuple[Action, ...]: observation = self._build_observation(state) - return llm_policy(observation, state) + if two_arg: + return call(observation, state) + return call(observation) return adapter @@ -10572,7 +10588,36 @@ def _ready_payload(action: Action) -> Action: Policy = Callable[[CombatEngine, CombatantState], tuple[Action, ...]] -LLMPolicy = Callable[[Observation, CombatantState], tuple[Action, ...]] +# D1 (docs/phase-2-llm-architecture.md section 13): the LLM policy entry is +# observation-only -- the snapshot is the policy's sole information source and +# the live CombatantState is never handed to a new-style policy. Pre-D1 +# two-argument callables remain accepted via CombatEngine._llm_adapter. +LLMPolicy = Callable[[Observation], tuple[Action, ...]] + +LegacyLLMPolicy = Callable[[Observation, CombatantState], tuple[Action, ...]] + +_LEGACY_LLM_POLICY_ARITY = 2 + + +def _accepts_state(llm_policy: LLMPolicy | LegacyLLMPolicy) -> bool: + """True when ``llm_policy`` is a pre-D1 callable taking (observation, state). + + Counts REQUIRED positional parameters (a defaulted second parameter is a + new-style policy that tolerates the 1-argument call). Callables whose + signature cannot be inspected default to the new-style 1-argument call. + """ + try: + parameters = list(inspect.signature(llm_policy).parameters.values()) + except (TypeError, ValueError): + return False + required_positional = [ + p + for p in parameters + if p.default is inspect.Parameter.empty + and p.kind + in (inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) + ] + return len(required_positional) >= _LEGACY_LLM_POLICY_ARITY ReactionPolicy = Callable[ [CombatEngine, CombatantState, ReactionEvent], Action | None diff --git a/tests/test_action_feedback.py b/tests/test_action_feedback.py index 852d80d..12240b6 100644 --- a/tests/test_action_feedback.py +++ b/tests/test_action_feedback.py @@ -229,7 +229,7 @@ def test_results_visible_in_next_turn_observation() -> None: # the second invocation must already see the first turn's rejection. seen: list[Observation] = [] - def llm_policy(obs: Observation, state: CombatantState) -> tuple[Action, ...]: + def llm_policy(obs: Observation) -> tuple[Action, ...]: seen.append(obs) if len(seen) == 1: return (Action(kind="move"),) diff --git a/tests/test_llm_engine_capabilities.py b/tests/test_llm_engine_capabilities.py index 4be6a57..e4bf7f5 100644 --- a/tests/test_llm_engine_capabilities.py +++ b/tests/test_llm_engine_capabilities.py @@ -1,7 +1,7 @@ """Package U8 — final validation: an observation-only LLM policy drives the Phase-2 capabilities (U1-U7b) end-to-end through the real engine. -Each test wires a ``llm_policy`` (``LLMPolicy(observation, state)``) that +Each test wires a ``llm_policy`` (``LLMPolicy(observation)``) that reads ONLY the frozen Observation snapshot — never ``engine.*`` — and emits one newly-unblocked tactical decision per capability: ``ready`` (U4), ``delay`` (U3), ``total_defense`` (U5a), ``move`` with an explicit @@ -121,7 +121,7 @@ def _counting(decide: Decide) -> LLMPolicy: """LLMPolicy calling ``decide(id, call_no, obs)`` per turn (obs only).""" calls: dict[str, int] = {} - def policy(obs: Observation, _state: CombatantState) -> tuple[Action, ...]: + def policy(obs: Observation) -> tuple[Action, ...]: cid = str(_me(obs)["id"]) n = calls.get(cid, 0) calls[cid] = n + 1 diff --git a/tests/test_observation.py b/tests/test_observation.py index f2a319a..5604cba 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -9,9 +9,12 @@ Covers the pinned schema: - Terrain/elevation layers. - Frozen copies: mutating live state after construction does not change the already-built Observation. -- ``to_dict()`` is JSON-serializable and exposes no ``engine.*`` callables. +- ``to_dict()`` is JSON-serializable, carries the pinned ``schema_version`` + key, and exposes no ``engine.*`` callables. - The ``llm_policy`` adapter builds the Observation at the point of policy - invocation and delegates to ``llm_policy(observation, state)``. + invocation and delegates to ``llm_policy(observation)`` (D1: observation + only); a legacy ``llm_policy(observation, state)`` callable keeps working + through the compatibility adapter. """ from __future__ import annotations @@ -20,7 +23,13 @@ import json import pytest -from pf1e_simulator.combat import Action, CombatantState, CombatEngine, Observation +from pf1e_simulator.combat import ( + OBSERVATION_SCHEMA_VERSION, + Action, + CombatantState, + CombatEngine, + Observation, +) from pf1e_simulator.conditions import SHAKEN from pf1e_simulator.dice import parse_dice from pf1e_simulator.effects import ActiveCondition @@ -441,11 +450,11 @@ def test_observation_frozen_copies() -> None: def test_llm_policy_wrapper_e2e() -> None: - """Given: a CombatEngine with an llm_policy that reads ONLY the observation + """Given: a CombatEngine with an observation-only llm_policy (D1 signature) When: the battle runs Then: the policy's decisions are honored and the transcript is coherent.""" - def llm_policy(obs: Observation, state: CombatantState) -> tuple: + def llm_policy(obs: Observation) -> tuple: # Read only the observation: full_attack if the target is adjacent, # otherwise move toward it. target_id = obs.combatants[obs.pairs[0]["target_index"]]["id"] @@ -470,6 +479,65 @@ def test_llm_policy_wrapper_e2e() -> None: assert any("move" in line for line in result.transcript) +def test_legacy_two_arg_llm_policy_still_works() -> None: + """Given: a pre-D1 llm_policy taking (observation, state) + When: the battle runs through the compatibility adapter + Then: the policy receives BOTH arguments and its decisions are honored.""" + seen_states: list[CombatantState] = [] + + def legacy_policy(obs: Observation, state: CombatantState) -> tuple: + seen_states.append(state) + assert state.combatant.id == obs.combatants[obs.acting_combatant]["id"] + target_id = obs.combatants[obs.pairs[0]["target_index"]]["id"] + return (Action(kind="move", target_id=target_id),) + + hero = make_combatant("hero") + foe = make_combatant("foe") + hero_state = make_state(hero, "players", (0, 0)) + foe_state = make_state(foe, "monsters", (0, 3)) + engine = CombatEngine( + ScriptedRng([10, 9, 8, 15, 3, 5]), + make_grid(), + [hero_state, foe_state], + round_cap=1, + llm_policy=legacy_policy, + ) + + result = engine.run() + assert seen_states, "legacy policy was never invoked" + assert any("move" in line for line in result.transcript) + + +def test_observation_schema_version_pinned() -> None: + """Given: a built Observation + When: to_dict() is called + Then: the top-level key set is exactly the pinned keys and + schema_version == OBSERVATION_SCHEMA_VERSION, JSON-serializable.""" + hero = make_combatant("hero") + foe = make_combatant("foe") + hero_state = make_state(hero, "players", (0, 0)) + foe_state = make_state(foe, "monsters", (0, 3)) + engine = make_engine([hero_state, foe_state]) + + data = engine._build_observation(hero_state).to_dict() + assert data["schema_version"] == OBSERVATION_SCHEMA_VERSION + assert set(data) == { + "schema_version", + "round", + "acting_combatant", + "combatants", + "pairs", + "terrain", + "elevation", + "threatened", + "ranged_threatened", + "terrain_semantics", + "condition_definitions", + "initiative_order", + } + json.dumps(data) # must not raise + + # --------------------------------------------------------------------------- # 12. policy + llm_policy → ValueError # --------------------------------------------------------------------------- diff --git a/tests/test_observation_enriched.py b/tests/test_observation_enriched.py index 8d54dfb..0ee03b2 100644 --- a/tests/test_observation_enriched.py +++ b/tests/test_observation_enriched.py @@ -1051,7 +1051,8 @@ def test_observation_enriched_terrain_semantics_no_callable_no_share() -> None: def test_observation_enriched_e5_existing_keys_preserved() -> None: """Given: an Observation built for the L5 fields When: the top-level dict is inspected - Then: all pre-existing top-level keys remain (round, acting_combatant, + Then: all pre-existing top-level keys remain (schema_version, round, + acting_combatant, combatants, pairs, terrain, elevation) and the new keys (threatened, ranged_threatened, terrain_semantics, condition_definitions, initiative_order) are added — exact key set.""" @@ -1063,6 +1064,7 @@ def test_observation_enriched_e5_existing_keys_preserved() -> None: obs = _snapshot(engine, hero_state) assert set(obs) == { + "schema_version", "round", "acting_combatant", "combatants", @@ -1138,7 +1140,7 @@ def _best_usable_attack(obs: Observation) -> dict[str, object] | None: return best -def _enriched_llm_policy(obs: Observation, state: CombatantState) -> tuple[Action, ...]: +def _enriched_llm_policy(obs: Observation) -> tuple[Action, ...]: """Read ONLY the observation — never state.combatant.* nor engine.*.""" cast = _castable_spell_action(obs) if cast is not None: