From 48def19d0158873759129f67ab71f6d156d8b277 Mon Sep 17 00:00:00 2001 From: Eugene Eisenstein Date: Fri, 28 Aug 2026 16:48:52 -0400 Subject: [PATCH] refactor(mock-provider): validate requests with Pydantic models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: hand-coercing the request bodies was defended on the grounds that FastAPI answers a malformed body with a 422, and a 422 mid-deriver-run reads as a Honcho bug. That argues against the default handler, not against the models. Registering an exception handler fixes it — and the resulting behaviour is more faithful, not less, because the real API answers a bad request with a 400 and an `error` envelope, which is now exactly what the mock returns. Adds src/mock_provider/schemas.py with ChatCompletionRequest and EmbeddingsRequest. Every model allows extra fields and every field is optional, so validation fires on a wrong type rather than on a parameter the mock has not heard of — a new upstream parameter must not turn a working setup into a hard failure. dimensions is a StrictInt because bool is an int subclass and a JSON `true` would otherwise mean a one-dimensional vector. coerce.py stays, narrowed to serving schema_gen, which walks arbitrary caller-supplied JSON Schema and is untyped by nature. response_format likewise stays dict[str, Any]: only its envelope is worth typing. Also records why schema_gen does not reuse src/utils/schema_conversion.py despite the overlapping $ref/$defs handling — it builds a model class rather than an instance, raises by contract where a mock must degrade, and rejects both allOf and the recursive $ref that reasoning-tree schemas rely on. Documents that LLM_OPENAI_API_KEY is only tested for truthiness; the previous wording read as though the value had to be the literal string "sandbox". Re-verified end to end after the refactor: 6 messages in, 4 conclusions and 6 1536-dim embeddings out, every real request answered 200, no calls to any real provider. Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 7 +- src/mock_provider/chat.py | 45 ++++++------- src/mock_provider/embeddings.py | 60 ++++++++--------- src/mock_provider/main.py | 38 ++++++++++- src/mock_provider/schema_gen.py | 9 +++ src/mock_provider/schemas.py | 58 +++++++++++++++++ tests/mock_provider/test_mock_provider.py | 79 +++++++++++++++++++++++ 7 files changed, 235 insertions(+), 61 deletions(-) create mode 100644 src/mock_provider/schemas.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 59b01cf9..d643c01c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -233,11 +233,16 @@ uv run fastapi run --host 0.0.0.0 --port 8106 src/mock_provider/main.py Then point Honcho at it. All three variables are required: ```bash -export LLM_OPENAI_API_KEY=sandbox # gates client construction, not just auth +export LLM_OPENAI_API_KEY=any-non-empty-string # only truthiness is checked export LLM_OPENAI_BASE_URL=http://localhost:8106/v1 export EMBEDDING_MODEL_CONFIG__OVERRIDES__BASE_URL=http://localhost:8106/v1 ``` +The key's *value* is never checked — the mock reads no Authorization header, and Honcho only +tests it for truthiness before building the client (`src/llm/registry.py`). Set the base URL +without it and the client is never constructed, so the base URL is silently ignored. Keep the +value obviously fake, so a module that ever escapes the override 401s rather than spends. + Embeddings resolve through a separate client that reads the base URL only from the per-module override, so without the third variable your embedding calls go to `api.openai.com` for real. Do not set any per-module credential override (`..._OVERRIDES__API_KEY` / `API_KEY_ENV`) — diff --git a/src/mock_provider/chat.py b/src/mock_provider/chat.py index df79a507..b8694d63 100644 --- a/src/mock_provider/chat.py +++ b/src/mock_provider/chat.py @@ -8,11 +8,12 @@ import re from collections.abc import AsyncIterator from typing import Any -from fastapi import APIRouter, Request +from fastapi import APIRouter from fastapi.responses import StreamingResponse -from src.mock_provider.coerce import as_dict, as_list, as_str +from src.mock_provider.coerce import as_dict, as_str from src.mock_provider.schema_gen import generate +from src.mock_provider.schemas import ChatCompletionRequest, ChatMessage router = APIRouter(tags=["mock-provider"]) @@ -22,10 +23,10 @@ router = APIRouter(tags=["mock-provider"]) _SCHEMA_HINT = re.compile(r"schema:\s*(\{)", re.IGNORECASE) -def _completion_id(payload: dict[str, Any]) -> str: +def _completion_id(body: ChatCompletionRequest) -> str: """Stable id, so a replayed request is byte-identical.""" digest = hashlib.sha256( - json.dumps(payload, sort_keys=True, default=str).encode() + body.model_dump_json(exclude_none=True).encode() ).hexdigest() return f"chatcmpl-mock-{digest[:24]}" @@ -64,13 +65,10 @@ def _extract_balanced_json(text: str, start: int) -> dict[str, Any] | None: return None -def _schema_from_messages(messages: list[Any]) -> dict[str, Any] | None: +def _schema_from_messages(messages: list[ChatMessage]) -> dict[str, Any] | None: """Recover an injected schema from the prompt, for json_object mode.""" for message in reversed(messages): - message_dict = as_dict(message) - if message_dict is None: - continue - content = as_str(message_dict.get("content")) + content = as_str(message.content) if content is None: continue for match in _SCHEMA_HINT.finditer(content): @@ -80,10 +78,9 @@ def _schema_from_messages(messages: list[Any]) -> dict[str, Any] | None: return None -def _response_content(payload: dict[str, Any]) -> str: +def _response_content(body: ChatCompletionRequest) -> str: """The assistant message body: schema-conforming JSON, or prose.""" - response_format = as_dict(payload.get("response_format")) - messages = as_list(payload.get("messages")) or [] + response_format = body.response_format if response_format is not None: kind = as_str(response_format.get("type")) @@ -98,7 +95,7 @@ def _response_content(payload: dict[str, Any]) -> str: # exists to avoid. An empty object at least parses. return "{}" if kind == "json_object": - schema = _schema_from_messages(messages) + schema = _schema_from_messages(body.messages) return json.dumps(generate(schema)) if schema else "{}" return ( @@ -107,13 +104,11 @@ def _response_content(payload: dict[str, Any]) -> str: ) -def _usage(payload: dict[str, Any], content: str) -> dict[str, int]: +def _usage(body: ChatCompletionRequest, content: str) -> dict[str, int]: """Rough token accounting, so cost telemetry has plausible numbers.""" - messages = as_list(payload.get("messages")) or [] prompt_chars = 0 - for message in messages: - message_dict = as_dict(message) - text = as_str(message_dict.get("content")) if message_dict else None + for message in body.messages: + text = as_str(message.content) if text is not None: prompt_chars += len(text) prompt_tokens = max(1, prompt_chars // 4) @@ -176,15 +171,13 @@ async def _stream( @router.post("/chat/completions") -async def chat_completions(request: Request) -> Any: - payload = as_dict(await request.json()) or {} +async def chat_completions(body: ChatCompletionRequest) -> Any: + model = body.model or "mock-model" + content = _response_content(body) + usage = _usage(body, content) + completion_id = _completion_id(body) - model = as_str(payload.get("model")) or "mock-model" - content = _response_content(payload) - usage = _usage(payload, content) - completion_id = _completion_id(payload) - - if payload.get("stream"): + if body.stream: return StreamingResponse( _stream(completion_id, model, content, usage), media_type="text/event-stream", diff --git a/src/mock_provider/embeddings.py b/src/mock_provider/embeddings.py index 522f6a5e..618ff7ba 100644 --- a/src/mock_provider/embeddings.py +++ b/src/mock_provider/embeddings.py @@ -7,9 +7,9 @@ import hashlib import struct from typing import Any -from fastapi import APIRouter, Request +from fastapi import APIRouter -from src.mock_provider.coerce import as_dict, as_int, as_list, as_str +from src.mock_provider.schemas import EmbeddingsRequest router = APIRouter(tags=["mock-provider"]) @@ -39,47 +39,41 @@ def _encode_base64(vector: list[float]) -> str: return base64.b64encode(struct.pack(f"<{len(vector)}f", *vector)).decode() -def _normalize_input(raw: object) -> list[str]: - """Accept every input shape the OpenAI embeddings API allows. +def _normalize_input( + raw: str | list[str] | list[int] | list[list[int]] | None, +) -> list[str]: + """Flatten the request input into one string per embedding to return. - Token-array inputs (list[int] / list[list[int]]) are rendered back to a - stable string rather than rejected — the vector only has to be deterministic. + Token-array inputs are rendered back to a stable string rather than + rejected — the vector only has to be deterministic, not meaningful. """ if raw is None: return [] - if (text := as_str(raw)) is not None: - return [text] - - items = as_list(raw) - if items is None: - return [str(raw)] - + if isinstance(raw, str): + return [raw] # A flat list of ints is one tokenized input, not many single-token ones. - if items and all(as_int(item) is not None for item in items): - return [",".join(str(item) for item in items)] + if raw and all(isinstance(item, int) for item in raw): + return [",".join(str(item) for item in raw)] texts: list[str] = [] - for item in items: - if (item_text := as_str(item)) is not None: - texts.append(item_text) - elif (parts := as_list(item)) is not None: - texts.append(",".join(str(part) for part in parts)) + for item in raw: + if isinstance(item, str): + texts.append(item) + elif isinstance(item, list): + texts.append(",".join(str(part) for part in item)) else: texts.append(str(item)) return texts @router.post("/embeddings") -async def embeddings(request: Request) -> Any: - payload = as_dict(await request.json()) or {} - - texts = _normalize_input(payload.get("input")) - requested = as_int(payload.get("dimensions")) - dimensions = requested if requested and requested > 0 else DEFAULT_DIMENSIONS - - # The SDK omits encoding_format only when it wants base64 (it sets the - # parameter explicitly for float), so absent means base64. - encoding_format = as_str(payload.get("encoding_format")) or "base64" +async def embeddings(body: EmbeddingsRequest) -> Any: + texts = _normalize_input(body.input) + dimensions = ( + body.dimensions + if body.dimensions and body.dimensions > 0 + else DEFAULT_DIMENSIONS + ) data: list[dict[str, Any]] = [] for index, text in enumerate(texts): @@ -89,7 +83,9 @@ async def embeddings(request: Request) -> Any: "object": "embedding", "index": index, "embedding": ( - vector if encoding_format == "float" else _encode_base64(vector) + vector + if body.encoding_format == "float" + else _encode_base64(vector) ), } ) @@ -98,6 +94,6 @@ async def embeddings(request: Request) -> Any: return { "object": "list", "data": data, - "model": as_str(payload.get("model")) or "mock-embedding", + "model": body.model or "mock-embedding", "usage": {"prompt_tokens": prompt_tokens, "total_tokens": prompt_tokens}, } diff --git a/src/mock_provider/main.py b/src/mock_provider/main.py index 154a62a3..b14dcc34 100644 --- a/src/mock_provider/main.py +++ b/src/mock_provider/main.py @@ -10,10 +10,17 @@ Runs as its own service from the standard Honcho image: Point Honcho at it with three variables — all three are required: - LLM_OPENAI_API_KEY=sandbox # gates client construction, not just auth + LLM_OPENAI_API_KEY=any-non-empty-string # only truthiness is checked; value ignored LLM_OPENAI_BASE_URL=http://mock-provider:8000/v1 EMBEDDING_MODEL_CONFIG__OVERRIDES__BASE_URL=http://mock-provider:8000/v1 +The key's *value* is never checked — not by this mock, which reads no +Authorization header, and not by Honcho, which only tests it for truthiness +before constructing the client (``src/llm/registry.py``). Set the base URL +without it and the client is never built, so the base URL is silently ignored. +Keep the value obviously fake: if a module ever escapes the base-URL override it +then 401s against the real provider instead of spending. + Embeddings resolve through a separate client that reads the base URL only from the per-module override, so without the third variable embedding calls go to api.openai.com for real. Do not set any per-module credential override @@ -28,7 +35,9 @@ from __future__ import annotations from typing import Any -from fastapi import FastAPI +from fastapi import FastAPI, Request +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse from src.mock_provider import chat, embeddings @@ -38,6 +47,31 @@ app = FastAPI( version="1.0.0", ) + +@app.exception_handler(RequestValidationError) +async def openai_error_response( + _request: Request, exc: RequestValidationError +) -> JSONResponse: + """Answer a malformed request the way the real API does. + + FastAPI's default is a 422 carrying its own error shape. Mid-run that reads + as a Honcho bug rather than a bad request, and it is not what an OpenAI + client expects — the real API returns 400 with an ``error`` envelope, so + that is what a faithful mock returns. + """ + return JSONResponse( + status_code=400, + content={ + "error": { + "message": f"Invalid request: {exc.errors()}", + "type": "invalid_request_error", + "param": None, + "code": None, + } + }, + ) + + # Mounted at both prefixes so the base URL works with or without /v1. for _router in (chat.router, embeddings.router): app.include_router(_router, prefix="/v1") diff --git a/src/mock_provider/schema_gen.py b/src/mock_provider/schema_gen.py index eb0f2533..65b5a739 100644 --- a/src/mock_provider/schema_gen.py +++ b/src/mock_provider/schema_gen.py @@ -9,6 +9,15 @@ actually sent, ``$ref`` indirection and all. Values are derived from a hash of the property path, so the same schema always produces the same instance and two different fields never collide. + +Not reused from ``src/utils/schema_conversion.py``, despite the overlapping +``$ref``/``$defs`` handling, because that module answers a different question and +does so under an incompatible contract. It builds a Pydantic *model class* where +this needs an *instance*; it raises by design (conversion doubles as validation, +surfaced to callers as a 422) where a mock must degrade rather than turn its own +defect into a 500; and it rejects both ``allOf`` and recursive ``$ref`` — the +latter being ordinary input here, since reasoning-tree schemas nest premises +inside conclusions. """ from __future__ import annotations diff --git a/src/mock_provider/schemas.py b/src/mock_provider/schemas.py new file mode 100644 index 00000000..3dfe39ab --- /dev/null +++ b/src/mock_provider/schemas.py @@ -0,0 +1,58 @@ +"""Request models for the mock provider's OpenAI-compatible endpoints. + +Validating the request envelope rather than hand-coercing it makes the mock +behave like the thing it mocks: real OpenAI answers a malformed request with a +400 and an error envelope, and ``openai_error_response`` in ``main`` turns +Pydantic's failure into exactly that. + +Two deliberate choices: + +- ``extra="allow"`` on every model, and every field optional. Validation should + fire on a wrong *type* (a string where a list belongs), never on a field this + mock has not heard of — otherwise a new upstream parameter turns a working + setup into a hard failure. +- Open-ended payloads stay ``dict[str, Any]``. ``response_format`` carries an + arbitrary caller-supplied JSON Schema, so only its envelope is worth typing; + ``schema_gen`` walks the rest. +""" + +from __future__ import annotations + +from typing import Any, ClassVar, Literal + +from pydantic import BaseModel, ConfigDict, StrictInt + + +class MockRequest(BaseModel): + """Permissive base: unknown fields pass through untouched.""" + + model_config: ClassVar[ConfigDict] = ConfigDict(extra="allow") + + +class ChatMessage(MockRequest): + role: str | None = None + # Multimodal requests send a list of content parts rather than a string, so + # this cannot narrow further. + content: Any = None + + +class ChatCompletionRequest(MockRequest): + model: str | None = None + messages: list[ChatMessage] = [] + response_format: dict[str, Any] | None = None + tools: list[dict[str, Any]] | None = None + stream: bool = False + stream_options: dict[str, Any] | None = None + + +class EmbeddingsRequest(MockRequest): + # Every input shape the OpenAI embeddings API accepts. Pydantic's smart + # union keeps list[str] and list[int] apart instead of coercing one to the + # other. + input: str | list[str] | list[int] | list[list[int]] | None = None + model: str | None = None + # StrictInt because bool is an int subclass: a JSON `true` here would + # otherwise silently become a one-dimensional vector. + dimensions: StrictInt | None = None + # The SDK omits this only when it wants base64, so absent means base64. + encoding_format: Literal["float", "base64"] = "base64" diff --git a/tests/mock_provider/test_mock_provider.py b/tests/mock_provider/test_mock_provider.py index f7863c2d..c08d122f 100644 --- a/tests/mock_provider/test_mock_provider.py +++ b/tests/mock_provider/test_mock_provider.py @@ -397,6 +397,85 @@ def test_health_and_catch_all_get(client: TestClient) -> None: assert client.get("/").status_code == 200 +# --- request validation ----------------------------------------------------- + + +def test_malformed_body_returns_an_openai_error_envelope(client: TestClient) -> None: + """A bad request must look like the real API's, not like FastAPI's 422. + + Mid-run, a 422 in FastAPI's own error shape reads as a Honcho bug rather + than a bad request, and no OpenAI client knows how to interpret it. + """ + response = client.post( + "/v1/embeddings", json={"input": "hello", "dimensions": "not-a-number"} + ) + + assert response.status_code == 400 + error = response.json()["error"] + assert error["type"] == "invalid_request_error" + assert error["message"] + assert set(error) == {"message", "type", "param", "code"} + + +def test_boolean_dimensions_is_rejected_not_silently_coerced( + client: TestClient, +) -> None: + """bool is an int subclass, so `true` would otherwise mean 1 dimension.""" + response = client.post( + "/v1/embeddings", json={"input": "hello", "dimensions": True} + ) + + assert response.status_code == 400 + + +def test_unknown_fields_are_accepted(client: TestClient) -> None: + """Validation must fire on wrong types, never on unrecognised parameters. + + A new upstream parameter should not turn a working setup into a hard + failure, so every model allows extras. + """ + body = _post_chat( + client, + temperature=0.7, + max_completion_tokens=256, + reasoning_effort="minimal", + some_parameter_invented_next_year=True, + ) + assert body["choices"][0]["finish_reason"] == "stop" + + +def test_wrongly_typed_messages_are_rejected(client: TestClient) -> None: + response = client.post( + "/v1/chat/completions", json={"model": "m", "messages": "not-a-list"} + ) + + assert response.status_code == 400 + assert response.json()["error"]["type"] == "invalid_request_error" + + +@pytest.mark.parametrize( + ("payload", "expected"), + [ + ({"input": "solo"}, 1), + ({"input": ["a", "b", "c"]}, 3), + ({"input": [1, 2, 3]}, 1), + ({"input": [[1, 2], [3, 4]]}, 2), + ({"input": None}, 0), + ], + ids=["string", "list-of-strings", "token-array", "token-arrays", "null"], +) +def test_every_documented_input_shape_is_accepted( + client: TestClient, payload: dict[str, Any], expected: int +) -> None: + """A flat int list is one tokenized input, not many single-token ones.""" + response = client.post( + "/v1/embeddings", json={**payload, "encoding_format": "float"} + ) + + assert response.status_code == 200, response.text + assert len(response.json()["data"]) == expected + + # --- streaming --------------------------------------------------------------