From 6d3c2be52ca1bdb968bd126eebb57b83546d6e86 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 4 Sep 2026 17:13:36 -0700 Subject: [PATCH 1/5] Stop reading embedding_types as a vocabulary check Proposal 0122 tightens retrieval section 8.4: the malformed test on a merge-extra is structural, never a vocabulary check. A well-typed string the provider does not recognize merges, including the empty string, and the provider rejects it if unsupported. The Cohere gate carried an `and t` truthiness clause that dropped the whole list to ["float"] on an empty element. Its own comment already said malformation was structural only, so the code and the comment disagreed. Fixture 053 gains a case pinning ["banana", ""]. The structural arm is unchanged: a non-string element still drops the whole list with no partial salvage. This was raised as a spec question rather than changed unilaterally, and the ruling went the way the comment described. --- .../retrieval/providers/cohere.py | 21 ++++++------ tests/unit/test_retrieval_provider.py | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/openarmature/retrieval/providers/cohere.py b/src/openarmature/retrieval/providers/cohere.py index eb853e7..26623f3 100644 --- a/src/openarmature/retrieval/providers/cohere.py +++ b/src/openarmature/retrieval/providers/cohere.py @@ -701,23 +701,22 @@ def _build_request_body( # The wire is order-insensitive here, so none of this carries meaning # beyond reproducibility. # - # A MALFORMED merge-extra -- not a list, or a list with any non-string / - # empty element -- is treated as ABSENT and the mapping sends only the - # mandatory ["float"]. All-or-nothing (no partial salvage), and no raise - # or diagnostic: 0113 (general §6 merge arm, inherited by §8.4) requires - # exactly this, as the request-side counterpart of §7's + # A MALFORMED merge-extra -- not a list, or a list holding a non-string + # element -- is treated as ABSENT and only the mandatory ["float"] is + # sent. All-or-nothing, with no raise or diagnostic: 0113's general §6 + # merge arm requires that, as the request-side counterpart of §7's # malformed-ancillary-is-not-reported rule. Salvaging the valid entries - # would send a precision set the caller never wrote; failing loud would - # make a malformed optional extra call-fatal on a valid request. - # Malformation is STRUCTURAL only: a well-typed but - # provider-unrecognized precision string is NOT malformed -- it merges, - # and the provider rejects it if unsupported. + # would send a precision set the caller never wrote. + # + # Malformation is STRUCTURAL, never a VOCABULARY check (0122 §8.4): a + # well-typed string the provider does not recognize merges, INCLUDING + # the empty string, and the provider rejects it if unsupported. caller_types = request_extras.get("embedding_types") embedding_types = ["float"] if ( isinstance(caller_types, list) and caller_types - and all(isinstance(t, str) and t for t in cast("list[object]", caller_types)) + and all(isinstance(t, str) for t in cast("list[object]", caller_types)) ): for precision in cast("list[str]", caller_types): if precision not in embedding_types: diff --git a/tests/unit/test_retrieval_provider.py b/tests/unit/test_retrieval_provider.py index fa7237e..dc8fb11 100644 --- a/tests/unit/test_retrieval_provider.py +++ b/tests/unit/test_retrieval_provider.py @@ -2866,3 +2866,35 @@ def handler(req: httpx.Request) -> httpx.Response: await provider.rerank("q", ["a", "b"], config=RerankRuntimeConfig.model_validate({"top_n": 1})) await provider.aclose() assert captured[0]["top_n"] == 1 + + +async def test_cohere_embed_unrecognized_precision_strings_merge_rather_than_malform() -> None: + # §8.4 as 0122 tightens it: the malformed test is STRUCTURAL, never a + # VOCABULARY check. A well-typed string the provider does not recognize + # merges, and the provider rejects it if unsupported. + # + # The empty string is the element an implementation reading "not a precision + # string" as "not one of the known names" gets wrong, and this mapping did: + # the gate carried an `and t` truthiness clause that dropped the whole list + # to ["float"]. Fixture 053 case 3 pins it. + captured: list[dict[str, Any]] = [] + + def handler(req: httpx.Request) -> httpx.Response: + captured.append(json.loads(req.content)) + return httpx.Response(200, json=_cohere_embed_body(id="c", vectors=[[0.1, 0.2]], input_tokens=3)) + + provider = _cohere_embed_provider(handler) + cfg = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["banana", ""]}) + await provider.embed(["x"], config=cfg) + assert captured[0]["embedding_types"] == ["float", "banana", ""], ( + "an unrecognized or empty precision string must merge, not read as malformed" + ) + + # The structural arm is unchanged: a non-string element is still malformed, + # and the whole list is dropped rather than partially salvaged. + captured.clear() + cfg_mixed = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["int8", 7]}) + await provider.embed(["x"], config=cfg_mixed) + assert captured[0]["embedding_types"] == ["float"], ( + "a non-string element must still drop the whole list, with no partial salvage" + ) From 08e37ca0d1f12969c521343d8d283719091bb73b Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sun, 6 Sep 2026 14:14:25 -0700 Subject: [PATCH 2/5] Move undeclared provider fields into an extras container Proposal 0122 settles the shape of the extras surface: undeclared fields live in a container on the config record that is separately addressable from the declared ones, and its name is normative. We had the flat reading. RuntimeConfig, EmbeddingRuntimeConfig and RerankRuntimeConfig accepted undeclared names as attributes on the record, which meant a key whose name matched a declared field bound the field instead of landing in extras. That made one arm of 0108's managed-field collision rule unreachable, and we reported it as such. 0122 rules the other way, so the arm is real and the reading was the defect. Breaking in the pre-1.0 sense: an undeclared name passed flat now raises, and the same call is written with extras={...}. Declared fields are unchanged. from_partial still only drops None-valued entries and does not route undeclared names, so there is one spelling rather than two. The conformance fixtures already nested their config.extras sub-block and four harnesses were flattening it to match our model. They now pass it through, which is the change that makes the fixture and the code agree about what the fixture always said. --- CHANGELOG.md | 1 + docs/concepts/llms.md | 13 ++- src/openarmature/llm/providers/openai.py | 6 +- src/openarmature/llm/response.py | 27 +++--- .../prompts/backends/filesystem.py | 20 ++--- .../retrieval/providers/cohere.py | 4 +- src/openarmature/retrieval/providers/jina.py | 4 +- .../retrieval/providers/openai.py | 2 +- src/openarmature/retrieval/providers/tei.py | 4 +- src/openarmature/retrieval/response.py | 14 ++- tests/conformance/test_llm_provider.py | 13 +-- tests/conformance/test_observability.py | 3 +- tests/conformance/test_prompt_management.py | 33 ++++--- tests/conformance/test_retrieval_provider.py | 14 +-- tests/unit/test_llm_provider.py | 78 +++++++++++++--- tests/unit/test_prompts.py | 4 +- tests/unit/test_retrieval_provider.py | 90 ++++++++++++------- tests/unit/test_structured_output.py | 2 +- 18 files changed, 210 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 485de4e..caf67b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Changed +- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Spec v0.117.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixtures 054 / 055 / 056 ride the pin bump. - **Managed wire fields now reject a conflicting extras key instead of silently losing it** (proposals 0105 + 0108, llm-provider §6, spec v0.100.0 / v0.103.0). **Breaking for a managed-key collision only.** A caller's undeclared extras key (`RuntimeConfig` / `EmbeddingRuntimeConfig` accepting extra fields) is forwarded to the wire body untouched, except when it names a field the mapping *manages*: one it sets for its own correctness (0105), or produces as the wire realization of a declared config field (0108). On such a collision the field's shape now decides. An additive list field (`stop` from `stop_sequences`; `embedding_types`) **merges** the caller's value(s) onto the managed value(s), managed-first, de-duplicated. A non-additive scalar or object (`model`, `messages`, `truncate` / `truncation`, `dimensions` / `output_dimension`, `input_type`, `response_format`, Jina `task`, …) takes a value **equal** to the managed one as a no-op and **rejects a conflicting** one pre-send with `ProviderInvalidRequest`. Previously such a collision was silently dropped (the OpenAI llm mapping used `setdefault`) or silently overrode the managed value (the retrieval mappings spread extras first), either of which could re-route the model, defeat a fail-loud `truncate` flag, or break structured-output validation. A field the mapping does not manage keeps untouched pass-through, and a conditionally-managed field is only managed while produced, so the escape hatches hold: an extras `response_format` on a free-form or prompt-augmentation-fallback call rides untouched (0105 §3.5, previously stripped on the fallback path), and an extras Jina `task` with no `input_type` rides untouched (the model-specific-task escape hatch). The rule spans one OpenAI llm mapping and seven retrieval mappings via a shared resolver (`apply_managed_extras`). Spec v0.100.0 / v0.103.0 are beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the reject / merge fixtures ride the pin bump. - **Cohere `/v2/embed` recognizes `classification` and `clustering`** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). **Breaking for these two values.** `EmbeddingRuntimeConfig.input_type` is an extensible string, and §2 names `classification` and `clustering` as well-known values a mapping may recognize when its backend supports them. Cohere's does, so the mapping now identity-maps both onto the wire instead of rejecting them. Previously either value raised `ProviderInvalidRequest` before the request was sent, so a caller who relied on that rejection as a guard (catching it to fall back to `document`, say) silently changes behavior. `query` / `document` / absent / unrecognized are all unchanged, and `image` stays out: it names an input modality rather than a purpose for embedded text, and `embed()` consumes strings. The widening is deliberately per-mapping and not portable. Jina keeps its closed `{query, document}` set, because its `task` support varies by model version (v3 accepts `classification` but not `clustering`, v4 neither, v5 both) and a provider is bound to a model identifier with no capability registry to consult, so that mapping cannot promise the values and declines them pre-send rather than letting the wire reject them later. Spec v0.94.0 is beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixture 033's new cases ride the pin bump. - **Cohere `/v2/embed` `embedding_types` merge is now deterministic** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). The mapping manages `embedding_types` as an explicit exception to untouched extras pass-through, because it must request `"float"` for its own response consumer (it reads `embeddings.float`). A caller-supplied `embedding_types` is merged with that mandatory `"float"` rather than replacing it, which was already the behavior; an override that dropped `float` would strip the key the mapping itself reads and fail the call. What changes is the shape of the merged list, which 0099 pins so the outbound body is reproducible and exact-match assertable: `"float"` first, then the caller's precisions in the order supplied, de-duplicated with the first occurrence winning. Previously the caller's precisions came first with `"float"` appended, and a repeated precision was sent twice, so `["int8"]` now yields `["float", "int8"]` rather than `["int8", "float"]`, and `["int8", "uint8", "int8"]` yields `["float", "int8", "uint8"]` rather than passing the duplicate through. The wire is order-insensitive here, so no request semantics change; callers still read their extra precisions off the verbatim response on `raw`. A malformed or empty extra still falls back to `["float"]`. diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index bb76e1a..10b3d8a 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -641,12 +641,17 @@ you reach a backend-specific knob the portable config does not model, for example a vLLM `guided_decoding`: ```python -config = RuntimeConfig.model_validate({ - "temperature": 0.2, - "guided_decoding": {"grammar": "..."}, # forwarded as-is -}) +config = RuntimeConfig( + temperature=0.2, + extras={"guided_decoding": {"grammar": "..."}}, # forwarded as-is +) ``` +Undeclared knobs go in `extras`, a container separate from the declared +fields. That separation is what lets you set a declared field and an +extras key of the same name in one call, which is how a provider-specific +override of a field OA already models is expressed. + The value itself is never translated or renamed. One caveat for byte-level consumers: the OpenAI Chat Completions mapping canonicalizes the request body for reproducibility, so a dict-valued extra keeps its diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 0fee562..1e8b7d0 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -1182,8 +1182,8 @@ def _build_request_body( # mapping actually produced it (present in the body), which realizes the # "while producing it" semantics for the declared-field realizations and # the conditionally-managed response_format. - if config is not None and config.model_extra: - extras = {k: _canonicalize_dict_keys(v) for k, v in config.model_extra.items()} + if config is not None and config.extras: + extras = {k: _canonicalize_dict_keys(v) for k, v in config.extras.items()} managed: dict[str, ManagedArm] = { key: arm for key, arm in _OPENAI_MANAGED_ARMS.items() @@ -1968,7 +1968,7 @@ def _request_extras_from_config(config: RuntimeConfig | None) -> dict[str, Any]: dict; empty when no extras are set or when ``config`` is None.""" if config is None: return {} - return dict(config.model_extra or {}) + return dict(config.extras) __all__ = [ diff --git a/src/openarmature/llm/response.py b/src/openarmature/llm/response.py index d6e36c0..dc2051a 100644 --- a/src/openarmature/llm/response.py +++ b/src/openarmature/llm/response.py @@ -126,15 +126,21 @@ class Response(BaseModel): response_model: str | None = None -# Spec §6 declared-field surface: seven optional fields. Undeclared -# fields supplied by callers MUST be forwarded to the wire body -# untouched (extras pass-through); declared fields with value ``None`` -# MUST be omitted from the wire body (null-skip). Both rules are -# enforced by the §8 wire-format mapping, not by RuntimeConfig itself. +# Spec §6 declared-field surface: seven optional fields. Undeclared fields go in +# the `extras` container and are forwarded to the wire body untouched; declared +# fields with value ``None`` are omitted (null-skip). Both rules are enforced by +# the §8 wire-format mapping, not by RuntimeConfig itself. class RuntimeConfig(BaseModel): """Per-call sampling parameters and budget hints.""" - model_config = ConfigDict(extra="allow") + # §6 (0122): undeclared fields live in a container that is separately + # addressable from the declared ones, so a caller can set a declared field + # AND an extras key of the same name in one call. That collision is what + # 0108 clause (b) governs, and it is unreachable if undeclared keys land + # flat on the record. The container's name is normative. + model_config = ConfigDict(extra="forbid") + + extras: dict[str, Any] = Field(default_factory=dict) temperature: float | None = None max_tokens: int | None = None @@ -156,10 +162,11 @@ class RuntimeConfig(BaseModel): # name; the declared layer matches the cross-vendor norm. stop_sequences: list[str] | None = None - # Pure Python ergonomic, not a spec contract. The wire-layer - # null-skip rule already drops ``None``-valued declared fields, so - # this helper exists solely to let callers splat a dict whose - # entries may be ``None`` without filtering at the call site. + # Pure Python ergonomic, not a spec contract. The wire-layer null-skip rule + # already drops ``None``-valued declared fields, so this exists solely to let + # callers splat a dict whose entries may be ``None`` without filtering at the + # call site. It does NOT route undeclared names: those go in ``extras`` like + # anywhere else, so there is one spelling rather than two. @classmethod def from_partial(cls, **kwargs: Any) -> RuntimeConfig: """Construct a config, dropping kwargs whose value is ``None``. diff --git a/src/openarmature/prompts/backends/filesystem.py b/src/openarmature/prompts/backends/filesystem.py index b4068b5..f38b98d 100644 --- a/src/openarmature/prompts/backends/filesystem.py +++ b/src/openarmature/prompts/backends/filesystem.py @@ -257,19 +257,15 @@ async def fetch( def _sampling_from_dict(data: dict[str, Any]) -> SamplingConfig: - # Top-level `extras` is flattened so caller-supplied vendor knobs - # end up in SamplingConfig's extras-allow bag rather than as a - # single literal `extras` key. Matches the YAML conformance-fixture - # convention from llm-provider/032 + the spec §5 sidecar example. - # `token_budget` (proposal 0083) is a sibling sub-object read by - # `_token_budget_from_dict`, not a sampling field, so it is excluded - # here alongside `extras`. - flat: dict[str, Any] = {k: v for k, v in data.items() if k not in ("extras", "token_budget")} + # The sidecar's `extras` sub-object maps onto the config's own extras + # container (0122). `token_budget` (0083) is a sibling sub-object read by + # `_token_budget_from_dict`, not a sampling field, so it is excluded too. + declared: dict[str, Any] = {k: v for k, v in data.items() if k not in ("extras", "token_budget")} extras = data.get("extras") - if isinstance(extras, dict): - for k, v in cast(dict[str, Any], extras).items(): - flat.setdefault(k, v) - return SamplingConfig(**flat) + return SamplingConfig( + **declared, + extras=dict(cast(dict[str, Any], extras)) if isinstance(extras, dict) else {}, + ) def _token_budget_from_dict(data: dict[str, Any]) -> TokenBudget | None: diff --git a/src/openarmature/retrieval/providers/cohere.py b/src/openarmature/retrieval/providers/cohere.py index 26623f3..45a5714 100644 --- a/src/openarmature/retrieval/providers/cohere.py +++ b/src/openarmature/retrieval/providers/cohere.py @@ -306,7 +306,7 @@ async def rerank( active_prompt_group = current_prompt_group() documents_list = list(documents) request_params = _request_params_from_config(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} adapter_start = time.perf_counter() try: validate_rerank_input(query, documents_list, top_k) @@ -576,7 +576,7 @@ async def embed( active_prompt_group = current_prompt_group() input_strings = list(input) request_params = _embedding_request_params(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} input_type = config.input_type if config is not None else None dimensions = config.dimensions if config is not None else None adapter_start = time.perf_counter() diff --git a/src/openarmature/retrieval/providers/jina.py b/src/openarmature/retrieval/providers/jina.py index c6295a4..387a736 100644 --- a/src/openarmature/retrieval/providers/jina.py +++ b/src/openarmature/retrieval/providers/jina.py @@ -281,7 +281,7 @@ async def embed( active_prompt_group = current_prompt_group() input_strings = list(input) request_params = _embedding_request_params(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} input_type = config.input_type if config is not None else None dimensions = config.dimensions if config is not None else None adapter_start = time.perf_counter() @@ -539,7 +539,7 @@ async def rerank( active_prompt_group = current_prompt_group() documents_list = list(documents) request_params = _rerank_request_params(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} return_documents = config.return_documents if config is not None else False adapter_start = time.perf_counter() try: diff --git a/src/openarmature/retrieval/providers/openai.py b/src/openarmature/retrieval/providers/openai.py index ddd7904..4c1b95f 100644 --- a/src/openarmature/retrieval/providers/openai.py +++ b/src/openarmature/retrieval/providers/openai.py @@ -303,7 +303,7 @@ async def embed( active_prompt_group = current_prompt_group() input_strings = list(input) request_params = _request_params_from_config(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} input_type = config.input_type if config is not None else None dimensions = config.dimensions if config is not None else None adapter_start = time.perf_counter() diff --git a/src/openarmature/retrieval/providers/tei.py b/src/openarmature/retrieval/providers/tei.py index c5ef937..554b1d6 100644 --- a/src/openarmature/retrieval/providers/tei.py +++ b/src/openarmature/retrieval/providers/tei.py @@ -235,7 +235,7 @@ async def embed( active_prompt_group = current_prompt_group() input_strings = list(input) request_params = _embedding_request_params(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} input_type = config.input_type if config is not None else None dimensions = config.dimensions if config is not None else None adapter_start = time.perf_counter() @@ -509,7 +509,7 @@ async def rerank( active_prompt_group = current_prompt_group() documents_list = list(documents) request_params = _rerank_request_params(config) - request_extras = dict(config.model_extra or {}) if config is not None else {} + request_extras = dict(config.extras) if config is not None else {} return_documents = config.return_documents if config is not None else False adapter_start = time.perf_counter() try: diff --git a/src/openarmature/retrieval/response.py b/src/openarmature/retrieval/response.py index cb4981c..cd72d68 100644 --- a/src/openarmature/retrieval/response.py +++ b/src/openarmature/retrieval/response.py @@ -93,7 +93,12 @@ class EmbeddingResponse(BaseModel): class EmbeddingRuntimeConfig(BaseModel): """Per-call embedding request parameters.""" - model_config = ConfigDict(extra="allow") + # §6 (0122): see RuntimeConfig. Undeclared fields live in `extras`, which is + # separately addressable so a declared field and a same-named extras key can + # both be set; the name is normative. + model_config = ConfigDict(extra="forbid") + + extras: dict[str, Any] = Field(default_factory=dict) dimensions: int | None = None input_type: str | None = None @@ -181,7 +186,12 @@ class RerankResponse(BaseModel): class RerankRuntimeConfig(BaseModel): """Per-call rerank request parameters.""" - model_config = ConfigDict(extra="allow") + # §6 (0122): see RuntimeConfig. Undeclared fields live in `extras`, which is + # separately addressable so a declared field and a same-named extras key can + # both be set; the name is normative. + model_config = ConfigDict(extra="forbid") + + extras: dict[str, Any] = Field(default_factory=dict) return_documents: bool = False diff --git a/tests/conformance/test_llm_provider.py b/tests/conformance/test_llm_provider.py index f6cb187..540d4c3 100644 --- a/tests/conformance/test_llm_provider.py +++ b/tests/conformance/test_llm_provider.py @@ -638,16 +638,11 @@ async def _run_one_call( response_schema = call_spec.get("response_schema") retry_mw_cfg = cast("Mapping[str, Any] | None", call_spec.get("retry_middleware")) config_block = call_spec.get("config") - # YAML convention: `config.extras: {...}` is the sub-block for - # undeclared (provider-specific) RuntimeConfig fields. Flatten it - # into the kwargs splat so the extras land in RuntimeConfig's - # model_extra rather than as a single `extras` key. + # The fixture's `config.extras: {...}` sub-block maps straight onto the + # config's `extras` container (0122), so it passes through rather than being + # flattened into the declared-field kwargs. if config_block: - block = dict(cast("Mapping[str, Any]", config_block)) - extras_block = cast("Mapping[str, Any] | None", block.pop("extras", None)) - if extras_block: - block.update(extras_block) - config = RuntimeConfig(**block) + config = RuntimeConfig(**cast("Mapping[str, Any]", config_block)) else: config = None diff --git a/tests/conformance/test_observability.py b/tests/conformance/test_observability.py index 489ab43..cfe5d2d 100644 --- a/tests/conformance/test_observability.py +++ b/tests/conformance/test_observability.py @@ -6412,8 +6412,7 @@ def _build_runtime_config(config_spec: Mapping[str, Any] | None) -> RuntimeConfi "stop_sequences", } } - kwargs.update(extras) - return RuntimeConfig(**kwargs) + return RuntimeConfig(**kwargs, extras=dict(extras)) def _require_text_content(role: object, content: object) -> str: diff --git a/tests/conformance/test_prompt_management.py b/tests/conformance/test_prompt_management.py index 8699425..5f8ecb7 100644 --- a/tests/conformance/test_prompt_management.py +++ b/tests/conformance/test_prompt_management.py @@ -215,17 +215,16 @@ def __init__(self, spec: FixtureBackendSpec) -> None: now = datetime.now(UTC) for ps in spec.prompts: # Sampling sub-record (fixture 013): flatten the fixture's - # `extras:` sub-block into top-level kwargs so caller- - # supplied vendor knobs land in SamplingConfig's extras- - # allow bag rather than as a literal `extras` key. + # The `extras:` sub-block maps onto the config's own extras + # container (0122). sampling: SamplingConfig | None = None if ps.sampling is not None: - flat: dict[str, Any] = {k: v for k, v in ps.sampling.items() if k != "extras"} + declared: dict[str, Any] = {k: v for k, v in ps.sampling.items() if k != "extras"} extras = ps.sampling.get("extras") - if isinstance(extras, dict): - for k, v in cast(dict[str, Any], extras).items(): - flat.setdefault(k, v) - sampling = SamplingConfig(**flat) + sampling = SamplingConfig( + **declared, + extras=dict(cast(dict[str, Any], extras)) if isinstance(extras, dict) else {}, + ) if ps.chat_template is not None: # Proposal 0046: chat-prompt variant. Map fixture # YAML segment dicts to OA ChatSegment entries via @@ -574,16 +573,14 @@ def _assert_capture_attrs(capture_name: str, actual: Any, expected: dict[str, An if key == "sampling": actual_sampling = getattr(actual, "sampling", None) assert actual_sampling is not None, f"{capture_name}.sampling: expected present, got None" - # Spec sidecar convention nests vendor extras under - # `extras:`; SamplingConfig.model_dump() flattens them to - # the top level (extra="allow"). Normalize the expected - # shape before equality compare. - expected_flat = {k: v for k, v in expected_value.items() if k != "extras"} - if isinstance(expected_value.get("extras"), dict): - expected_flat.update(expected_value["extras"]) - actual_flat = actual_sampling.model_dump(exclude_none=True) - assert actual_flat == expected_flat, ( - f"{capture_name}.sampling: expected {expected_flat!r}, got {actual_flat!r}" + # The sidecar's `extras:` sub-block and the config's `extras` + # container are the same shape (0122), so this compares directly. + # An absent extras block means an empty container, not a missing key. + expected_shape = dict(expected_value) + expected_shape.setdefault("extras", {}) + actual_shape = actual_sampling.model_dump(exclude_none=True) + assert actual_shape == expected_shape, ( + f"{capture_name}.sampling: expected {expected_shape!r}, got {actual_shape!r}" ) continue actual_value = getattr(actual, key) diff --git a/tests/conformance/test_retrieval_provider.py b/tests/conformance/test_retrieval_provider.py index 57e52e7..7ee4766 100644 --- a/tests/conformance/test_retrieval_provider.py +++ b/tests/conformance/test_retrieval_provider.py @@ -240,11 +240,8 @@ def _build_provider( def _build_config(config_block: Mapping[str, Any] | None) -> EmbeddingRuntimeConfig | None: if not config_block: return None - block = dict(config_block) - extras = cast("Mapping[str, Any] | None", block.pop("extras", None)) - if extras: - block.update(extras) - return EmbeddingRuntimeConfig(**block) + # `config.extras` maps onto the config's own extras container (0122). + return EmbeddingRuntimeConfig(**config_block) def _build_rerank_provider( @@ -312,11 +309,8 @@ def _build_rerank_provider( def _build_rerank_config(config_block: Mapping[str, Any] | None) -> RerankRuntimeConfig | None: if not config_block: return None - block = dict(config_block) - extras = cast("Mapping[str, Any] | None", block.pop("extras", None)) - if extras: - block.update(extras) - return RerankRuntimeConfig(**block) + # `config.extras` maps onto the config's own extras container (0122). + return RerankRuntimeConfig(**config_block) def _assert_embedding_response( diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 0a1617f..6f75b98 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -916,12 +916,18 @@ def test_runtime_config_from_partial_drops_nones() -> None: def test_runtime_config_from_partial_forwards_extras() -> None: + # `from_partial` drops None-valued entries; it does not route undeclared + # names. Extras reach the container the same way they do everywhere else, + # so there is one spelling rather than two (0122). from openarmature.llm import RuntimeConfig - config = RuntimeConfig.from_partial(temperature=0.5, repetition_penalty=1.05, top_k=None) + config = RuntimeConfig.from_partial(temperature=0.5, extras={"repetition_penalty": 1.05}, top_k=None) assert config.temperature == 0.5 - assert (config.model_extra or {}) == {"repetition_penalty": 1.05} + assert config.extras == {"repetition_penalty": 1.05} + # The None-dropping is the whole job: `top_k=None` is dropped before + # construction rather than rejected as an undeclared field. + assert not hasattr(config, "top_k") def test_runtime_config_from_partial_empty() -> None: @@ -2550,7 +2556,7 @@ async def test_llm_completion_event_request_extras_flows_through() -> None: # so pyright doesn't flag the undeclared kwarg. await provider.complete( [UserMessage(content="hi")], - config=RuntimeConfig.model_validate({"guided_decoding": {"choice": ["a", "b"]}}), + config=RuntimeConfig.model_validate({"extras": {"guided_decoding": {"choice": ["a", "b"]}}}), ) finally: await provider.aclose() @@ -2938,10 +2944,10 @@ def _handler(req: httpx.Request) -> httpx.Response: ) config_a = RuntimeConfig.model_validate( - {"guided_decoding": {"choice": ["a", "b"], "backend": "outlines"}} + {"extras": {"guided_decoding": {"choice": ["a", "b"], "backend": "outlines"}}} ) config_b = RuntimeConfig.model_validate( - {"guided_decoding": {"backend": "outlines", "choice": ["a", "b"]}} + {"extras": {"guided_decoding": {"backend": "outlines", "choice": ["a", "b"]}}} ) provider = OpenAIProvider( base_url="http://test", model="m", api_key="k", transport=httpx.MockTransport(_handler) @@ -3288,7 +3294,7 @@ def never(_req: httpx.Request) -> httpx.Response: # pragma: no cover provider = _collision_provider(never) with pytest.raises(ProviderInvalidRequest): await provider.complete( - [UserMessage(content="hi")], config=RuntimeConfig.model_validate({"model": "other"}) + [UserMessage(content="hi")], config=RuntimeConfig.model_validate({"extras": {"model": "other"}}) ) await provider.aclose() @@ -3305,7 +3311,7 @@ def never(_req: httpx.Request) -> httpx.Response: # pragma: no cover tool = {"type": "function", "function": {"name": "x"}} for extra in ({"tools": [tool]}, {"tool_choice": "auto"}): with pytest.raises(ProviderInvalidRequest): - await provider.complete([UserMessage(content="hi")], config=RuntimeConfig.model_validate(extra)) + await provider.complete([UserMessage(content="hi")], config=RuntimeConfig(extras=extra)) await provider.aclose() @@ -3313,7 +3319,9 @@ async def test_llm_matching_structural_extra_is_a_noop() -> None: # An extras model equal to the bound model is a redundant no-op. bodies: list[dict[str, Any]] = [] provider = _collision_provider(_ok_handler(bodies)) - await provider.complete([UserMessage(content="hi")], config=RuntimeConfig.model_validate({"model": "m"})) + await provider.complete( + [UserMessage(content="hi")], config=RuntimeConfig.model_validate({"extras": {"model": "m"}}) + ) await provider.aclose() assert bodies[0]["model"] == "m" @@ -3323,7 +3331,7 @@ async def test_llm_stop_merges_declared_and_extras() -> None: # wire-name extras `stop` MERGE, managed-first, de-duplicated. bodies: list[dict[str, Any]] = [] provider = _collision_provider(_ok_handler(bodies)) - cfg = RuntimeConfig.model_validate({"stop_sequences": ["A"], "stop": ["B", "A"]}) + cfg = RuntimeConfig.model_validate({"stop_sequences": ["A"], "extras": {"stop": ["B", "A"]}}) await provider.complete([UserMessage(content="hi")], config=cfg) await provider.aclose() assert bodies[0]["stop"] == ["A", "B"] @@ -3332,7 +3340,7 @@ async def test_llm_stop_merges_declared_and_extras() -> None: async def test_llm_unmanaged_extra_rides_untouched() -> None: bodies: list[dict[str, Any]] = [] provider = _collision_provider(_ok_handler(bodies)) - cfg = RuntimeConfig.model_validate({"guided_decoding": {"grammar": "g"}}) + cfg = RuntimeConfig.model_validate({"extras": {"guided_decoding": {"grammar": "g"}}}) await provider.complete([UserMessage(content="hi")], config=cfg) await provider.aclose() assert bodies[0]["guided_decoding"] == {"grammar": "g"} @@ -3345,7 +3353,55 @@ async def test_llm_response_format_rides_untouched_on_free_form_call() -> None: # tests.) bodies: list[dict[str, Any]] = [] provider = _collision_provider(_ok_handler(bodies)) - cfg = RuntimeConfig.model_validate({"response_format": {"type": "text"}}) + cfg = RuntimeConfig.model_validate({"extras": {"response_format": {"type": "text"}}}) await provider.complete([UserMessage(content="hi")], config=cfg) await provider.aclose() assert bodies[0]["response_format"] == {"type": "text"} + + +@pytest.mark.parametrize( + ("factory", "declared"), + [ + ("RuntimeConfig", {"temperature": 0.2}), + ("SamplingConfig", {"temperature": 0.2}), + ("EmbeddingRuntimeConfig", {"input_type": "document"}), + ("RerankRuntimeConfig", {"return_documents": True}), + ], +) +def test_undeclared_fields_must_go_in_the_extras_container(factory: str, declared: dict[str, Any]) -> None: + # 0122 gives the extras surface one shape: a container separately + # addressable from the declared fields. An undeclared name passed flat is + # rejected, so there is one spelling rather than two. + # + # Without this the config classes accept BOTH forms and the flat one silently + # keeps working. Verified by mutation: flipping `extra="forbid"` back to + # `extra="allow"` left the whole suite green before this landed. + from openarmature.llm import RuntimeConfig + from openarmature.prompts import SamplingConfig + from openarmature.retrieval import EmbeddingRuntimeConfig, RerankRuntimeConfig + + cls = { + "RuntimeConfig": RuntimeConfig, + "SamplingConfig": SamplingConfig, + "EmbeddingRuntimeConfig": EmbeddingRuntimeConfig, + "RerankRuntimeConfig": RerankRuntimeConfig, + }[factory] + + # Built through `model_validate` rather than the constructor: `cls` is a + # union of the four config types here, and a checker cannot verify a keyword + # against all of them. + with pytest.raises(ValidationError): + cls.model_validate({**declared, "vendor_specific_knob": 1}) + + # The container is the way through, and the declared field is untouched by it. + config = cls.model_validate({**declared, "extras": {"vendor_specific_knob": 1}}) + assert config.extras == {"vendor_specific_knob": 1} + for name, value in declared.items(): + assert getattr(config, name) == value + + # A same-named key is a legitimate extras key, not a rebinding of the + # declared field. This is the arm the flat reading could not express. + same_name = next(iter(declared)) + collided = cls.model_validate({**declared, "extras": {same_name: "from-extras"}}) + assert getattr(collided, same_name) == declared[same_name] + assert collided.extras == {same_name: "from-extras"} diff --git a/tests/unit/test_prompts.py b/tests/unit/test_prompts.py index 11466c6..b218bb8 100644 --- a/tests/unit/test_prompts.py +++ b/tests/unit/test_prompts.py @@ -627,7 +627,7 @@ async def test_filesystem_backend_per_prompt_sidecar(tmp_path: Path) -> None: assert prompt.sampling.temperature == 0.0 assert prompt.sampling.max_tokens == 256 # Vendor extra rides through the extras-allow bag. - assert (prompt.sampling.model_extra or {}).get("repetition_penalty") == 1.05 + assert prompt.sampling.extras.get("repetition_penalty") == 1.05 async def test_filesystem_backend_unified_sampling(tmp_path: Path) -> None: @@ -678,7 +678,7 @@ async def test_filesystem_backend_token_budget_per_prompt_sidecar(tmp_path: Path # sampling excludes the token_budget sub-object (not a sampling field). assert prompt.sampling is not None assert prompt.sampling.temperature == 0.2 - assert "token_budget" not in (prompt.sampling.model_extra or {}) + assert "token_budget" not in prompt.sampling.extras async def test_filesystem_backend_token_budget_unrecognized_key_is_filtered(tmp_path: Path) -> None: diff --git a/tests/unit/test_retrieval_provider.py b/tests/unit/test_retrieval_provider.py index dc8fb11..b9d5903 100644 --- a/tests/unit/test_retrieval_provider.py +++ b/tests/unit/test_retrieval_provider.py @@ -1093,7 +1093,9 @@ def handler(req: httpx.Request) -> httpx.Response: # return_documents=True is a silent no-op on the Cohere wire (no such field); # max_tokens_per_doc rides the extras pass-through bag (model_validate so # the undeclared extra is accepted, mirroring the conformance config path). - config = RerankRuntimeConfig.model_validate({"return_documents": True, "max_tokens_per_doc": 100}) + config = RerankRuntimeConfig.model_validate( + {"return_documents": True, "extras": {"max_tokens_per_doc": 100}} + ) await provider.rerank("q", ["a", "b", "c"], top_k=2, config=config) body = captured[0] assert body["model"] == "rerank-test" @@ -1708,18 +1710,22 @@ def handler(req: httpx.Request) -> httpx.Response: captured.append(json.loads(req.content)) return httpx.Response(200, json=_cohere_embed_body(id="c", vectors=[[0.1, 0.2]], input_tokens=3)) - cfg_both = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["float", "int8"]}) - cfg_int8 = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["int8"]}) - cfg_float_last = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["int8", "float"]}) - cfg_dupes = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["int8", "uint8", "int8"]}) + cfg_both = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["float", "int8"]}}) + cfg_int8 = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["int8"]}}) + cfg_float_last = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["int8", "float"]}}) + cfg_dupes = EmbeddingRuntimeConfig.model_validate( + {"extras": {"embedding_types": ["int8", "uint8", "int8"]}} + ) # Supplied order is NOT alphabetical here, and "binary" sorts BEFORE # "float". These two cases are what distinguish "in the order supplied" from # a sorted implementation: without them, every list under test is already # ascending and a sorted(set(...)) merge passes the whole suite while # violating both the supplied-order and float-first rules. - cfg_unsorted = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["uint8", "int8"]}) - cfg_pre_float = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["binary", "int8", "binary"]}) - cfg_bad = EmbeddingRuntimeConfig.model_validate({"embedding_types": [{"x": 1}]}) + cfg_unsorted = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["uint8", "int8"]}}) + cfg_pre_float = EmbeddingRuntimeConfig.model_validate( + {"extras": {"embedding_types": ["binary", "int8", "binary"]}} + ) + cfg_bad = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": [{"x": 1}]}}) provider = _cohere_embed_provider(handler) # Caller names float first -> unchanged, and float appears exactly once. await provider.embed(["x"], config=cfg_both) @@ -2698,18 +2704,24 @@ async def test_cohere_embed_conflicting_managed_extra_rejects_pre_send() -> None # mapping's value. output_dimension is the wire realization of the declared # `dimensions`, so an extras output_dimension against a declared dimensions # is a genuine collision -- the wire name differs from the declared name. - # input_type is deliberately NOT tested here: its wire name equals its - # declared name, so a like-named key binds the declared field rather than - # model_extra and can never reach the reject arm as an extra. + # + # The SAME-name arm is reachable too, since 0122: `extras` is a container + # separately addressable from the declared fields, so a key whose name + # matches a declared field is a legitimate extras key rather than binding + # the field. It was unreachable while undeclared keys landed flat on the + # record, which is what made this arm look like dead code. provider = _cohere_embed_provider(_never_called) - configs = ( - {"input_type": "document", "model": "other"}, - {"input_type": "document", "truncate": "END"}, - {"input_type": "document", "dimensions": 512, "output_dimension": 256}, + configs: tuple[tuple[dict[str, Any], dict[str, Any]], ...] = ( + ({"input_type": "document"}, {"model": "other"}), + ({"input_type": "document"}, {"truncate": "END"}), + ({"input_type": "document", "dimensions": 512}, {"output_dimension": 256}), + # Same declared name on both sides: the wire name equals the declared + # name, so this is the arm the flat reading could not express. + ({"input_type": "document"}, {"input_type": "query"}), ) - for raw in configs: + for declared, extras in configs: with pytest.raises(ProviderInvalidRequest): - await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate(raw)) + await provider.embed(["x"], config=EmbeddingRuntimeConfig(**declared, extras=extras)) await provider.aclose() @@ -2722,7 +2734,7 @@ def handler(req: httpx.Request) -> httpx.Response: provider = _cohere_embed_provider(handler) # A matching truncate is a no-op; an unmanaged extra rides untouched. - cfg = EmbeddingRuntimeConfig.model_validate({"truncate": "NONE", "user_tag": "keep"}) + cfg = EmbeddingRuntimeConfig.model_validate({"extras": {"truncate": "NONE", "user_tag": "keep"}}) await provider.embed(["x"], config=cfg) await provider.aclose() assert captured[0]["truncate"] == "NONE" @@ -2732,7 +2744,9 @@ def handler(req: httpx.Request) -> httpx.Response: async def test_openai_embed_conflicting_model_extra_rejects() -> None: provider = _openai_embed_provider(_never_called) with pytest.raises(ProviderInvalidRequest): - await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate({"model": "other"})) + await provider.embed( + ["x"], config=EmbeddingRuntimeConfig.model_validate({"extras": {"model": "other"}}) + ) await provider.aclose() @@ -2753,7 +2767,9 @@ def handler(req: httpx.Request) -> httpx.Response: return _jina_embed_response(req) provider = _jina_embed_provider(handler) - await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate({"task": "text-matching"})) + await provider.embed( + ["x"], config=EmbeddingRuntimeConfig.model_validate({"extras": {"task": "text-matching"}}) + ) await provider.aclose() assert captured[0]["task"] == "text-matching" @@ -2761,7 +2777,9 @@ def handler(req: httpx.Request) -> httpx.Response: async def test_jina_embed_task_conflict_rejects_when_input_type_set() -> None: # input_type set -> task managed -> a conflicting extras task is rejected. provider = _jina_embed_provider(_never_called) - cfg = EmbeddingRuntimeConfig.model_validate({"input_type": "query", "task": "retrieval.passage"}) + cfg = EmbeddingRuntimeConfig.model_validate( + {"input_type": "query", "extras": {"task": "retrieval.passage"}} + ) with pytest.raises(ProviderInvalidRequest): await provider.embed(["x"], config=cfg) await provider.aclose() @@ -2775,7 +2793,9 @@ def handler(req: httpx.Request) -> httpx.Response: return _jina_embed_response(req) provider = _jina_embed_provider(handler) - cfg = EmbeddingRuntimeConfig.model_validate({"input_type": "query", "task": "retrieval.query"}) + cfg = EmbeddingRuntimeConfig.model_validate( + {"input_type": "query", "extras": {"task": "retrieval.query"}} + ) await provider.embed(["x"], config=cfg) await provider.aclose() assert captured[0]["task"] == "retrieval.query" @@ -2786,7 +2806,9 @@ async def test_tei_embed_relied_on_truncate_default_conflict_rejects() -> None: # sending it; a conflicting extras truncate is still rejected (0105 fx 047). provider = _tei_embed_provider(_never_called) with pytest.raises(ProviderInvalidRequest): - await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate({"truncate": True})) + await provider.embed( + ["x"], config=EmbeddingRuntimeConfig.model_validate({"extras": {"truncate": True}}) + ) await provider.aclose() @@ -2800,7 +2822,7 @@ def handler(req: httpx.Request) -> httpx.Response: return httpx.Response(200, json=[[0.1, 0.2]]) provider = _tei_embed_provider(handler) - await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate({"truncate": False})) + await provider.embed(["x"], config=EmbeddingRuntimeConfig.model_validate({"extras": {"truncate": False}})) await provider.aclose() assert "truncate" not in captured[0] @@ -2809,7 +2831,9 @@ async def test_jina_rerank_conflicting_truncation_rejects() -> None: # 0105 fixture 048: the distinct `truncation` name on Jina /v1/rerank. provider = _jina_rerank_provider(_never_called) with pytest.raises(ProviderInvalidRequest): - await provider.rerank("q", ["d"], config=RerankRuntimeConfig.model_validate({"truncation": True})) + await provider.rerank( + "q", ["d"], config=RerankRuntimeConfig.model_validate({"extras": {"truncation": True}}) + ) await provider.aclose() @@ -2821,7 +2845,7 @@ async def test_cohere_rerank_conflicting_top_n_extra_rejects() -> None: provider = _rerank_provider(_never_called) with pytest.raises(ProviderInvalidRequest): await provider.rerank( - "q", ["a", "b"], top_k=2, config=RerankRuntimeConfig.model_validate({"top_n": 1}) + "q", ["a", "b"], top_k=2, config=RerankRuntimeConfig.model_validate({"extras": {"top_n": 1}}) ) await provider.aclose() @@ -2834,7 +2858,9 @@ def handler(req: httpx.Request) -> httpx.Response: return httpx.Response(200, json=_rerank_body(results=[{"index": 0, "relevance_score": 0.9}])) provider = _rerank_provider(handler) - await provider.rerank("q", ["a", "b"], config=RerankRuntimeConfig.model_validate({"top_n": 1})) + await provider.rerank( + "q", ["a", "b"], config=RerankRuntimeConfig.model_validate({"extras": {"top_n": 1}}) + ) await provider.aclose() assert captured[0]["top_n"] == 1 @@ -2843,7 +2869,7 @@ async def test_jina_rerank_conflicting_top_n_extra_rejects() -> None: provider = _jina_rerank_provider(_never_called) with pytest.raises(ProviderInvalidRequest): await provider.rerank( - "q", ["a", "b"], top_k=2, config=RerankRuntimeConfig.model_validate({"top_n": 1}) + "q", ["a", "b"], top_k=2, config=RerankRuntimeConfig.model_validate({"extras": {"top_n": 1}}) ) await provider.aclose() @@ -2863,7 +2889,9 @@ def handler(req: httpx.Request) -> httpx.Response: ) provider = _jina_rerank_provider(handler) - await provider.rerank("q", ["a", "b"], config=RerankRuntimeConfig.model_validate({"top_n": 1})) + await provider.rerank( + "q", ["a", "b"], config=RerankRuntimeConfig.model_validate({"extras": {"top_n": 1}}) + ) await provider.aclose() assert captured[0]["top_n"] == 1 @@ -2884,7 +2912,7 @@ def handler(req: httpx.Request) -> httpx.Response: return httpx.Response(200, json=_cohere_embed_body(id="c", vectors=[[0.1, 0.2]], input_tokens=3)) provider = _cohere_embed_provider(handler) - cfg = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["banana", ""]}) + cfg = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["banana", ""]}}) await provider.embed(["x"], config=cfg) assert captured[0]["embedding_types"] == ["float", "banana", ""], ( "an unrecognized or empty precision string must merge, not read as malformed" @@ -2893,7 +2921,7 @@ def handler(req: httpx.Request) -> httpx.Response: # The structural arm is unchanged: a non-string element is still malformed, # and the whole list is dropped rather than partially salvaged. captured.clear() - cfg_mixed = EmbeddingRuntimeConfig.model_validate({"embedding_types": ["int8", 7]}) + cfg_mixed = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["int8", 7]}}) await provider.embed(["x"], config=cfg_mixed) assert captured[0]["embedding_types"] == ["float"], ( "a non-string element must still drop the whole list, with no partial salvage" diff --git a/tests/unit/test_structured_output.py b/tests/unit/test_structured_output.py index c3a59cf..d1a46ea 100644 --- a/tests/unit/test_structured_output.py +++ b/tests/unit/test_structured_output.py @@ -689,7 +689,7 @@ async def capturing_post(*args: Any, **kwargs: Any) -> Any: provider._client.post = capturing_post # type: ignore[method-assign] try: caller_extra = {"type": "json_object"} - config = RuntimeConfig(response_format=caller_extra) # type: ignore[call-arg] + config = RuntimeConfig(extras={"response_format": caller_extra}) # type: ignore[call-arg] await provider.complete( [UserMessage(content="hello")], config=config, From e1604ed309f42141acd52475c6a5063034381dd8 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sun, 6 Sep 2026 14:24:03 -0700 Subject: [PATCH 3/5] Un-defer the two same-name collision fixtures llm-provider 075 and retrieval-provider 052 were held because the coded reject looked unreachable: a declared-name key bound the declared field instead of landing in extras, so nothing could construct the collision. The container makes it constructible, and both fixtures pass. Mutation-verified rather than taken on a green run: making the reject arm never fire turns both red. Also corrects the comments that asserted the unreachability, and two module comments still describing the configs as extra="allow". --- CHANGELOG.md | 2 +- src/openarmature/retrieval/response.py | 4 ++-- tests/conformance/test_llm_provider.py | 6 ------ tests/conformance/test_retrieval_provider.py | 10 ---------- tests/unit/test_llm_provider.py | 14 ++++++-------- tests/unit/test_prompts.py | 2 +- 6 files changed, 10 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caf67b6..13e284b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Changed -- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Spec v0.117.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixtures 054 / 055 / 056 ride the pin bump. +- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Two fixtures come off the deferred list as a direct result: llm-provider 075 and retrieval-provider 052, both held because the same-name reject looked unreachable through the real caller path. Both now run and are mutation-verified against the reject. Spec v0.117.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixtures 054 / 055 / 056 ride the pin bump. - **Managed wire fields now reject a conflicting extras key instead of silently losing it** (proposals 0105 + 0108, llm-provider §6, spec v0.100.0 / v0.103.0). **Breaking for a managed-key collision only.** A caller's undeclared extras key (`RuntimeConfig` / `EmbeddingRuntimeConfig` accepting extra fields) is forwarded to the wire body untouched, except when it names a field the mapping *manages*: one it sets for its own correctness (0105), or produces as the wire realization of a declared config field (0108). On such a collision the field's shape now decides. An additive list field (`stop` from `stop_sequences`; `embedding_types`) **merges** the caller's value(s) onto the managed value(s), managed-first, de-duplicated. A non-additive scalar or object (`model`, `messages`, `truncate` / `truncation`, `dimensions` / `output_dimension`, `input_type`, `response_format`, Jina `task`, …) takes a value **equal** to the managed one as a no-op and **rejects a conflicting** one pre-send with `ProviderInvalidRequest`. Previously such a collision was silently dropped (the OpenAI llm mapping used `setdefault`) or silently overrode the managed value (the retrieval mappings spread extras first), either of which could re-route the model, defeat a fail-loud `truncate` flag, or break structured-output validation. A field the mapping does not manage keeps untouched pass-through, and a conditionally-managed field is only managed while produced, so the escape hatches hold: an extras `response_format` on a free-form or prompt-augmentation-fallback call rides untouched (0105 §3.5, previously stripped on the fallback path), and an extras Jina `task` with no `input_type` rides untouched (the model-specific-task escape hatch). The rule spans one OpenAI llm mapping and seven retrieval mappings via a shared resolver (`apply_managed_extras`). Spec v0.100.0 / v0.103.0 are beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the reject / merge fixtures ride the pin bump. - **Cohere `/v2/embed` recognizes `classification` and `clustering`** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). **Breaking for these two values.** `EmbeddingRuntimeConfig.input_type` is an extensible string, and §2 names `classification` and `clustering` as well-known values a mapping may recognize when its backend supports them. Cohere's does, so the mapping now identity-maps both onto the wire instead of rejecting them. Previously either value raised `ProviderInvalidRequest` before the request was sent, so a caller who relied on that rejection as a guard (catching it to fall back to `document`, say) silently changes behavior. `query` / `document` / absent / unrecognized are all unchanged, and `image` stays out: it names an input modality rather than a purpose for embedded text, and `embed()` consumes strings. The widening is deliberately per-mapping and not portable. Jina keeps its closed `{query, document}` set, because its `task` support varies by model version (v3 accepts `classification` but not `clustering`, v4 neither, v5 both) and a provider is bound to a model identifier with no capability registry to consult, so that mapping cannot promise the values and declines them pre-send rather than letting the wire reject them later. Spec v0.94.0 is beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixture 033's new cases ride the pin bump. - **Cohere `/v2/embed` `embedding_types` merge is now deterministic** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). The mapping manages `embedding_types` as an explicit exception to untouched extras pass-through, because it must request `"float"` for its own response consumer (it reads `embeddings.float`). A caller-supplied `embedding_types` is merged with that mandatory `"float"` rather than replacing it, which was already the behavior; an override that dropped `float` would strip the key the mapping itself reads and fail the call. What changes is the shape of the merged list, which 0099 pins so the outbound body is reproducible and exact-match assertable: `"float"` first, then the caller's precisions in the order supplied, de-duplicated with the first occurrence winning. Previously the caller's precisions came first with `"float"` appended, and a repeated precision was sent twice, so `["int8"]` now yields `["float", "int8"]` rather than `["int8", "float"]`, and `["int8", "uint8", "int8"]` yields `["float", "int8", "uint8"]` rather than passing the duplicate through. The wire is order-insensitive here, so no request semantics change; callers still read their extra precisions off the verbatim response on `raw`. A malformed or empty extra still falls back to `["float"]`. diff --git a/src/openarmature/retrieval/response.py b/src/openarmature/retrieval/response.py index cd72d68..1fcdb1a 100644 --- a/src/openarmature/retrieval/response.py +++ b/src/openarmature/retrieval/response.py @@ -83,7 +83,7 @@ class EmbeddingResponse(BaseModel): # Spec §2 declared-field surface: an optional ``dimensions``, an optional # ``input_type`` (proposal 0077), plus the extras pass-through bag -# (``extra="allow"``). Undeclared fields supplied by callers are forwarded to +# in the ``extras`` container. Undeclared fields supplied by callers are forwarded to # the wire body untouched by the §8 wire-format mapping; declared fields with # value ``None`` are omitted on the wire. ``input_type`` ("query" / "document", # an extensible string) declares what the embedded text is for; absent means @@ -178,7 +178,7 @@ class RerankResponse(BaseModel): # Spec §2 rerank runtime config: one declared field ``return_documents`` -# (boolean, default False) plus the extras pass-through bag (``extra="allow"``). +# (boolean, default False) plus the ``extras`` container. # Undeclared fields supplied by callers are forwarded to the wire body by the # §8 wire-format mapping, except for the provider-reserved keys a mapping # manages itself (e.g. the Cohere mapping owns model / query / documents / diff --git a/tests/conformance/test_llm_provider.py b/tests/conformance/test_llm_provider.py index 540d4c3..cf98010 100644 --- a/tests/conformance/test_llm_provider.py +++ b/tests/conformance/test_llm_provider.py @@ -158,12 +158,6 @@ "073-managed-stream-options-collision": ( "Proposal 0105 stream_options collision; streaming not implemented (0062)" ), - # Proposal 0108 (spec v0.103.0) declared-field-vs-extras collision. - "075-managed-declared-scalar-collision": ( - "Proposal 0108 same-name declared collision: the coded reject is unreachable via " - "the real caller path (a declared-name key routes to the declared field, never " - "model_extra), so adoption is held pending the batched spec review" - ), "077-managed-declared-stream-collision": ( "Proposal 0108 stream collision; streaming not implemented (0062)" ), diff --git a/tests/conformance/test_retrieval_provider.py b/tests/conformance/test_retrieval_provider.py index 7ee4766..9d0a3fa 100644 --- a/tests/conformance/test_retrieval_provider.py +++ b/tests/conformance/test_retrieval_provider.py @@ -100,16 +100,6 @@ # v0.17.0 spec-pin bump (v0.88.0 -> v0.107.0). Behavior for each of # these shipped + is unit-tested ahead of the pin; the conformance # fixture wiring rides the v0.17.0 fixture-wiring PR. - # Proposal 0108 (spec v0.103.0) same-NAME declared-field collision. The - # dimensions reject is coded (openai.py) but a declared-field-named extras - # key routes to the declared field, never into model_extra, so the collision - # is not reachable through the real caller path -- adoption is held pending - # the batched spec review (the llm 075 sibling is held for the same reason). - "052-embed-openai-dimensions-collision": ( - "Proposal 0108 same-name declared collision: the coded reject is unreachable via " - "the real caller path (a declared-name key routes to the declared field, never " - "model_extra), so adoption is held pending the batched spec review" - ), } diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 6f75b98..2dfa24b 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -3277,14 +3277,12 @@ def _collision_provider(handler: Any) -> OpenAIProvider: return OpenAIProvider(base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(handler)) -# A same-named declared sampling field (temperature, top_p, ...) cannot be -# shadowed by an extras key via the config API: pydantic binds the key to the -# declared field, so `model_validate({"temperature": ...})` never lands in -# model_extra. That collision is therefore unconstructible here (the resolver -# still handles it defensively; its reject arm is covered generically in -# test_managed_extras.py). The reachable collisions are the STRUCTURAL fields -# (model / messages / tools / tool_choice, not RuntimeConfig fields) and the -# RENAMED realizations (stop from stop_sequences), plus response_format. +# A same-named declared sampling field IS shadowable: `extras` is a container +# separately addressable from the declared fields, so `RuntimeConfig( +# temperature=0.2, extras={"temperature": ...})` sets both and the extras key +# stays an extras key. The other reachable collisions are the STRUCTURAL fields +# (model / messages / tools / tool_choice) and the RENAMED realizations (stop +# from stop_sequences), plus response_format. async def test_llm_conflicting_structural_extra_rejects() -> None: # A structural managed field (model) shadowed by an extra is rejected # pre-send rather than silently re-routing the model. diff --git a/tests/unit/test_prompts.py b/tests/unit/test_prompts.py index b218bb8..3c2b0f3 100644 --- a/tests/unit/test_prompts.py +++ b/tests/unit/test_prompts.py @@ -626,7 +626,7 @@ async def test_filesystem_backend_per_prompt_sidecar(tmp_path: Path) -> None: assert prompt.sampling is not None assert prompt.sampling.temperature == 0.0 assert prompt.sampling.max_tokens == 256 - # Vendor extra rides through the extras-allow bag. + # Vendor extra rides through the extras container. assert prompt.sampling.extras.get("repetition_penalty") == 1.05 From 873ddaf957562d0c6eae42d26e28ff624b6dec99 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sun, 6 Sep 2026 15:12:08 -0700 Subject: [PATCH 4/5] Fix the extras regressions the review found Two were real defects this branch introduced. A retry with a per-attempt override silently dropped the base config's extras. `extras` is a declared field defaulting to an empty dict, which exclude_none keeps, so the generic dump carried an empty container into the merge and replaced the caller's vendor knobs on every attempt. It was invisible in the trace too: request_params is projected once from the base config before the retry loop, so the emitted event reported extras the attempt never sent. The merge is now per key, with the override winning on a collision. A filesystem sidecar carrying an unrecognized top-level key raised a pydantic error out of fetch(). That is neither of the two documented error types, so PromptManager's multi-backend fallback never ran and one stray key in one operator-authored file took down every fetch for that prompt. Unrecognized keys are now filtered, matching what the token_budget path and the Langfuse backend already did. A Langfuse prompt.config now lifts an extras sub-object as well, so the container is honored on both documented sources rather than one. Also: a fifth conformance harness was still flattening the fixture's extras block, PromptManager's defensive copy shared the container by reference, and an assertion presented as covering the None-dropping was a tautology under the new strictness. Comment and docs corrections, including two module comments still describing the configs as extra="allow", a comment mangled into a half-sentence, and one I wrote narrating a mutation result. --- CHANGELOG.md | 2 +- docs/concepts/llms.md | 9 +-- docs/concepts/prompts.md | 17 ++++- docs/concepts/retrieval.md | 23 +++++++ src/openarmature/llm/providers/openai.py | 11 ++- .../prompts/backends/filesystem.py | 13 +++- src/openarmature/prompts/backends/langfuse.py | 10 ++- src/openarmature/prompts/manager.py | 11 ++- src/openarmature/retrieval/response.py | 2 +- .../test_observability_langfuse.py | 7 +- tests/conformance/test_prompt_management.py | 5 +- tests/unit/test_llm_provider.py | 69 ++++++++++++++++--- tests/unit/test_prompts.py | 44 ++++++++++++ tests/unit/test_retrieval_provider.py | 2 +- 14 files changed, 195 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13e284b..94a7407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Changed -- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Two fixtures come off the deferred list as a direct result: llm-provider 075 and retrieval-provider 052, both held because the same-name reject looked unreachable through the real caller path. Both now run and are mutation-verified against the reject. Spec v0.117.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixtures 054 / 055 / 056 ride the pin bump. +- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Two fixtures come off the deferred list as a direct result: llm-provider 075 and retrieval-provider 052, both held because the same-name reject looked unreachable through the real caller path. Both now run and are mutation-verified against the reject. Two smaller behaviors move with the container. A Langfuse `prompt.config` now lifts an `extras` sub-object onto `Prompt.sampling`, so a vendor knob reaches it from that source as it already did from a filesystem sidecar; the full config still rides `Prompt.metadata` either way. And a filesystem sidecar's unrecognized top-level key is now filtered rather than fatal, matching what the `token_budget` path and the Langfuse backend already did: with the config rejecting undeclared names, splatting the sidecar verbatim would turn one stray key in an operator-authored file into an error escaping `fetch()`, which is neither of the two documented error types and so would bypass the manager's multi-backend fallback. A vendor knob written flat in a sidecar is one such key, so it is filtered and does not reach `sampling.extras`. Spec v0.117.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixtures 054 / 055 / 056 ride the pin bump. - **Managed wire fields now reject a conflicting extras key instead of silently losing it** (proposals 0105 + 0108, llm-provider §6, spec v0.100.0 / v0.103.0). **Breaking for a managed-key collision only.** A caller's undeclared extras key (`RuntimeConfig` / `EmbeddingRuntimeConfig` accepting extra fields) is forwarded to the wire body untouched, except when it names a field the mapping *manages*: one it sets for its own correctness (0105), or produces as the wire realization of a declared config field (0108). On such a collision the field's shape now decides. An additive list field (`stop` from `stop_sequences`; `embedding_types`) **merges** the caller's value(s) onto the managed value(s), managed-first, de-duplicated. A non-additive scalar or object (`model`, `messages`, `truncate` / `truncation`, `dimensions` / `output_dimension`, `input_type`, `response_format`, Jina `task`, …) takes a value **equal** to the managed one as a no-op and **rejects a conflicting** one pre-send with `ProviderInvalidRequest`. Previously such a collision was silently dropped (the OpenAI llm mapping used `setdefault`) or silently overrode the managed value (the retrieval mappings spread extras first), either of which could re-route the model, defeat a fail-loud `truncate` flag, or break structured-output validation. A field the mapping does not manage keeps untouched pass-through, and a conditionally-managed field is only managed while produced, so the escape hatches hold: an extras `response_format` on a free-form or prompt-augmentation-fallback call rides untouched (0105 §3.5, previously stripped on the fallback path), and an extras Jina `task` with no `input_type` rides untouched (the model-specific-task escape hatch). The rule spans one OpenAI llm mapping and seven retrieval mappings via a shared resolver (`apply_managed_extras`). Spec v0.100.0 / v0.103.0 are beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the reject / merge fixtures ride the pin bump. - **Cohere `/v2/embed` recognizes `classification` and `clustering`** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). **Breaking for these two values.** `EmbeddingRuntimeConfig.input_type` is an extensible string, and §2 names `classification` and `clustering` as well-known values a mapping may recognize when its backend supports them. Cohere's does, so the mapping now identity-maps both onto the wire instead of rejecting them. Previously either value raised `ProviderInvalidRequest` before the request was sent, so a caller who relied on that rejection as a guard (catching it to fall back to `document`, say) silently changes behavior. `query` / `document` / absent / unrecognized are all unchanged, and `image` stays out: it names an input modality rather than a purpose for embedded text, and `embed()` consumes strings. The widening is deliberately per-mapping and not portable. Jina keeps its closed `{query, document}` set, because its `task` support varies by model version (v3 accepts `classification` but not `clustering`, v4 neither, v5 both) and a provider is bound to a model identifier with no capability registry to consult, so that mapping cannot promise the values and declines them pre-send rather than letting the wire reject them later. Spec v0.94.0 is beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the `conformance.toml` entry and fixture 033's new cases ride the pin bump. - **Cohere `/v2/embed` `embedding_types` merge is now deterministic** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). The mapping manages `embedding_types` as an explicit exception to untouched extras pass-through, because it must request `"float"` for its own response consumer (it reads `embeddings.float`). A caller-supplied `embedding_types` is merged with that mandatory `"float"` rather than replacing it, which was already the behavior; an override that dropped `float` would strip the key the mapping itself reads and fail the call. What changes is the shape of the merged list, which 0099 pins so the outbound body is reproducible and exact-match assertable: `"float"` first, then the caller's precisions in the order supplied, de-duplicated with the first occurrence winning. Previously the caller's precisions came first with `"float"` appended, and a repeated precision was sent twice, so `["int8"]` now yields `["float", "int8"]` rather than `["int8", "float"]`, and `["int8", "uint8", "int8"]` yields `["float", "int8", "uint8"]` rather than passing the duplicate through. The wire is order-insensitive here, so no request semantics change; callers still read their extra precisions off the verbatim response on `raw`. A malformed or empty extra still falls back to `["float"]`. diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index 10b3d8a..8d02729 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -635,10 +635,11 @@ is a one-node change. ## Provider-specific extras `RuntimeConfig` (and the retrieval `EmbeddingRuntimeConfig` / -`RerankRuntimeConfig`) accept fields beyond the declared set, and any -undeclared field is forwarded to the wire request untouched. This is how -you reach a backend-specific knob the portable config does not model, for -example a vLLM `guided_decoding`: +`RerankRuntimeConfig`) carry undeclared fields in an `extras` container, +and everything in it is forwarded to the wire request untouched. Passing +an undeclared name directly is rejected. This is how you reach a +backend-specific knob the portable config does not model, for example a +vLLM `guided_decoding`: ```python config = RuntimeConfig( diff --git a/docs/concepts/prompts.md b/docs/concepts/prompts.md index e5b6d1a..d6d8c66 100644 --- a/docs/concepts/prompts.md +++ b/docs/concepts/prompts.md @@ -201,11 +201,24 @@ behavior; most callers just pass the prompt back into `render()`. A `Prompt` carries an optional `sampling` field: a `SamplingConfig` sub-record mirroring `RuntimeConfig`'s seven declared fields (`temperature`, `max_tokens`, `top_p`, `seed`, `frequency_penalty`, -`presence_penalty`, `stop_sequences`) plus the extras pass-through -bag. Backends that source per-prompt config (Langfuse's +`presence_penalty`, `stop_sequences`) plus the `extras` container for +vendor knobs. Backends that source per-prompt config (Langfuse's `prompt.config`, a filesystem sidecar) populate it; backends that don't leave it `None`. +Both sources spell vendor knobs the same way, as an `extras` sub-object +beside the declared keys: + +```json +{"temperature": 0.0, "max_tokens": 256, "extras": {"repetition_penalty": 1.05}} +``` + +An unrecognized key beside them is ignored rather than failing the +fetch, so a stray or future key does not invalidate a well-formed +config. A vendor knob written flat, next to the declared keys rather +than inside `extras`, is one such key: it is filtered out and does not +reach `sampling.extras`. + ```python prompt = await manager.fetch("classify", "production") if prompt.sampling is not None: diff --git a/docs/concepts/retrieval.md b/docs/concepts/retrieval.md index 582c286..3a9dea9 100644 --- a/docs/concepts/retrieval.md +++ b/docs/concepts/retrieval.md @@ -137,6 +137,29 @@ providers, will quietly index unprefixed vectors and cost you recall with no signal. So treat `query` / `document` as the portable pair, and reach for the other purposes only when you know which provider you are on. +## Provider-specific extras + +`EmbeddingRuntimeConfig` and `RerankRuntimeConfig` carry undeclared +fields in an `extras` container, forwarded to the wire request +untouched. Passing an undeclared name directly is rejected. + +```python +await provider.embed( + passages, + config=EmbeddingRuntimeConfig( + input_type="document", + extras={"output_dimension": 256}, + ), +) +``` + +The container is separate from the declared fields, so you can set a +declared field and an extras key of the same name in one call. That is +how you override a wire field OA already models from the declared side. +Where the mapping itself produces that wire field, the conflicting +extras key is rejected before the request is sent rather than silently +winning or losing. + ## Long input lists are chunked for you Every hosted embedding API caps how many inputs one request may carry. diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 1e8b7d0..18cbd18 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -640,7 +640,16 @@ def _config_for_attempt( base_or_empty = base if base is not None else RuntimeConfig() # exclude_none (not exclude_unset): a None override field inherits the # base per §6 null-skip, rather than an explicit None clobbering it. - return base_or_empty.model_copy(update=override.model_dump(exclude_none=True)) + # + # `extras` is excluded from that dump and merged per key below. It is a + # declared field whose default is `{}`, which `exclude_none` keeps, so + # leaving it in the update would replace the base container on every + # attempt and drop the caller's vendor knobs from the retry body. + update = override.model_dump(exclude_none=True, exclude={"extras"}) + merged_extras = {**base_or_empty.extras, **override.extras} + if merged_extras: + update["extras"] = merged_extras + return base_or_empty.model_copy(update=update) @staticmethod def _append_reask_pair( diff --git a/src/openarmature/prompts/backends/filesystem.py b/src/openarmature/prompts/backends/filesystem.py index f38b98d..43aa068 100644 --- a/src/openarmature/prompts/backends/filesystem.py +++ b/src/openarmature/prompts/backends/filesystem.py @@ -259,8 +259,17 @@ async def fetch( def _sampling_from_dict(data: dict[str, Any]) -> SamplingConfig: # The sidecar's `extras` sub-object maps onto the config's own extras # container (0122). `token_budget` (0083) is a sibling sub-object read by - # `_token_budget_from_dict`, not a sampling field, so it is excluded too. - declared: dict[str, Any] = {k: v for k, v in data.items() if k not in ("extras", "token_budget")} + # `_token_budget_from_dict`, not a sampling field. + # + # UNRECOGNIZED top-level keys are ignored rather than raising (0109 + # tolerate-and-filter, as `_token_budget_from_dict` and the langfuse backend + # already do). The config rejects undeclared names, so splatting the sidecar + # verbatim would turn one stray key in an operator-authored file into a + # pydantic error escaping `fetch()`, which is not one of the two documented + # error types and so bypasses the manager's multi-backend fallback. + declared: dict[str, Any] = { + k: v for k, v in data.items() if k in SamplingConfig.model_fields and k != "extras" + } extras = data.get("extras") return SamplingConfig( **declared, diff --git a/src/openarmature/prompts/backends/langfuse.py b/src/openarmature/prompts/backends/langfuse.py index ff61f37..557cf97 100644 --- a/src/openarmature/prompts/backends/langfuse.py +++ b/src/openarmature/prompts/backends/langfuse.py @@ -173,9 +173,15 @@ def _sampling_from_config(config: dict[str, Any] | None) -> SamplingConfig | Non if not config: return None declared = {k: config[k] for k in _SAMPLING_FIELDS if k in config} - if not declared: + # The `extras` sub-object maps onto the config's extras container (0122), + # so a vendor knob reaches `Prompt.sampling` from this source as it does + # from the filesystem sidecar. The full config still rides + # `Prompt.metadata`, so nothing is lost either way. + raw_extras = config.get("extras") + extras = dict(cast("dict[str, Any]", raw_extras)) if isinstance(raw_extras, dict) else {} + if not declared and not extras: return None - return SamplingConfig(**declared) + return SamplingConfig(**declared, extras=extras) def _token_budget_from_config(config: dict[str, Any] | None) -> TokenBudget | None: diff --git a/src/openarmature/prompts/manager.py b/src/openarmature/prompts/manager.py index 3365d06..651050c 100644 --- a/src/openarmature/prompts/manager.py +++ b/src/openarmature/prompts/manager.py @@ -540,8 +540,15 @@ def _build_result( variables=variables, fetched_at=prompt.fetched_at, rendered_at=datetime.now(UTC), - # Defensive copy of the mutable propagated fields. - sampling=prompt.sampling.model_copy() if prompt.sampling is not None else None, + # Defensive copy of the mutable propagated fields. `extras` is + # copied explicitly: `model_copy` shares the container by reference, + # so mutating a result's extras would reach back into the Prompt and + # every other result rendered from it. + sampling=( + prompt.sampling.model_copy(update={"extras": dict(prompt.sampling.extras)}) + if prompt.sampling is not None + else None + ), # Proposal 0083: advisory token budget propagated verbatim (defensive # copy), mirroring sampling -- rendering does not modify it. token_budget=prompt.token_budget.model_copy() if prompt.token_budget is not None else None, diff --git a/src/openarmature/retrieval/response.py b/src/openarmature/retrieval/response.py index 1fcdb1a..4c6cbba 100644 --- a/src/openarmature/retrieval/response.py +++ b/src/openarmature/retrieval/response.py @@ -83,7 +83,7 @@ class EmbeddingResponse(BaseModel): # Spec §2 declared-field surface: an optional ``dimensions``, an optional # ``input_type`` (proposal 0077), plus the extras pass-through bag -# in the ``extras`` container. Undeclared fields supplied by callers are forwarded to +# in the ``extras`` container, from which they are forwarded to # the wire body untouched by the §8 wire-format mapping; declared fields with # value ``None`` are omitted on the wire. ``input_type`` ("query" / "document", # an extensible string) declares what the embedded text is for; absent means diff --git a/tests/conformance/test_observability_langfuse.py b/tests/conformance/test_observability_langfuse.py index edb9cd2..2b8aa63 100644 --- a/tests/conformance/test_observability_langfuse.py +++ b/tests/conformance/test_observability_langfuse.py @@ -2274,8 +2274,11 @@ def _runtime_config_from_spec(config_spec: dict[str, Any] | None) -> RuntimeConf } kwargs = {k: v for k, v in config_spec.items() if k in declared} extras = cast("dict[str, Any]", config_spec.get("extras") or {}) - kwargs.update(extras) - return RuntimeConfig(**kwargs) + # The fixture's `extras` sub-block maps onto the config's own container + # (0122). Merging it into the declared kwargs would let an extras key naming + # a declared field rebind that field, which is the collision 0122 exists to + # make expressible. + return RuntimeConfig(**kwargs, extras=dict(extras)) # --------------------------------------------------------------------------- diff --git a/tests/conformance/test_prompt_management.py b/tests/conformance/test_prompt_management.py index 5f8ecb7..d35023c 100644 --- a/tests/conformance/test_prompt_management.py +++ b/tests/conformance/test_prompt_management.py @@ -214,9 +214,8 @@ def __init__(self, spec: FixtureBackendSpec) -> None: self._prompts: dict[tuple[str, str], Prompt] = {} now = datetime.now(UTC) for ps in spec.prompts: - # Sampling sub-record (fixture 013): flatten the fixture's - # The `extras:` sub-block maps onto the config's own extras - # container (0122). + # Sampling sub-record (fixture 013): the `extras:` sub-block maps + # onto the config's own extras container (0122). sampling: SamplingConfig | None = None if ps.sampling is not None: declared: dict[str, Any] = {k: v for k, v in ps.sampling.items() if k != "extras"} diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 2dfa24b..c2b0a84 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -925,9 +925,10 @@ def test_runtime_config_from_partial_forwards_extras() -> None: assert config.temperature == 0.5 assert config.extras == {"repetition_penalty": 1.05} - # The None-dropping is the whole job: `top_k=None` is dropped before - # construction rather than rejected as an undeclared field. - assert not hasattr(config, "top_k") + # The None-dropping is the whole job: an undeclared name whose value is + # None is dropped before construction, where passing it directly raises. + with pytest.raises(ValidationError): + RuntimeConfig(top_k=None) # type: ignore[call-arg] def test_runtime_config_from_partial_empty() -> None: @@ -2551,9 +2552,8 @@ async def test_llm_completion_event_request_extras_flows_through() -> None: ) provider = OpenAIProvider(base_url="http://test", model="m", api_key="k", transport=transport) try: - # ``guided_decoding`` is a vLLM-specific extra; RuntimeConfig - # accepts undeclared fields via extra="allow". Use model_validate - # so pyright doesn't flag the undeclared kwarg. + # ``guided_decoding`` is a vLLM-specific extra, so it rides the + # ``extras`` container rather than being passed flat. await provider.complete( [UserMessage(content="hi")], config=RuntimeConfig.model_validate({"extras": {"guided_decoding": {"choice": ["a", "b"]}}}), @@ -3371,9 +3371,8 @@ def test_undeclared_fields_must_go_in_the_extras_container(factory: str, declare # addressable from the declared fields. An undeclared name passed flat is # rejected, so there is one spelling rather than two. # - # Without this the config classes accept BOTH forms and the flat one silently - # keeps working. Verified by mutation: flipping `extra="forbid"` back to - # `extra="allow"` left the whole suite green before this landed. + # The rejection is the assertion that matters: without it the flat form keeps + # working alongside the container and there are two spellings. from openarmature.llm import RuntimeConfig from openarmature.prompts import SamplingConfig from openarmature.retrieval import EmbeddingRuntimeConfig, RerankRuntimeConfig @@ -3403,3 +3402,55 @@ def test_undeclared_fields_must_go_in_the_extras_container(factory: str, declare collided = cls.model_validate({**declared, "extras": {same_name: "from-extras"}}) assert getattr(collided, same_name) == declared[same_name] assert collided.extras == {same_name: "from-extras"} + + +async def test_per_attempt_override_preserves_base_extras() -> None: + # A retry override merges into the base extras per key rather than replacing + # the container. `extras` is a declared field defaulting to `{}`, which + # `exclude_none` keeps, so a generic dump would carry an empty container into + # the update and wipe the caller's vendor knobs on every attempt. + # + # The loss is silent on the wire and invisible in the trace: `request_params` + # is projected once from the base config before the retry loop, so the + # emitted event still reports extras the attempt did not send. + bodies: list[dict[str, Any]] = [] + calls = {"n": 0} + + def handler(req: httpx.Request) -> httpx.Response: + bodies.append(json.loads(req.content)) + calls["n"] += 1 + if calls["n"] == 1: + return httpx.Response(503, json={"error": {"message": "upstream"}}) + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + {"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"} + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + from openarmature.llm import LlmRetryConfig + + provider = _collision_provider(handler) + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(temperature=0.2, extras={"guided_decoding": {"grammar": "g"}}), + retry=LlmRetryConfig( + max_attempts=2, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(temperature=0.6)], + ), + ) + await provider.aclose() + + assert len(bodies) == 2, f"expected a retry, got {len(bodies)} call(s)" + assert bodies[0]["guided_decoding"] == {"grammar": "g"} + assert bodies[1]["guided_decoding"] == {"grammar": "g"}, ( + "the retry attempt dropped the base config's extras" + ) + # The override's own declared field still applies on the retry. + assert bodies[1]["temperature"] == 0.6 diff --git a/tests/unit/test_prompts.py b/tests/unit/test_prompts.py index 3c2b0f3..eb7f9a7 100644 --- a/tests/unit/test_prompts.py +++ b/tests/unit/test_prompts.py @@ -1190,3 +1190,47 @@ def test_cross_variable_substring_stability_chat_prompt() -> None: # degenerate-equality false pass. assert "alice's email" in user_a and "bob's email" in user_b assert user_a.endswith("hello") and user_b.endswith("world") + + +async def test_filesystem_sidecar_ignores_an_unrecognized_sampling_key(tmp_path: Path) -> None: + # An unrecognized top-level key is filtered, not fatal (0109 + # tolerate-and-filter, matching the token_budget path and the langfuse + # backend). The config rejects undeclared names, so splatting the sidecar + # verbatim raises a pydantic error out of `fetch()`. That is neither + # PromptNotFound nor PromptStoreUnavailable, so PromptManager's + # multi-backend fallback would never run and one stray key in one + # operator-authored file would take down every fetch for that prompt. + (tmp_path / "production").mkdir() + (tmp_path / "production" / "summarize.j2").write_text("S: {{ text }}", encoding="utf-8") + (tmp_path / "production" / "summarize.config.json").write_text( + '{"temperature": 0.0, "repetition_penalty": 1.05}', encoding="utf-8" + ) + + backend = FilesystemPromptBackend(tmp_path, sampling_source="per-prompt-sidecar") + prompt = await backend.fetch("summarize", "production") + + assert prompt.sampling is not None + assert prompt.sampling.temperature == 0.0 + # Filtered rather than lifted: the flat spelling is not a second way to + # reach the container. + assert prompt.sampling.extras == {} + + +def test_langfuse_prompt_config_lifts_the_extras_sub_object() -> None: + # The container name is normative, so a vendor knob reaches Prompt.sampling + # from a Langfuse `prompt.config` as it does from a filesystem sidecar. + from openarmature.prompts.backends.langfuse import _sampling_from_config + + sampling = _sampling_from_config( + {"temperature": 0.3, "extras": {"repetition_penalty": 1.05}, "unrelated": "x"} + ) + + assert sampling is not None + assert sampling.temperature == 0.3 + assert sampling.extras == {"repetition_penalty": 1.05} + # A config carrying ONLY extras still yields a config rather than None. + only_extras = _sampling_from_config({"extras": {"k": 1}}) + assert only_extras is not None + assert only_extras.extras == {"k": 1} + # Nothing recognized at all still yields None. + assert _sampling_from_config({"unrelated": "x"}) is None diff --git a/tests/unit/test_retrieval_provider.py b/tests/unit/test_retrieval_provider.py index b9d5903..080161b 100644 --- a/tests/unit/test_retrieval_provider.py +++ b/tests/unit/test_retrieval_provider.py @@ -2741,7 +2741,7 @@ def handler(req: httpx.Request) -> httpx.Response: assert captured[0]["user_tag"] == "keep" -async def test_openai_embed_conflicting_model_extra_rejects() -> None: +async def test_openai_embed_conflicting_extras_key_rejects() -> None: provider = _openai_embed_provider(_never_called) with pytest.raises(ProviderInvalidRequest): await provider.embed( From 9c5bf18ffc76ac9bef2118b130c9e8e46de75f0b Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sun, 6 Sep 2026 15:48:24 -0700 Subject: [PATCH 5/5] Close the test provider and drop a stale type ignore The new cohere test left its provider open, so a failing assertion mid-test leaked the transport. Wrapped in try/finally, verified by forcing the assertion red and confirming no unclosed-transport warning. The type ignore on a RuntimeConfig construction predated the container: extras is a typed declared field now, so pyright accepts it. The sibling ignore on RuntimeConfig(top_k=None) stays, since that name is deliberately undeclared. Also drops a comment clause describing what the gate used to do. --- tests/unit/test_retrieval_provider.py | 36 ++++++++++++++------------- tests/unit/test_structured_output.py | 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_retrieval_provider.py b/tests/unit/test_retrieval_provider.py index 080161b..ab7cd43 100644 --- a/tests/unit/test_retrieval_provider.py +++ b/tests/unit/test_retrieval_provider.py @@ -2901,10 +2901,8 @@ async def test_cohere_embed_unrecognized_precision_strings_merge_rather_than_mal # VOCABULARY check. A well-typed string the provider does not recognize # merges, and the provider rejects it if unsupported. # - # The empty string is the element an implementation reading "not a precision - # string" as "not one of the known names" gets wrong, and this mapping did: - # the gate carried an `and t` truthiness clause that dropped the whole list - # to ["float"]. Fixture 053 case 3 pins it. + # The empty string is the element a reading of "not a precision string" as + # "not one of the known names" gets wrong. Fixture 053 case 3 pins it. captured: list[dict[str, Any]] = [] def handler(req: httpx.Request) -> httpx.Response: @@ -2912,17 +2910,21 @@ def handler(req: httpx.Request) -> httpx.Response: return httpx.Response(200, json=_cohere_embed_body(id="c", vectors=[[0.1, 0.2]], input_tokens=3)) provider = _cohere_embed_provider(handler) - cfg = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["banana", ""]}}) - await provider.embed(["x"], config=cfg) - assert captured[0]["embedding_types"] == ["float", "banana", ""], ( - "an unrecognized or empty precision string must merge, not read as malformed" - ) + try: + cfg = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["banana", ""]}}) + await provider.embed(["x"], config=cfg) + assert captured[0]["embedding_types"] == ["float", "banana", ""], ( + "an unrecognized or empty precision string must merge, not read as malformed" + ) - # The structural arm is unchanged: a non-string element is still malformed, - # and the whole list is dropped rather than partially salvaged. - captured.clear() - cfg_mixed = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["int8", 7]}}) - await provider.embed(["x"], config=cfg_mixed) - assert captured[0]["embedding_types"] == ["float"], ( - "a non-string element must still drop the whole list, with no partial salvage" - ) + # The structural arm is unchanged: a non-string element is still + # malformed, and the whole list is dropped rather than partially + # salvaged. + captured.clear() + cfg_mixed = EmbeddingRuntimeConfig.model_validate({"extras": {"embedding_types": ["int8", 7]}}) + await provider.embed(["x"], config=cfg_mixed) + assert captured[0]["embedding_types"] == ["float"], ( + "a non-string element must still drop the whole list, with no partial salvage" + ) + finally: + await provider.aclose() diff --git a/tests/unit/test_structured_output.py b/tests/unit/test_structured_output.py index d1a46ea..29f01a9 100644 --- a/tests/unit/test_structured_output.py +++ b/tests/unit/test_structured_output.py @@ -689,7 +689,7 @@ async def capturing_post(*args: Any, **kwargs: Any) -> Any: provider._client.post = capturing_post # type: ignore[method-assign] try: caller_extra = {"type": "json_object"} - config = RuntimeConfig(extras={"response_format": caller_extra}) # type: ignore[call-arg] + config = RuntimeConfig(extras={"response_format": caller_extra}) await provider.complete( [UserMessage(content="hello")], config=config,