Adopt 0093 nullable provider usage records#218
Merged
Conversation
Proposal 0093 gives OA one model for "the provider reported no usage" across both retrieval responses and both typed events: no usage yields usage = null, never a fabricated record, zero, or estimate. The response types and both observers were already correct. The gap was OpenAIEmbeddingProvider, which recorded EmbeddingUsage(input_tokens=0) when the usage block was absent or malformed. A zero asserts "zero tokens billed", a claim the provider never made. It now surfaces usage = null; a reported zero still yields a record carrying 0. Reviewing that diff surfaced a second, unrelated defect. Proposal 0092 requires every capped embedding mapping to chunk-and-stitch and names OpenAI's 2048-input cap, but the 0092 implementation adopted the shared helper for TEI and Cohere only. An over-2048 embed call sent one over-cap request the wire rejects, breaking the arbitrary-length input contract. OpenAI now chunks at 2048. Conformance stayed green because the spec ships no over-cap fixture for that mapping; tracked. The shared helper now takes the response identity (id and model) from the first chunk, so OpenAI keeps reporting the provider-returned model while the wires that carry none fall back to the bound identifier. A missing usage figure and a corrupt one were both swallowed silently, so a gateway sending garbage looked identical to a healthy no-usage call. The shared nonneg_int reader now logs a warning when a figure is present but not a non-negative int, while an absent figure stays quiet. nonneg_int lifts into _wire.py and is adopted by OpenAI, Cohere, and Jina. Un-defers observability fixtures 139 and 140. Fixture 143 stays deferred behind the unimplemented section 11 embedding-metrics path, not behind anything in 0093.
There was a problem hiding this comment.
Pull request overview
This PR updates the retrieval-provider implementation to fully conform to spec proposal 0093 (nullable provider usage records), while also fixing OpenAI’s embedding batch chunking behavior per the §8 rules and adding observability for corrupt usage figures.
Changes:
- Ensure embedding/rerank usage is represented uniformly as
usage = nullwhen not reported (never fabricated as zero), including OpenAI-compatible embeddings. - Apply chunk-and-stitch to OpenAI embeddings when input count exceeds the 2048 per-request cap, using the shared
chunk_and_stitch_embedhelper. - Centralize usage-figure parsing in
_wire.nonneg_int, which warns on present-but-corrupt figures while remaining quiet for absent usage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_retrieval_provider.py | Adds unit coverage for OpenAI 2048-cap chunking and for nullable/corrupt usage parsing + warning behavior. |
| tests/conformance/test_observability.py | Re-enables embedding “no usage” observability fixtures (139/140) and clarifies the remaining deferred fixture rationale (143). |
| src/openarmature/retrieval/providers/tei.py | Updates TEI embed chunk parsing signature to match the shared chunk-and-stitch identity model. |
| src/openarmature/retrieval/providers/openai.py | Implements OpenAI embedding chunk-and-stitch at 2048 inputs and switches usage parsing to nullable semantics via nonneg_int. |
| src/openarmature/retrieval/providers/jina.py | Removes local non-negative-int helper in favor of shared _wire.nonneg_int for consistent nullable usage handling. |
| src/openarmature/retrieval/providers/cohere.py | Switches usage parsing to shared _wire.nonneg_int and updates chunk parsing signature for unified response identity handling. |
| src/openarmature/retrieval/_wire.py | Adds nonneg_int (warn-on-corrupt usage figure) and extends chunk_and_stitch_embed to propagate provider-returned response model from the first chunk. |
| conformance.toml | Marks proposal 0093 implemented and updates proposal 0092 notes to include OpenAI’s 2048-cap chunking behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow-up to the adversarial and CoPilot review of the 0093 PR. Usage accounting in chunk_and_stitch_embed is now all-or-nothing: a total is reported only when EVERY chunk reports a figure. A chunk that omits usage (absent or corrupt) makes the whole-call total null rather than a partial sum, which would present a confident undercount and diverge from the single-request path (which returns null for the same unaccountable figure). Behaviorally invisible for the reference providers (hosted OpenAI and Cohere report on every chunk; TEI never reports), it only changes the unspecified mixed case an inconsistent gateway could produce. A new unit test pins it and fails under the old partial-sum logic. An empty-string wire model or id from an OpenAI-compatible backend now reads as absent (None) rather than surfacing as "", so the stitch falls back to the bound model identifier per §4. Documents that a chunked embed is non-atomic: a mid-call failure may leave earlier chunks billed by the provider with no usage event, since there is no rollback over a non-transactional wire. Corrects the Cohere chunk-closure comment to the 5-tuple return shape (model always null for Cohere), and notes the per-chunk warning multiplicity is accepted.
The conformance note described the pre-hardening per-chunk usage semantics (sum when reported, else null). chunk_and_stitch_embed now combines usage all-or-nothing: it sums only when every chunk reports a figure, and a mixed report yields usage = null rather than a partial sum. Bring the note in line with the code.
The nullable-usage unit tests closed the provider after the assertions, so a failing assert would skip aclose() and leak the httpx client. Wrap embed() in try/finally to match the sibling tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts proposal 0093 (spec v0.88.0), the last item in the v0.16.0
retrieval-provider capability. Also folds in an OpenAI batch-chunking bug
that a review of the 0093 diff surfaced, plus a warning for corrupt usage
figures.
Nullable usage records (0093)
One uniform model for "the provider reported no usage" across both
retrieval responses (
EmbeddingResponse.usage,RerankResponse.usage)and both typed events: no usage yields
usage = null, never a fabricatedrecord, zero, or client-side estimate.
The response types and both bundled observers were already correct. The
gap was
OpenAIEmbeddingProvider, which recordedEmbeddingUsage(input_tokens=0)when the OpenAI usage block was absent ormalformed. A zero asserts "zero tokens billed", a claim the provider never
made. It now surfaces
usage = null; a reported zero still yields a recordcarrying 0. A malformed figure reads as not-reported rather than failing
the call, since the vectors are sound and usage is secondary accounting.
TEI, Cohere, and Jina were already conformant.
OpenAI 2048-input chunking (completes 0092)
Reviewing the 0093 diff surfaced a second, unrelated defect. Proposal 0092
requires every capped embedding mapping to chunk-and-stitch and names
OpenAI's 2048-input cap, but the 0092 implementation adopted the shared
helper for TEI and Cohere only. An over-2048 embed call sent one over-cap
request the wire rejects, breaking the arbitrary-length input contract.
OpenAI now chunks at 2048. Conformance stayed green because the spec ships
no over-cap fixture for that mapping (tracked as a follow-up to raise with
spec). The shared
chunk_and_stitch_embedhelper now takes the responseidentity (id and model) from the first chunk, so OpenAI keeps reporting the
provider-returned model while the wires that carry none (TEI's bare array,
Cohere
/v2/embed) fall back to the bound identifier.Warn on a corrupt usage figure
A missing usage figure and a corrupt one were both swallowed silently, so a
gateway sending garbage (a string, a negative, a bool) looked identical to
a healthy no-usage call and the count silently went missing. The shared
nonneg_intreader now logs a warning naming the field when a figure ispresent but not a non-negative int, while an absent figure stays quiet (or
every TEI call would log noise).
nonneg_intlifts into_wire.pyand isthe single reader across OpenAI, Cohere, and Jina.
Conformance
gen_ai.usage.input_tokens) and 140 (Langfuse embeddingusageDetailscarries no
inputkey). Verified they fail without the provider fix.section 11 GenAI token metric is not observed, and the OTel observer has
no embedding-metric path yet (proposal 0067). It defers behind the same
gate as 089 / 109. 0093 changes nothing in section 11.
conformance.tomlmarks 0093 implemented and corrects the 0092 note,which claimed only TEI and Cohere adopted the chunking rule.
Testing
Full suite 1686 passed. Live-tested against the real OpenAI
/v1/embeddingsendpoint (embed plus dimensions). New unit tests coverabsent / malformed / reported-zero usage, the over-2048 chunking (2049
inputs to 2 requests sized 2048/1), and the warn-vs-quiet distinction.