diff --git a/CHANGELOG.md b/CHANGELOG.md index 673d795..d891085 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **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"]`. +- **A malformed extra for a merge-managed wire field is treated as absent** (proposal 0113, llm-provider §6, inherited by retrieval-provider §8.4, spec v0.107.0). **Behavioral at the malformed edge.** Completes the §6 managed-field MERGE arm from 0105 / 0108. A caller's extras value for a merge-managed field (the llm `stop` from `stop_sequences`, Cohere's `embedding_types`) merges onto the managed value only when it carries the field's string-or-array-of-strings shape. A malformed value (a non-string scalar, or a list holding any non-string element) is now treated as absent: the mapping sends only the value(s) it would send with no such extra present, all-or-nothing, with no partial salvage of the well-formed elements and no raise or diagnostic. Malformation is judged structurally, not against a semantic vocabulary. Previously the shared `apply_managed_extras` merge arm coerced any value into the list, so a bare `5` became `["STOP", 5]` on the wire and a `["END", 123]` sent the stray `123`, letting a caller inject a malformed token onto a managed field. The retrieval `embedding_types` arm already fell back to `["float"]` on a malformed extra through 0099's bespoke Cohere merge; the general merge resolver now applies the same malformed-is-absent rule to `stop`. Each arm judges malformation against its own field's expected shape (`stop` is string-or-array-of-strings, so a scalar string coerces to a one-element list; `embedding_types` is array-only), so the shared rule is the behavior, not a byte-identical gate. The general merge-arm fix and conformance fixture 081 land in this release. - **A malformed LLM usage counter is treated as not reported instead of raising** (proposal 0101, llm-provider §6 / §7 + observability §5.5.3 / §11.2, spec v0.96.0). **Behavioral reversal.** A usage counter present on the wire but not a non-negative integer (a string, a negative, a bool) is now nulled rather than raising `ProviderInvalidResponse`: the completion succeeded, the message is intact, and the verbatim value is preserved on `raw`. The sound counters stand, so a `{"prompt_tokens": -5, "completion_tokens": 1, "total_tokens": 1}` record surfaces as `{null, 1, 1}`; when every counter is malformed the record is `{null, null, null}` (the §6 null-together shape), still a present record so `LlmCompletionEvent.usage` mirrors it rather than going null. The value is never coerced or clamped, since a repaired counter is indistinguishable from a reported one, and `cached_tokens` follows the same rule. Previously any such counter raised `provider_invalid_response`, discarding a sound completion over an accounting figure. The observability surfaces already omit a not-reported counter per field (the OTel `openarmature.llm.usage.*` / `gen_ai.usage.*` span attributes, the token-usage histogram, and the token-budget instruments), so a null counter reaches none of them, and the Langfuse Generation `usage` omits it. Spec v0.96.0 is beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the conformance fixtures ride the pin bump. - **Jina `400` maps to `provider_invalid_request`, and an empty-string retrieval `response_id` is null** (proposal 0104, retrieval-provider §4 / §6 / §8.2, spec v0.99.0). **Behavioral at two edges.** The Jina error enumeration listed only `422`, so a bare `400` fell through to the transient `provider_unavailable` catch-all and invited a pointless retry of a request that will not succeed on retry; it now maps to `provider_invalid_request`, aligning Jina with the TEI / OpenAI / Cohere mappings. Separately, an empty-string `response_id` (`""`) on the Jina and Cohere embed / rerank responses is now treated as absent (`null`) rather than surfaced literally: an identifier that correlates nothing is not a present one, extending 0100's "malformed id is null" rule to the empty string. This deliberately differs from 0097's empty-`document` echo, which stays present, because a document is content while a `response_id` is an identifier. The OpenAI embed mapping already folded `""` to null and is unchanged. Well-formed responses and non-`400` errors are unaffected. Spec v0.99.0 is beyond the current v0.88.0 pin, so this ships ahead of the pin (unit-tested); the conformance fixtures ride the pin bump. - **`OpenAIEmbeddingProvider` accepts a test-only `chunk_size` cap override** (proposal 0103, retrieval-provider §8.3 + conformance-adapter §5.14, spec v0.98.0). OpenAI's per-call input cap is a fixed vendor 2048, not construction-configurable like TEI's, so the §8 batch-chunking path could not be driven with a small body. An optional `chunk_size` constructor argument (validated positive; production leaves it unset and the fixed 2048 applies) overrides the cap for tests and the conformance harness, so a fixture can exercise chunk-and-stitch with a handful of inputs. No production behavior changes: `chunk_size` is unset by default, and the count-based chunking rule (no client-side token estimation; an over-token request fails loud as `provider_invalid_request`) is unchanged. Spec v0.98.0 is beyond the current v0.88.0 pin; the §8.3 over-cap fixture 043 and the single-request `raw` assertion ride the pin bump. diff --git a/conformance.toml b/conformance.toml index b11f005..fc03a38 100644 --- a/conformance.toml +++ b/conformance.toml @@ -1069,6 +1069,6 @@ note = "0111 scoped the collect-channel warning to inputs-based seeding; it now # Spec v0.107.0 (proposal 0113). Malformed extras on a merge-managed # wire field (llm-provider §6, inherited by retrieval-provider §8.4). [proposals."0113"] -status = "partial" +status = "implemented" since = "0.17.0" -note = "0113 pins the malformed case of the §6 Managed-field collision MERGE arm: a caller's extras value that is not the field's expected list shape, or a list containing any element not of the expected element type/shape, is treated as ABSENT (the mapping sends only the value(s) it would send with no such extra present -- all-or-nothing, no partial salvage, no raise / diagnostic); malformation is judged structurally (no semantic vocabulary check). PARTIAL: the retrieval-provider §8.4 embedding_types arm (a malformed extras embedding_types -> wire [\"float\"]) already ships via 0099's bespoke Cohere merge, and its fixture 053 passes. The GENERAL merge-arm malformed rule is NOT yet implemented -- the shared apply_managed_extras merge arm coerces a non-list-shaped extra to a one-element list rather than treating a malformed value as absent, so an llm stop malformed extra (fixture 081) is not yet handled. The general merge-arm malformed handling + fixture 081 ride the v0.17.0 fixture-wiring PR." +note = "0113 pins the malformed case of the §6 Managed-field collision MERGE arm: a caller's extras value that is not the field's expected list shape, or a list containing any element not of the expected element type/shape, is treated as ABSENT (the mapping sends only the value(s) it would send with no such extra present -- all-or-nothing, no partial salvage, no raise / diagnostic); malformation is judged structurally (no semantic vocabulary check). The shared apply_managed_extras merge arm gates on well-formedness (a string, or a list whose every element is a string): a malformed extra is skipped and the managed value stands alone. Fixture 081 (llm stop) exercises the general rule; the retrieval-provider §8.4 embedding_types arm (a malformed extras embedding_types -> wire [\"float\"], fixture 053) already shipped via 0099's bespoke Cohere merge." diff --git a/src/openarmature/_managed_extras.py b/src/openarmature/_managed_extras.py index beb39b2..dd5a30e 100644 --- a/src/openarmature/_managed_extras.py +++ b/src/openarmature/_managed_extras.py @@ -95,7 +95,15 @@ def apply_managed_extras( # produced, so there is nothing to collide with). body[key] = value elif arm == "merge": - body[key] = _merge_list(body.get(key), value) + # A malformed extra for a merge-managed field (not a string, not a + # list of strings) is structurally invalid and, per 0113, is treated + # as absent: the managed value stands alone. No partial salvage of + # the well-formed elements, no raise -- merge is all-or-nothing. + if _is_mergeable_extra(value): + # Resolve the managed base the same way the reject arm does: a + # relied-upon wire default lives in `defaults`, not `body`. + base = defaults[key] if key in defaults else body.get(key) + body[key] = _merge_list(base, value) else: # "reject" managed_value = defaults[key] if key in defaults else body.get(key) if managed_value == value: @@ -110,6 +118,16 @@ def apply_managed_extras( ) +def _is_mergeable_extra(value: Any) -> bool: + """Whether an extras value has the string-or-array-of-strings shape a + merge-managed wire field accepts (e.g. OpenAI ``stop``).""" + if isinstance(value, str): + return True + if isinstance(value, list): + return all(isinstance(item, str) for item in cast("list[Any]", value)) + return False + + def _merge_list(managed_value: Any, extra_value: Any) -> list[Any]: """Merge an extras value onto a managed list value: managed entries first, then the extra's, de-duplicated first-occurrence-wins. A scalar on either diff --git a/tests/conformance/harness/expectations.py b/tests/conformance/harness/expectations.py index d5cbdfa..0f44a0e 100644 --- a/tests/conformance/harness/expectations.py +++ b/tests/conformance/harness/expectations.py @@ -110,6 +110,12 @@ class LlmProviderExpected(_ForbidExtras): response: LlmProviderResponseAssertion | None = None raises: LlmProviderRaisesAssertion | None = None success: bool | None = None + # Managed-collision fixtures (072/074/076/081) attach a block of documentary + # boolean flags naming the invariant each case proves. The machine assertion + # is the case's ``expected_wire_request`` / ``raises``; these are prose the + # reader checks against. Typed ``dict[str, bool]`` (not ``Any``) so a machine + # assertion mis-filed under ``invariants`` (a nested dict) is still rejected. + invariants: dict[str, bool] | None = None # --------------------------------------------------------------------------- diff --git a/tests/conformance/test_fixture_parsing.py b/tests/conformance/test_fixture_parsing.py index 7083e0c..c31cf23 100644 --- a/tests/conformance/test_fixture_parsing.py +++ b/tests/conformance/test_fixture_parsing.py @@ -664,38 +664,24 @@ def _id(case: tuple[str, Path]) -> str: "llm-provider/071-usage-counter-malformed-streaming-terminal-chunk": ( "Proposal 0101 usage-counter fixture model; rides the v0.17.0 fixture-wiring PR" ), - # Proposals 0105 (v0.100.0) / 0108 (v0.103.0) managed-field collision; - # the fixtures carry per-case `invariants` flags the parser does not model. - "llm-provider/072-managed-response-format-collision": ( - "Proposal 0105 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" - ), + # Managed-field collision (0105/0108/0113): the per-case `invariants` block is + # now modeled, so 072/074/075/076/081 parse and are un-deferred. The rows below + # stay deferred on a SECOND, unrelated parse blocker -- a fixture shape whose + # runtime proposal is still unimplemented -- not on the invariants model: "llm-provider/073-managed-stream-options-collision": ( - "Proposal 0105 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" - ), - "llm-provider/074-managed-structural-model-collision": ( - "Proposal 0105 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" - ), - "llm-provider/075-managed-declared-scalar-collision": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" - ), - "llm-provider/076-managed-declared-stop-merge": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" + "Proposal 0062 streaming fixture shape (`no_token_events_emitted`) not modeled" ), "llm-provider/077-managed-declared-stream-collision": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" + "Proposal 0062 streaming fixture shape (`no_token_events_emitted`) not modeled" ), "llm-provider/078-managed-anthropic-stream-reject": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" + "Proposal 0037 Anthropic fixture shape (top-level `mapping`) not modeled" ), "llm-provider/079-managed-anthropic-stop-merge": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" + "Proposal 0037 Anthropic fixture shape (top-level `mapping`) not modeled" ), "llm-provider/080-managed-gemini-stop-merge": ( - "Proposal 0108 managed-collision invariants model; rides the v0.17.0 fixture-wiring PR" - ), - # Proposal 0113 (v0.107.0) malformed extras on a merge-managed field. - "llm-provider/081-managed-declared-stop-malformed": ( - "Proposal 0113 malformed merge-managed extras invariants model; rides the v0.17.0 fixture-wiring PR" + "Proposal 0038 Gemini fixture shape (top-level `mapping`) not modeled" ), # Proposal 0109 (v0.104.0) token-budget config extra-key handling. "prompt-management/037-token-budget-unrecognized-key-ignored": ( diff --git a/tests/conformance/test_llm_provider.py b/tests/conformance/test_llm_provider.py index a7757f5..f6cb187 100644 --- a/tests/conformance/test_llm_provider.py +++ b/tests/conformance/test_llm_provider.py @@ -160,8 +160,9 @@ ), # Proposal 0108 (spec v0.103.0) declared-field-vs-extras collision. "075-managed-declared-scalar-collision": ( - "Proposal 0108 same-name declared collision; caller-reachability flagged to spec, " - "wiring rides the v0.17.0 fixture-wiring PR" + "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)" @@ -175,16 +176,6 @@ "080-managed-gemini-stop-merge": ( "Proposal 0108 stop merge; Gemini provider not implemented (0038 not-yet)" ), - # Proposal 0113 (spec v0.107.0) malformed extras on a merge-managed field. - # PARTIAL: retrieval embedding_types (fixture 053) works via 0099's bespoke - # merge, but the GENERAL apply_managed_extras merge arm coerces a malformed - # extra instead of treating it as absent, so llm stop malformed is not yet - # handled -- a behavior gap, not just harness wiring. The general merge-arm - # malformed rule + this fixture ride the v0.17.0 fixture-wiring PR. - "081-managed-declared-stop-malformed": ( - "Proposal 0113 general merge-arm malformed handling not yet implemented; " - "rides the v0.17.0 fixture-wiring PR" - ), } diff --git a/tests/conformance/test_retrieval_provider.py b/tests/conformance/test_retrieval_provider.py index b591148..57e52e7 100644 --- a/tests/conformance/test_retrieval_provider.py +++ b/tests/conformance/test_retrieval_provider.py @@ -103,11 +103,12 @@ # 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 -- FLAGGED to the batched - # spec review; the resolution + fixture wiring ride the v0.17.0 fixture-wiring PR. + # 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; caller-reachability flagged to spec, " - "wiring rides the v0.17.0 fixture-wiring PR" + "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_managed_extras.py b/tests/unit/test_managed_extras.py index 69858d0..e46fef2 100644 --- a/tests/unit/test_managed_extras.py +++ b/tests/unit/test_managed_extras.py @@ -90,6 +90,42 @@ def test_merge_arm_onto_absent_managed_value() -> None: assert body["stop"] == ["A", "B"] +def test_merge_arm_malformed_list_element_is_treated_as_absent() -> None: + # 0113: a merge extra with a non-string element is structurally malformed and + # treated as absent -- all-or-nothing, the well-formed "END" is NOT salvaged + # and no error is raised. The managed value stands alone. + body: dict[str, Any] = {"stop": ["STOP"]} + apply_managed_extras(body, {"stop": ["END", 123]}, {"stop": "merge"}) + assert body["stop"] == ["STOP"] + + +def test_merge_arm_malformed_scalar_is_treated_as_absent() -> None: + # 0113: a non-string scalar has no string-or-array-of-strings shape, so it is + # malformed and treated as absent rather than coerced onto the wire. + body: dict[str, Any] = {"stop": ["STOP"]} + apply_managed_extras(body, {"stop": 5}, {"stop": "merge"}) + assert body["stop"] == ["STOP"] + + +def test_merge_arm_empty_string_element_is_well_formed_and_merges() -> None: + # 0113 judges malformation STRUCTURALLY: an empty string is a well-typed + # string, so ["A", ""] is a list of strings -- well-formed -- and merges. The + # mapping does not semantically validate element values, so a "" stop token + # reaches the wire and the provider decides. Pins the structural-only rule + # against a future narrowing to a semantic (non-empty) check. + body: dict[str, Any] = {"stop": ["STOP"]} + apply_managed_extras(body, {"stop": ["A", ""]}, {"stop": "merge"}) + assert body["stop"] == ["STOP", "A", ""] + + +def test_merge_arm_empty_list_is_well_formed_no_op() -> None: + # An empty list is well-formed (every element -- none -- is a string) and + # merges as a no-op, leaving the managed value untouched. + body: dict[str, Any] = {"stop": ["STOP"]} + apply_managed_extras(body, {"stop": []}, {"stop": "merge"}) + assert body["stop"] == ["STOP"] + + def test_conditionally_managed_field_when_not_produced_rides_untouched() -> None: # The escape hatch: a field the mapping is NOT currently producing is absent # from `managed`, so its extra rides untouched (e.g. Jina task with no