feat(phase2): OpenAI-compatible provider client (P5.1)

2026-09-25 22:22:03 +02:00
parent 790df17623
commit 42802f7bf5
4 changed files with 597 additions and 2 deletions
@@ -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"<html>oops</html>"),))
+    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,
+        )