Skip to content

Adopt 0093 nullable provider usage records#218

Merged
chris-colinsky merged 4 commits into
mainfrom
feature/0093-nullable-usage-records
Jul 18, 2026
Merged

Adopt 0093 nullable provider usage records#218
chris-colinsky merged 4 commits into
mainfrom
feature/0093-nullable-usage-records

Conversation

@chris-colinsky

Copy link
Copy Markdown
Member

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 fabricated
record, zero, or client-side estimate.

The response types and both bundled observers were already correct. The
gap was OpenAIEmbeddingProvider, which recorded
EmbeddingUsage(input_tokens=0) when the OpenAI 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. 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_embed 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 (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_int reader now logs a warning naming the field when a figure is
present but not a non-negative int, while an absent figure stays quiet (or
every TEI call would log noise). nonneg_int lifts into _wire.py and is
the single reader across OpenAI, Cohere, and Jina.

Conformance

  • Un-defers observability fixtures 139 (OTel embedding span omits
    gen_ai.usage.input_tokens) and 140 (Langfuse embedding usageDetails
    carries no input key). Verified they fail without the provider fix.
  • Fixture 143 stays deferred, and not for a 0093 reason: it asserts a
    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.toml marks 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/embeddings endpoint (embed plus dimensions). New unit tests cover
absent / malformed / reported-zero usage, the over-2048 chunking (2049
inputs to 2 requests sized 2048/1), and the warn-vs-quiet distinction.

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.
Copilot AI review requested due to automatic review settings July 18, 2026 00:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = null when 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_embed helper.
  • 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.

Comment thread src/openarmature/retrieval/providers/cohere.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/openarmature/retrieval/providers/openai.py
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.
Copilot AI review requested due to automatic review settings July 18, 2026 01:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread conformance.toml Outdated
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.
Copilot AI review requested due to automatic review settings July 18, 2026 01:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread tests/unit/test_retrieval_provider.py Outdated
Comment thread tests/unit/test_retrieval_provider.py Outdated
Comment thread tests/unit/test_retrieval_provider.py Outdated
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.
Copilot AI review requested due to automatic review settings July 18, 2026 02:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@chris-colinsky
chris-colinsky merged commit 45ff723 into main Jul 18, 2026
7 checks passed
@chris-colinsky
chris-colinsky deleted the feature/0093-nullable-usage-records branch July 18, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants