feat(phase2): few-shot prompts + bounded repair loop (P5.2)

tests/test_phase2_nl_policy.py

@@ -3,17 +3,28 @@
 from __future__ import annotations
 
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 import pytest
 
-from pf1e_simulator.client import FakeLLMClient, ResponseCache, ScriptExhaustedError
+from pf1e_simulator.client import (
+    ClientParams,
+    Completion,
+    FakeLLMClient,
+    ResponseCache,
+    ScriptExhaustedError,
+)
 from pf1e_simulator.combat import Action, Observation
 from pf1e_simulator.nl_policy import (
     NLPolicy,
     NLPolicyError,
+    NLPolicyStats,
     fallback_default,
     parse_actions,
 )
+
+if TYPE_CHECKING:
+    from collections.abc import Sequence
 from pf1e_simulator.prompt import PromptPack
 from pf1e_simulator.prompt_builder import PromptBuilder
 
@@ -51,8 +62,26 @@ def _make_client(script: list[str], *, builder: PromptBuilder, **kwargs) -> Fake
     )
 
 
-def _make_policy(client: FakeLLMClient, builder: PromptBuilder) -> NLPolicy:
-    return NLPolicy(client=client, builder=builder)
+def _make_policy(
+    client: FakeLLMClient, builder: PromptBuilder, *, max_repairs: int = 1
+) -> NLPolicy:
+    return NLPolicy(client=client, builder=builder, max_repairs=max_repairs)
+
+
+class _RecordingClient(FakeLLMClient):
+    """Scripted client that records every prompt sent to the model.
+
+    The repair-loop tests assert on the accumulated error trail, so the
+    prompts themselves must be observable.
+    """
+
+    def __init__(self, script: Sequence[str], *, prompt_version: str) -> None:
+        super().__init__(script=script, prompt_version=prompt_version)
+        self.prompts: list[str] = []
+
+    def _call_once(self, prompt: str, *, params: ClientParams) -> Completion:
+        self.prompts.append(prompt)
+        return super()._call_once(prompt, params=params)
 
 
 @pytest.fixture
@@ -278,3 +307,108 @@ def test_nl_policy_shared_cache_hit() -> None:
     # Second call was a cache hit (no ScriptExhaustedError)
     assert client2.stats.cache_hits == 1
     assert client1.stats.cache_misses == 1
+
+
+# ---- P5.2 repair loop --------------------------------------------------------
+
+def test_nl_policy_stats_defaults_to_zero() -> None:
+    """A fresh NLPolicy reports zero calls until the first decision."""
+    builder = _builder()
+    client = _make_client(['[{"kind": "wait"}]'], builder=builder)
+    policy = _make_policy(client, builder)
+    assert policy.stats == NLPolicyStats()
+    assert policy.stats.calls == 0
+
+
+def test_nl_policy_max_repairs_zero_falls_back_immediately() -> None:
+    """max_repairs=0 disables repair: first bad reply triggers fallback."""
+    builder = _builder()
+    client = _make_client(["bad one"], builder=builder)
+    policy = _make_policy(client, builder, max_repairs=0)
+
+    actions = policy(_obs())
+    assert actions == (Action(kind="total_defense"),)
+    assert policy.stats.calls == 1
+    assert policy.stats.repairs_attempted == 0
+    assert policy.stats.repairs_succeeded == 0
+    assert policy.stats.fallbacks == 1
+
+
+def test_nl_policy_max_repairs_two_repairs_then_succeeds() -> None:
+    """Two invalid replies are repaired, then the valid reply is parsed."""
+    builder = _builder()
+    script = ["bad one", "bad two", '[{"kind": "move", "destination": [2, 3]}]']
+    client = _make_client(script, builder=builder)
+    policy = _make_policy(client, builder, max_repairs=2)
+
+    actions = policy(_obs())
+    assert actions == (Action(kind="move", destination=(2, 3)),)
+    assert policy.stats.calls == 3
+    assert policy.stats.repairs_attempted == 2
+    assert policy.stats.repairs_succeeded == 1
+    assert policy.stats.fallbacks == 0
+
+
+def test_nl_policy_max_repairs_two_then_falls_back() -> None:
+    """Past max_repairs the policy stops repairing and falls back."""
+    builder = _builder()
+    script = ["bad one", "bad two", "bad three"]
+    client = _make_client(script, builder=builder)
+    policy = _make_policy(client, builder, max_repairs=2)
+
+    actions = policy(_obs())
+    assert actions == (Action(kind="total_defense"),)
+    assert policy.stats.calls == 3
+    assert policy.stats.repairs_attempted == 2
+    assert policy.stats.repairs_succeeded == 0
+    assert policy.stats.fallbacks == 1
+
+
+def test_nl_policy_repair_trail_accumulates() -> None:
+    """Each retry prompt carries the full accumulated error trail."""
+    builder = _builder()
+    script = ["bad one", "bad two", '[{"kind": "wait"}]']
+    client = _RecordingClient(
+        script=script,
+        prompt_version=builder.prompt_version("nl_default"),
+    )
+    policy = _make_policy(client, builder, max_repairs=2)
+
+    actions = policy(_obs())
+    assert actions == (Action(kind="wait"),)
+
+    # First call: bare prompt with no trail.
+    assert len(client.prompts) == 3
+    assert "-- previous reply invalid" not in client.prompts[0]
+    # First retry: trail carries attempt 1 only.
+    assert "attempt 1" in client.prompts[1]
+    assert "attempt 2" not in client.prompts[1]
+    assert "Return a valid JSON action sequence." in client.prompts[1]
+    # Second retry: trail carries attempts 1 AND 2, in order.
+    attempt1 = client.prompts[2].index("-- previous reply invalid (attempt 1) --")
+    attempt2 = client.prompts[2].index("-- previous reply invalid (attempt 2) --")
+    assert attempt1 < attempt2
+    # The base prompt is preserved at the head of every retry.
+    assert client.prompts[1].startswith(client.prompts[0])
+
+
+def test_nl_policy_max_repairs_negative_rejected() -> None:
+    """A negative max_repairs is a configuration error, not a fallback."""
+    builder = _builder()
+    client = _make_client(['[{"kind": "wait"}]'], builder=builder)
+    with pytest.raises(ValueError, match=r"max_repairs must be >= 0, got -1"):
+        _make_policy(client, builder, max_repairs=-1)
+
+
+def test_nl_policy_stats_after_direct_success() -> None:
+    """A clean first-try success leaves the repair counters untouched."""
+    builder = _builder()
+    client = _make_client(['[{"kind": "wait"}]'], builder=builder)
+    policy = _make_policy(client, builder)
+
+    actions = policy(_obs())
+    assert actions == (Action(kind="wait"),)
+    assert policy.stats.calls == 1
+    assert policy.stats.repairs_attempted == 0
+    assert policy.stats.repairs_succeeded == 0
+    assert policy.stats.fallbacks == 0