diff --git a/pyproject.toml b/pyproject.toml index f8e0353..1750cf4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,6 +58,12 @@ allowed-confusables = ["σ"] "src/pf1e_simulator/runner.py" = ["PLR0913"] # client.py: LLMClient/FakeLLMClient need the full safeguard knobs (cache, budget, pricing, retry, breaker) "src/pf1e_simulator/client.py" = ["PLR0913"] +# provider.py: OpenAILLMClient.__init__ needs the full knob set (model, key, +# base url, timeout, transport, schema, cache, budget, pricing, retry, breaker) +# and the chat-completions URL is built from an env-gated base URL defaulting +# to HTTPS (section 15), so the S310 audit and the 14-param constructor are +# deliberate +"src/pf1e_simulator/provider.py" = ["PLR0913", "S310"] # robustness.py: the cartography API takes the full knob set (scenario, specs, # side, runs, seed, fractions, target, precision, budget) as keyword parameters "src/pf1e_simulator/robustness.py" = ["PLR0913"] diff --git a/src/pf1e_simulator/config.py b/src/pf1e_simulator/config.py index 03e8d63..50e1348 100644 --- a/src/pf1e_simulator/config.py +++ b/src/pf1e_simulator/config.py @@ -1,4 +1,4 @@ -"""Environment-driven configuration for the phase-2 LLM layer (P1). +"""Environment-driven configuration for the phase-2 LLM layer (P1/P5.1). Architecture doc section 15: the phase-2 code never hard-codes data or configuration paths -- every externally supplied value is resolved here from @@ -14,9 +14,19 @@ from pathlib import Path # Environment variable names (documented in the architecture doc, sections # 14-15). ``PF1E_PROMPT_PACK_DIR`` points at the external, versioned prompt -# pack; ``PF1E_LLM_API_KEY`` feeds the remote provider client (P5.1). +# pack; ``PF1E_LLM_API_KEY`` feeds the remote provider client (P5.1); +# ``PF1E_LLM_BASE_URL`` and ``PF1E_LLM_MODEL`` select the provider endpoint +# and model, so a self-hosted OpenAI-compatible gateway (vLLM, llama.cpp...) +# is configured without touching the code. ENV_PROMPT_PACK_DIR = "PF1E_PROMPT_PACK_DIR" ENV_LLM_API_KEY = "PF1E_LLM_API_KEY" +ENV_LLM_BASE_URL = "PF1E_LLM_BASE_URL" +ENV_LLM_MODEL = "PF1E_LLM_MODEL" + +# Sensible offline defaults: the OpenAI-compatible chat endpoint and a small, +# cheap model. Nothing here is a secret -- the API key stays environment-only. +DEFAULT_LLM_BASE_URL = "https://api.openai.com/v1" +DEFAULT_LLM_MODEL = "gpt-4o-mini" def env_str(name: str, *, default: str | None = None) -> str | None: @@ -34,3 +44,19 @@ def llm_api_key() -> str | None: """The provider API key, or None when the caller runs offline.""" return env_str(ENV_LLM_API_KEY) + +def llm_base_url() -> str: + """The OpenAI-compatible base URL (``PF1E_LLM_BASE_URL``, else default). + + The returned value has no trailing slash; ``OpenAILLMClient`` appends + ``/chat/completions`` to it (P5.1). + """ + raw = env_str(ENV_LLM_BASE_URL) + return raw.rstrip("/") if raw else DEFAULT_LLM_BASE_URL + + +def llm_model() -> str: + """The provider model name (``PF1E_LLM_MODEL``, else default).""" + raw = env_str(ENV_LLM_MODEL) + return raw or DEFAULT_LLM_MODEL + diff --git a/src/pf1e_simulator/provider.py b/src/pf1e_simulator/provider.py new file mode 100644 index 0000000..e16c0b1 --- /dev/null +++ b/src/pf1e_simulator/provider.py @@ -0,0 +1,227 @@ +"""Real OpenAI-compatible chat client for the phase-2 LLM layer (P5.1). + +``OpenAILLMClient`` speaks the OpenAI chat-completions wire format over HTTPS +using only the standard library (``urllib.request``), so the repository ships +no HTTP dependency. The base URL, model and API key are resolved from the +environment (:mod:`pf1e_simulator.config`, section 15); the key is never +embedded in code. + +Error taxonomy (shared with :mod:`pf1e_simulator.client`): + +- ``ProviderError`` for transient failures (transport errors, HTTP 429, any + 5xx) -- retried by ``LLMClient``'s backoff loop and counted by the breaker; +- ``ProviderRejectedError`` for permanent 4xx rejections (bad key, unknown + model, quota) -- raised immediately, never retried; +- ``ProviderResponseError`` for a 2xx body that is not a valid + chat-completions payload -- raised immediately, never retried. +""" + +from __future__ import annotations + +import json +import time +import urllib.request +from typing import TYPE_CHECKING, Protocol, Self, TypeGuard +from urllib.error import HTTPError +from urllib.request import Request + +from pf1e_simulator.client import ( + CircuitBreaker, + ClientError, + ClientParams, + Completion, + CostBudget, + LLMClient, + Pricing, + ProviderError, + ResponseCache, + TokenUsage, + estimate_tokens, +) +from pf1e_simulator.combat import OBSERVATION_SCHEMA_VERSION +from pf1e_simulator.config import ( + ENV_LLM_API_KEY, + llm_api_key, + llm_base_url, + llm_model, +) + +if TYPE_CHECKING: + from collections.abc import Callable + +# Status-code buckets of the chat-completions wire (RFC 9110): 2xx is a +# success, 429 and every 5xx mean "try again later" (the only statuses safe +# to retry), and any other 4xx is a permanent rejection. +_OK_MIN = 200 +_OK_EXCLUSIVE = 300 +_RATE_LIMITED = 429 +_SERVER_ERROR_MIN = 500 + + +class ProviderRejectedError(ClientError): + """A permanent 4xx provider rejection - never retried.""" + + +class ProviderResponseError(ClientError): + """A 2xx body that is not a valid chat-completions payload.""" + + +class HttpResponse(Protocol): + """The minimal ``urllib.response`` surface the wire call uses.""" + + status: int + + def read(self) -> bytes: ... + + def __enter__(self) -> Self: ... + + def __exit__(self, _exc_type: object, _exc_value: object, _traceback: object) -> None: ... + + +def _is_object_dict(value: object) -> TypeGuard[dict[str, object]]: + return isinstance(value, dict) + + +def _is_object_list(value: object) -> TypeGuard[list[object]]: + return isinstance(value, list) + + +def _parse_completion(payload: dict[str, object]) -> str: + """Extract the completion text from a chat-completions payload.""" + choices_raw = payload.get("choices") + if not _is_object_list(choices_raw) or not choices_raw: + msg = "provider response has no choices" + raise ProviderResponseError(msg) + choice = choices_raw[0] + if not _is_object_dict(choice): + msg = "provider response choice is not a JSON object" + raise ProviderResponseError(msg) + message_raw = choice.get("message") + if not _is_object_dict(message_raw): + msg = "provider response message is not a JSON object" + raise ProviderResponseError(msg) + content = message_raw.get("content") + if not isinstance(content, str): + msg = "provider response content is not a string" + raise ProviderResponseError(msg) + return content + + +def _parse_usage(payload: dict[str, object], prompt: str, text: str) -> TokenUsage: + """Read the usage block, or estimate tokens when the provider omits it.""" + usage_raw = payload.get("usage") + if not _is_object_dict(usage_raw): + return TokenUsage( + prompt_tokens=estimate_tokens(prompt), + completion_tokens=estimate_tokens(text), + ) + prompt_tokens = usage_raw.get("prompt_tokens") + completion_tokens = usage_raw.get("completion_tokens") + if not isinstance(prompt_tokens, int) or not isinstance(completion_tokens, int): + msg = "provider usage block is malformed" + raise ProviderResponseError(msg) + return TokenUsage(prompt_tokens=prompt_tokens, completion_tokens=completion_tokens) + + +class OpenAILLMClient(LLMClient): + """Real chat-completions client (OpenAI or any compatible gateway). + + Endpoint, model and key default to the environment (section 15); all may + be overridden explicitly. ``urlopen`` and ``sleep`` are injectable so + tests exercise the full retry/breaker machinery offline. + """ + + def __init__( + self, + *, + prompt_version: str, + model: str | None = None, + api_key: str | None = None, + base_url: str | None = None, + timeout_s: float = 30.0, + urlopen: Callable[..., HttpResponse] = urllib.request.urlopen, + schema_version: int = OBSERVATION_SCHEMA_VERSION, + cache: ResponseCache | None = None, + budget: CostBudget | None = None, + pricing: Pricing | None = None, + retries: int = 2, + backoff_base_s: float = 0.25, + breaker: CircuitBreaker | None = None, + sleep: Callable[[float], None] | None = None, + ) -> None: + key = api_key if api_key is not None else llm_api_key() + if key is None or key == "": + msg = ( + f"no LLM API key: set the {ENV_LLM_API_KEY} environment " + "variable or pass an explicit api_key" + ) + raise ValueError(msg) + if timeout_s <= 0: + msg = f"timeout_s must be > 0, got {timeout_s}" + raise ValueError(msg) + super().__init__( + model=model if model is not None else llm_model(), + prompt_version=prompt_version, + schema_version=schema_version, + cache=cache, + budget=budget, + pricing=pricing, + retries=retries, + backoff_base_s=backoff_base_s, + sleep=sleep if sleep is not None else time.sleep, + breaker=breaker, + ) + resolved_base_url = base_url if base_url is not None else llm_base_url() + self._api_key = key + self._base_url = resolved_base_url.rstrip("/") + self._timeout_s = timeout_s + self._urlopen = urlopen + + def _call_once(self, prompt: str, *, params: ClientParams) -> Completion: + """One chat-completions attempt; raise ``ProviderError`` when retryable.""" + body = { + "model": self.model, + "messages": [{"role": "user", "content": prompt}], + "temperature": params.temperature, + "max_tokens": params.max_tokens, + "top_p": params.top_p, + "response_format": {"type": "json_object"}, + } + request = Request( + f"{self._base_url}/chat/completions", + data=json.dumps(body).encode("utf-8"), + headers={ + "Authorization": f"Bearer {self._api_key}", + "Content-Type": "application/json", + }, + method="POST", + ) + try: + with self._urlopen(request, timeout=self._timeout_s) as response: + status = response.status + data = response.read() + except HTTPError as exc: + status = exc.code + data = exc.read() + except OSError as exc: + msg = f"provider unreachable: {exc}" + raise ProviderError(msg) from exc + + if status == _RATE_LIMITED or status >= _SERVER_ERROR_MIN: + msg = f"provider transient failure: HTTP {status}" + raise ProviderError(msg) + if not _OK_MIN <= status < _OK_EXCLUSIVE: + msg = f"provider rejected request: HTTP {status}" + raise ProviderRejectedError(msg) + + try: + payload = json.loads(data.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + msg = f"provider response is not valid JSON: {exc}" + raise ProviderResponseError(msg) from exc + if not _is_object_dict(payload): + msg = "provider payload is not a JSON object" + raise ProviderResponseError(msg) + text = _parse_completion(payload) + usage = _parse_usage(payload, prompt, text) + return Completion(text=text, usage=usage) diff --git a/tests/test_phase2_provider.py b/tests/test_phase2_provider.py new file mode 100644 index 0000000..d5ef59d --- /dev/null +++ b/tests/test_phase2_provider.py @@ -0,0 +1,336 @@ +"""Phase-2 real provider client (docs/phase-2 P5.1, sections 15/16). + +``OpenAILLMClient`` (``provider.py``) speaks the OpenAI chat-completions +wire format over ``urllib.request`` with zero HTTP dependency. These tests +pin the wire format, the error taxonomy (transient 429/5xx retried, 4xx +rejected, invalid payload rejected), the env-key resolution, the token +ledger and the cache behaviour - all through an injectable fake transport, +never the network. Every key used below is a fake test credential. +""" + +from __future__ import annotations + +import io +import json +from typing import TYPE_CHECKING, Self +from urllib.error import HTTPError + +import pytest + +from pf1e_simulator.client import ( + RetryExhaustedError, + estimate_tokens, +) +from pf1e_simulator.config import ( + ENV_LLM_API_KEY, + ENV_LLM_BASE_URL, + ENV_LLM_MODEL, +) +from pf1e_simulator.provider import ( + OpenAILLMClient, + ProviderRejectedError, + ProviderResponseError, +) + +if TYPE_CHECKING: + from collections.abc import Sequence + from urllib.request import Request + +_KEY = "sk-test" +_ENV_KEY = "sk-env-key" +_PROMPT_VERSION = "fixture:provider:0" +_BASE_URL = "https://api.openai.com/v1" +_MODEL = "gpt-4o-mini" + + +class _FakeResponse: + """Minimal ``urllib.response`` double: status + body, context manager.""" + + def __init__(self, status: int, body: bytes) -> None: + self.status = status + self._body = body + + def read(self) -> bytes: + return self._body + + def __enter__(self) -> Self: + return self + + def __exit__(self, _exc_type: object, _exc_value: object, _traceback: object) -> None: + return None + + +class _FakeTransport: + """Injectable ``urlopen``: replays a script, keeping the last entry. + + An ``Exception`` entry is raised; provider code sees it exactly as it + would see a real transport failure. + """ + + def __init__(self, script: Sequence[_FakeResponse | Exception]) -> None: + self._script = tuple(script) + self._calls: list[tuple[Request, float]] = [] + + def __call__(self, request: Request, *, timeout: float) -> _FakeResponse: + self._calls.append((request, timeout)) + index = min(len(self._calls) - 1, len(self._script) - 1) + entry = self._script[index] + if isinstance(entry, Exception): + raise entry + return entry + + @property + def calls(self) -> tuple[tuple[Request, float], ...]: + return tuple(self._calls) + + +def _http_error(status: int) -> HTTPError: + return HTTPError( + f"{_BASE_URL}/chat/completions", + status, + "simulated HTTP error", + hdrs={}, + fp=io.BytesIO(b"{}"), + ) + + +def _ok_response(*, content: str = "action json", include_usage: bool = True) -> _FakeResponse: + body: dict[str, object] = { + "choices": [{"message": {"content": content}}], + } + if include_usage: + body["usage"] = {"prompt_tokens": 12, "completion_tokens": 7} + return _FakeResponse(200, json.dumps(body).encode("utf-8")) + + +def _fake_transport(script: Sequence[_FakeResponse | Exception]) -> _FakeTransport: + return _FakeTransport(script) + + +def _client( + transport: _FakeTransport, + *, + api_key: str = _KEY, + retries: int = 2, + timeout_s: float = 30.0, + base_url: str = _BASE_URL, + model: str = _MODEL, +) -> OpenAILLMClient: + return OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + model=model, + api_key=api_key, + base_url=base_url, + timeout_s=timeout_s, + urlopen=transport, + retries=retries, + sleep=lambda _: None, + ) + + +# ── wire format ───────────────────────────────────────────────────────────── + + +def test_wire_format_matches_openai_chat_completions() -> None: + """The request is a POST to /v1/chat/completions with the raw body.""" + transport = _fake_transport((_ok_response(),)) + client = _client(transport) + assert client.complete("hello") == "action json" + assert len(transport.calls) == 1 + request, timeout = transport.calls[0] + assert timeout == 30.0 + assert request.full_url == f"{_BASE_URL}/chat/completions" + assert request.method == "POST" + assert request.get_header("Authorization") == f"Bearer {_KEY}" + # urllib capitalizes header keys via str.capitalize(): "Content-Type" is + # stored as "Content-type". + assert request.get_header("Content-type") == "application/json" + assert request.data is not None + body = json.loads(request.data) + assert body["model"] == _MODEL + assert body["messages"] == [{"role": "user", "content": "hello"}] + assert body["temperature"] == 0.0 + assert body["max_tokens"] == 1024 + assert body["top_p"] == 1.0 + assert body["response_format"] == {"type": "json_object"} + + +def test_ledger_records_provider_usage() -> None: + """Provider-reported token counts land in the client ledger.""" + transport = _fake_transport((_ok_response(),)) + client = _client(transport) + client.complete("hello") + assert client.stats.calls == 1 + assert client.stats.prompt_tokens == 12 + assert client.stats.completion_tokens == 7 + assert client.stats.cost_usd == 0.0 + assert client.stats.cache_hits == 0 + assert client.stats.cache_misses == 1 + + +def test_usage_estimated_when_provider_omits_it() -> None: + """Without a usage block, tokens are estimated from character counts.""" + transport = _fake_transport((_ok_response(include_usage=False),)) + client = _client(transport) + client.complete("hello") + assert client.stats.prompt_tokens == estimate_tokens("hello") + assert client.stats.completion_tokens == estimate_tokens("action json") + + +# ── key resolution ────────────────────────────────────────────────────────── + + +def test_missing_or_empty_api_key_raises(monkeypatch: pytest.MonkeyPatch) -> None: + """A missing or empty key is rejected with the env-var name in the error.""" + monkeypatch.delenv(ENV_LLM_API_KEY, raising=False) + with pytest.raises(ValueError, match="PF1E_LLM_API_KEY"): + OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + api_key=None, + base_url=_BASE_URL, + urlopen=_fake_transport((_ok_response(),)), + ) + with pytest.raises(ValueError, match="PF1E_LLM_API_KEY"): + OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + api_key="", + base_url=_BASE_URL, + urlopen=_fake_transport((_ok_response(),)), + ) + + +def test_api_key_read_from_environment(monkeypatch: pytest.MonkeyPatch) -> None: + """With no explicit key, PF1E_LLM_API_KEY is used for the bearer token.""" + monkeypatch.setenv(ENV_LLM_API_KEY, _ENV_KEY) + transport = _fake_transport((_ok_response(),)) + client = OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + api_key=None, + base_url=_BASE_URL, + urlopen=transport, + sleep=lambda _: None, + ) + client.complete("hello") + request, _timeout = transport.calls[0] + assert request.get_header("Authorization") == f"Bearer {_ENV_KEY}" + + +# ── error taxonomy and retries ────────────────────────────────────────────── + + +def test_rate_limit_retries_then_succeeds() -> None: + """HTTP 429 is transient: with retries=2 the third attempt succeeds.""" + transport = _fake_transport( + (_http_error(429), _http_error(429), _ok_response()), + ) + client = _client(transport, retries=2) + assert client.complete("hello") == "action json" + assert client.calls == 3 + + +def test_transient_oserror_retries_then_exhausts() -> None: + """A transport error is retried, then RetryExhaustedError names it.""" + transport = _fake_transport((TimeoutError("simulated timeout"),)) + client = _client(transport, retries=1) + with pytest.raises(RetryExhaustedError, match="unreachable"): + client.complete("hello") + assert client.calls == 2 + + +def test_permanent_4xx_is_not_retried() -> None: + """HTTP 401 is a permanent rejection: one attempt, then raise.""" + transport = _fake_transport((_http_error(401),)) + client = _client(transport, retries=3) + with pytest.raises(ProviderRejectedError, match="HTTP 401"): + client.complete("hello") + assert client.calls == 1 + + +def test_server_error_retries_then_exhausts() -> None: + """HTTP 500 is transient: retries exhausted raises with the status.""" + transport = _fake_transport((_http_error(500),)) + client = _client(transport, retries=1) + with pytest.raises(RetryExhaustedError, match="HTTP 500"): + client.complete("hello") + assert client.calls == 2 + + +# ── payload validation ────────────────────────────────────────────────────── + + +def test_invalid_json_body_raises_provider_response_error() -> None: + """A 2xx body that is not JSON is a permanent response error.""" + transport = _fake_transport((_FakeResponse(200, b"oops"),)) + client = _client(transport) + with pytest.raises(ProviderResponseError, match="not valid JSON"): + client.complete("hello") + + +@pytest.mark.parametrize( + ("payload", "message"), + [ + ({"choices": []}, "no choices"), + ({"choices": [{}]}, "message"), + ({"choices": [{"message": {}}]}, "content"), + ([], "not a JSON object"), + ], +) +def test_malformed_payload_raises_provider_response_error( + payload: object, + message: str, +) -> None: + """Non-chat-completions payloads are rejected with a stable message.""" + transport = _fake_transport( + (_FakeResponse(200, json.dumps(payload).encode("utf-8")),), + ) + client = _client(transport) + with pytest.raises(ProviderResponseError, match=message): + client.complete("hello") + + +# ── caching and environment defaults ──────────────────────────────────────── + + +def test_cache_serves_second_call_without_provider() -> None: + """An identical second call is served from the cache, no wire traffic.""" + transport = _fake_transport((_ok_response(),)) + client = _client(transport) + assert client.complete("hello") == "action json" + assert client.complete("hello") == "action json" + assert len(transport.calls) == 1 + assert client.calls == 1 + assert client.cache_hits == 1 + assert client.cache_misses == 1 + + +def test_base_url_and_model_read_from_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """PF1E_LLM_BASE_URL/PF1E_LLM_MODEL drive the endpoint and the body.""" + monkeypatch.setenv(ENV_LLM_BASE_URL, "http://localhost:8080/v1/") + monkeypatch.setenv(ENV_LLM_MODEL, "local-model") + transport = _fake_transport((_ok_response(),)) + client = OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + api_key=_KEY, + base_url=None, + model=None, + urlopen=transport, + sleep=lambda _: None, + ) + client.complete("hello") + request, _timeout = transport.calls[0] + assert request.full_url == "http://localhost:8080/v1/chat/completions" + assert request.data is not None + assert json.loads(request.data)["model"] == "local-model" + + +def test_invalid_timeout_rejected() -> None: + """A non-positive timeout_s is a construction error, not a wire call.""" + with pytest.raises(ValueError, match="timeout_s"): + OpenAILLMClient( + prompt_version=_PROMPT_VERSION, + api_key=_KEY, + base_url=_BASE_URL, + timeout_s=0, + )