diff --git a/SKILL.md b/SKILL.md index 61a2794..bc0044e 100644 --- a/SKILL.md +++ b/SKILL.md @@ -63,8 +63,10 @@ api_key_env, dialect: auto|vllm|llamacpp|generic, timeout_s, extra_body}`, servers that take a switch there rather than as a template argument. The `generic` dialect sends no template arguments, so on Ollama's `/v1` route a thinking model needs `extra_body: {reasoning_effort: none}` (`think: - false` is ignored there). A key jevify already sets for the readout - (`max_tokens`, `logprobs`, ...) is refused, not overridden. + false` is ignored there). A key jevify itself sets (`max_tokens`, + `logprobs`, `logit_bias`, ...) is refused when the backend is built, before + any request, so probe, ask, eval and serve all fail the same way. The + fields also reach the tokenize and template routes the probe uses. - `template.mode: raw` sends a completion prompt you write with the model's own control tokens; use it when the chat template would not put the model in its trained regime (the Qwen3-VL reranker recipe is the example). diff --git a/docs/adr/0004-recipes.md b/docs/adr/0004-recipes.md index ea0013d..dae31f8 100644 --- a/docs/adr/0004-recipes.md +++ b/docs/adr/0004-recipes.md @@ -32,7 +32,7 @@ hash is the recipe's identity in every answer and every scorecard. | Section | Holds | |---|---| | `model` | Identifier as the backend knows it, revision when known, the adapter kind, and for endpoints the dialect the probe detected. | -| `endpoint` | For endpoint-backed kinds: base URL, the environment variable holding an API key, the dialect, the timeout, and `extra_body`, server-specific fields merged into the top level of every request (never replacing a field the readout sets; omitted from the hash when empty, so older recipes keep theirs). | +| `endpoint` | For endpoint-backed kinds: base URL, the environment variable holding an API key, the dialect, the timeout, and `extra_body`, server-specific fields merged into the top level of every request (a field jevify itself sets is refused when the backend is built, from one registry in the endpoint client; omitted from the hash when empty, so older recipes keep theirs). | | `template` | For endpoints: how state, instructions and options are rendered into messages or a raw prompt; where the question goes; the answer-slot suffix; template arguments such as thinking off; whether assistant prefill is used. For encoders: the sequence layout and marker scheme. | | `answers` | The answer tokens for `noul`, the identifier scheme for `choice`, the level tokens for `score`, each with the token ids the probe verified. | | `readout` | The rung of the ladder to use or `auto`; the permutation policy and call count for `choice`. | diff --git a/jevify/adapters/endpoint/client.py b/jevify/adapters/endpoint/client.py index f6a0b3d..dd78bef 100644 --- a/jevify/adapters/endpoint/client.py +++ b/jevify/adapters/endpoint/client.py @@ -18,6 +18,55 @@ ) +# Every top-level request field jevify itself sets: the readout requests of every dialect +# and rung, the probe's measurements, the tokenize and template routes, and the rerank and +# embeddings bodies. `endpoint.extra_body` may not name one of these; the client refuses +# when it is built, before any request, so probe, ask, eval and serve all fail the same way. +# Guarded by tests/guards/test_request_fields.py, which records every field the adapters +# send to the fakes and fails naming any that is missing here. +RESERVED_REQUEST_FIELDS: frozenset[str] = frozenset( + { + # the readout request (dialects.base_request) and prefill + "model", + "messages", + "prompt", + "max_tokens", + "n_predict", + "temperature", + "stream", + "logprobs", + "continue_final_message", + "add_generation_prompt", + "chat_template_kwargs", + # the rungs and the probe + "top_logprobs", + "logprob_token_ids", + "return_as_token_id", + "logit_bias", + "grammar", + "n_probs", + # llama.cpp neutral samplers and slot pinning + "top_k", + "top_p", + "min_p", + "post_sampling_probs", + "id_slot", + # tokenize and apply-template + "content", + "add_special", + "parse_special", + "add_special_tokens", + "return_token_strs", + # rerank and embeddings + "query", + "documents", + "top_n", + "instruction", + "input", + } +) + + class OpenAICompatibleClient: """HTTP access to one server. `v1/*` for the OpenAI surface, root for server extras.""" @@ -38,6 +87,7 @@ def __init__( self.timeout_s = timeout_s self.max_retries = max_retries self.extra_body = dict(extra_body or {}) + _refuse_reserved(set(self.extra_body) & RESERVED_REQUEST_FIELDS) self._http = http or httpx.AsyncClient(timeout=httpx.Timeout(timeout_s)) def _headers(self) -> dict[str, str]: @@ -81,13 +131,18 @@ async def aclose(self) -> None: await self._http.aclose() -def _merge_extra(body: dict[str, Any], extra: dict[str, Any]) -> dict[str, Any]: - """Add the recipe's extra fields; never let one replace a field the readout set.""" - clash = sorted(set(body) & set(extra)) +def _refuse_reserved(clash: set[str]) -> None: if clash: raise BackendError( - f"endpoint.extra_body may not set {', '.join(clash)}: jevify sets it for the readout" + f"endpoint.extra_body may not set {', '.join(sorted(clash))}: jevify sets it" ) + + +def _merge_extra(body: dict[str, Any], extra: dict[str, Any]) -> dict[str, Any]: + """Add the recipe's extra fields; never let one replace a field jevify set. The registry + check at construction is the rule; this per-request check is the backstop for a field + the registry does not know yet.""" + _refuse_reserved(set(body) & set(extra)) return {**body, **extra} diff --git a/tests/fakes/fake_openai_server.py b/tests/fakes/fake_openai_server.py index a7e409b..8c468de 100644 --- a/tests/fakes/fake_openai_server.py +++ b/tests/fakes/fake_openai_server.py @@ -125,6 +125,8 @@ def __init__(self, behaviour: Behaviour | None = None) -> None: self.behaviour = behaviour or Behaviour() self.tokenizer = FakeTokenizer(self.behaviour.dialect) self.requests: list[dict[str, Any]] = [] + # tokenize and apply-template bodies, kept apart so request counts stay scoring-only + self.template_requests: list[dict[str, Any]] = [] self.responses: list[dict[str, Any]] = [] self.in_flight = 0 self.max_in_flight = 0 @@ -555,6 +557,7 @@ async def tokenize(self, request: Request) -> JSONResponse: if not self.behaviour.supports_tokenize: return JSONResponse({"detail": "Not Found"}, status_code=404) body = await request.json() + self.template_requests.append({"path": "/tokenize", **body}) if self.behaviour.dialect == "llamacpp": ids = self.tokenizer.encode(body["content"]) if body["content"] in self.behaviour.multi_token_answers: @@ -590,4 +593,5 @@ async def tokenize(self, request: Request) -> JSONResponse: async def apply_template(self, request: Request) -> JSONResponse: body = await request.json() + self.template_requests.append({"path": "/apply-template", **body}) return JSONResponse({"prompt": self._rendered_chat(body)}) diff --git a/tests/guards/test_request_fields.py b/tests/guards/test_request_fields.py new file mode 100644 index 0000000..6582f00 --- /dev/null +++ b/tests/guards/test_request_fields.py @@ -0,0 +1,79 @@ +"""Guard: `endpoint.extra_body` may never override a field jevify sets (ADR-0004 D2). + +The client refuses a reserved key when the backend is built, from one registry, +`RESERVED_REQUEST_FIELDS`. A registry kept by hand drifts, so this guard drives +every adapter kind and dialect through the fakes the way an operator does +(probe, write the probe in, warm, evaluate), collects every top-level field that +reached a server, and fails naming any field the registry lacks. +""" + +from __future__ import annotations + +import asyncio + +from jevify.adapters.endpoint.client import RESERVED_REQUEST_FIELDS +from jevify.compose import build_backend +from jevify.domain.questions import ChoiceQuestion, NoulQuestion, Option, ScoreQuestion, State +from jevify.recipes import apply_probe, load_recipe +from tests.fakes.fake_openai_server import Behaviour + +STATE = State.from_jev("The customer wrote: I was charged twice and I am furious.") +CHOICE = ChoiceQuestion( + id="dept", + instructions="Which team handles this?", + options=(Option("billing", "invoices and refunds"), Option("support", None)), +) +SCORE = ScoreQuestion(id="anger", instructions="How angry?", levels=("calm", "annoyed", "furious")) +NOUL = NoulQuestion(id="cancel", instructions="The customer threatens to cancel.") +ALL = [CHOICE, SCORE, NOUL] + +# Five answer tokens, all inside the fake's top-5 cap, so every readout succeeds. +SCORES = {" yes": 0.0, " no": -1.0, " A": -0.5, " B": -1.2, " C": -1.5} +RAW_SCORES = {"yes": 0.0, "no": -1.0, "A": -0.5, "B": -1.2, "C": -1.5} + +# Every adapter kind and every dialect, with the question types each kind answers. The +# probe also issues the request of every rung it measures. +RUNS = [ + (Behaviour(scores=SCORES), "fake-vllm.yaml", "auto", ALL), + (Behaviour(dialect="llamacpp", grammar=True, scores=RAW_SCORES), "fake-raw.yaml", "auto", ALL), + ( + Behaviour(dialect="generic", thinking_kwarg=None, thinking_default_on=False, scores=SCORES), + "fake-vllm.yaml", + "generic", + ALL, + ), + ( + Behaviour(dialect="llamacpp", rerank_scores={"x": 0.5}), + "fake-rerank.yaml", + "auto", + [CHOICE, NOUL], + ), + (Behaviour(embeddings={"x": [1.0, 0.0]}), "fake-embedding.yaml", "auto", [CHOICE]), +] + + +def _sent(server) -> set[str]: + requests = server.requests + server.template_requests + return {key for req in requests for key in req if key != "path"} + + +def test_every_field_the_adapters_send_is_in_the_reserved_registry(fixtures, fake_server_factory): + unregistered: dict[str, set[str]] = {} + for behaviour, recipe_name, dialect, questions in RUNS: + server, http = fake_server_factory(behaviour) + recipe = load_recipe(fixtures / "recipes" / recipe_name) + recipe = recipe.model_copy( + update={"endpoint": recipe.endpoint.model_copy(update={"dialect": dialect})} + ) + capabilities = asyncio.run(build_backend(recipe, http=http).probe()) + backend = build_backend(apply_probe(recipe, capabilities), http=http) + handle = asyncio.run(backend.warm(STATE)) + answers = asyncio.run(backend.evaluate(handle, questions)) + assert len(answers) == len(questions) + missing = _sent(server) - RESERVED_REQUEST_FIELDS + if missing: + unregistered[f"{recipe_name} ({dialect})"] = missing + assert not unregistered, ( + f"request fields not in RESERVED_REQUEST_FIELDS (jevify/adapters/endpoint/client.py): " + f"{unregistered}. Add them there so endpoint.extra_body cannot override them." + ) diff --git a/tests/test_endpoint_adapter.py b/tests/test_endpoint_adapter.py index bd943a2..9fb451d 100644 --- a/tests/test_endpoint_adapter.py +++ b/tests/test_endpoint_adapter.py @@ -231,12 +231,22 @@ def test_extra_body_reaches_every_request_on_the_generic_dialect(fixtures, fake_ assert all("chat_template_kwargs" not in req for req in server.requests) -def test_extra_body_cannot_override_a_readout_field(fixtures, fake_server_factory): +@pytest.mark.parametrize( + "key", + [ + "max_tokens", # set on every readout request + "logit_bias", # set only by the equal-bias rung and the probe: warm would have passed + ], +) +def test_extra_body_naming_a_field_jevify_sets_is_refused_before_any_request( + fixtures, fake_server_factory, key +): + # The refusal is decided from the recipe alone, so it is the same for probe, ask, eval and + # serve, rather than surfacing on whichever request happens to set the key first. server, http = fake_server_factory(Behaviour(scores={" yes": 0.0, " no": -1.0})) recipe = load_recipe(fixtures / "recipes" / "fake-vllm.yaml") - recipe = _with_endpoint(recipe, extra_body={"max_tokens": 64}) - backend = build_backend(recipe, http=http) + recipe = _with_endpoint(recipe, extra_body={key: 64}) - with pytest.raises(BackendError, match="max_tokens"): - run(backend.warm(STATE)) + with pytest.raises(BackendError, match=key): + build_backend(recipe, http=http) assert server.requests == [] # refused before anything was sent