Skip to content

Commit 0301843

Browse files
Harden 0093 chunk stitching per review
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.
1 parent 2c36b77 commit 0301843

4 files changed

Lines changed: 72 additions & 14 deletions

File tree

src/openarmature/retrieval/_wire.py

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939
# misbehaving on a field it did populate, so it logs at WARNING before returning
4040
# None -- otherwise a corrupt count is indistinguishable from the healthy absent
4141
# case and usage silently goes missing. ``field`` names the figure in the log.
42+
# Under batch chunking this fires once per chunk, so a persistently-corrupt
43+
# gateway logs N warnings for one embed() call; that per-chunk multiplicity is
44+
# accepted -- deduping would move the warning out of this single-request-shaped
45+
# reader and lose it for the non-chunked providers (Cohere / Jina rerank).
4246
def nonneg_int(value: Any, *, field: str = "usage figure") -> int | None:
4347
"""Return ``value`` when it is a non-negative int, else ``None``.
4448
@@ -135,8 +139,9 @@ def apply_client_side_prefix(
135139
# chunked input list identical across slices, concatenate the per-chunk vectors
136140
# IN INPUT ORDER (so §4's one-vector-per-input + input-order invariants hold
137141
# across the whole call), and combine the per-chunk usage per §4's nullable
138-
# usage contract -- sum input_tokens when the provider reports usage, else
139-
# usage = null. response_id is the FIRST chunk's id (a single-request call uses
142+
# usage contract ALL-OR-NOTHING -- sum input_tokens only when EVERY chunk reports
143+
# a figure, else usage = null (a mixed report is an unaccountable total, never a
144+
# partial sum). response_id is the FIRST chunk's id (a single-request call uses
140145
# that request's id). raw is that one response for a single-request call, or the
141146
# LIST of per-chunk responses in request order for a chunked call (proposal
142147
# 0096). embed_chunk owns the per-mapping wire shaping / POST / parse and returns
@@ -166,6 +171,12 @@ async def chunk_and_stitch_embed(
166171
chunk; the per-mapping wire shaping, POST, and parse live in the closure.
167172
``model`` is the bound identifier, reported when the chunk bodies carry no
168173
model of their own. Returns the stitched :class:`EmbeddingResponse`.
174+
175+
A chunked call issues its per-chunk requests sequentially and is NOT atomic:
176+
if a later chunk fails, the earlier chunks have already been sent (and billed
177+
by the provider) but their vectors are discarded and the call raises. There
178+
is no rollback over a non-transactional wire, and the failure event carries
179+
no usage, so a mid-call failure may leave earlier chunks billed unobservably.
169180
"""
170181
# cap is the provider's per-call input limit; a non-positive cap is a caller
171182
# misconfiguration -- fail loudly rather than surfacing a raw range() error
@@ -180,7 +191,8 @@ async def chunk_and_stitch_embed(
180191
validate_embedding_input(input_strings)
181192
stitched_vectors: list[list[float]] = []
182193
chunk_bodies: list[Any] = []
183-
input_tokens_total: int | None = None
194+
input_tokens_total = 0
195+
usage_fully_reported = True
184196
response_id: str | None = None
185197
response_model: str | None = None
186198
for offset in range(0, len(input_strings), cap):
@@ -202,15 +214,23 @@ async def chunk_and_stitch_embed(
202214
if offset == 0:
203215
response_id = chunk_id
204216
response_model = chunk_model
205-
# Sum input_tokens across chunks; a chunk that omits usage does not
206-
# contribute, and usage stays null only when NO chunk reports it (§4 /
207-
# §8 batch-chunking step 4 -- never fabricate).
208-
if chunk_tokens is not None:
209-
input_tokens_total = (input_tokens_total or 0) + chunk_tokens
217+
# Sum input_tokens ALL-OR-NOTHING: a total is reported only when EVERY
218+
# chunk reported one. Any chunk that omits usage (a None figure, incl. an
219+
# absent OR a corrupt one via nonneg_int) makes the whole-call total
220+
# unaccountable -- so usage = null rather than a partial sum that would
221+
# present a confident undercount and diverge from the single-request
222+
# path (which returns null for the same unaccountable figure). All report
223+
# (hosted OpenAI / Cohere) -> sum; none report (TEI) -> null; mixed (an
224+
# inconsistent gateway) -> null (§4 / §8 batch-chunking step 4 -- never
225+
# fabricate a figure you cannot fully account for).
226+
if chunk_tokens is None:
227+
usage_fully_reported = False
228+
else:
229+
input_tokens_total += chunk_tokens
210230
# §4 cross-impl invariants (one vector per input, uniform dimensionality)
211231
# are enforced against the STITCHED result.
212232
dimensions = validate_embedding_response(stitched_vectors, len(input_strings))
213-
usage = EmbeddingUsage(input_tokens=input_tokens_total) if input_tokens_total is not None else None
233+
usage = EmbeddingUsage(input_tokens=input_tokens_total) if usage_fully_reported else None
214234
# raw is the verbatim deserialized response (§4 / §8 per proposal 0096, dict
215235
# | list): a single call carries that response's body; a chunked call carries
216236
# the LIST of per-chunk bodies in request order.

src/openarmature/retrieval/providers/cohere.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -616,9 +616,11 @@ async def _embed_chunked(
616616
# general embed rule) via the shared chunk_and_stitch_embed helper. The
617617
# per-chunk closure builds the /v2/embed body with IDENTICAL per-call
618618
# params (only texts differs), POSTs, classifies the HTTP error, and
619-
# parses the chunk into (vectors, input_tokens, id, body); the helper
620-
# loops the consecutive <=96 slices, concatenates the vectors IN INPUT
621-
# ORDER, sums input_tokens, takes response_id from the FIRST chunk, and
619+
# parses the chunk into (vectors, input_tokens, id, model, body) -- model
620+
# is always None (the /v2/embed envelope carries no model), so the stitch
621+
# falls back to the bound model; the helper loops the consecutive <=96
622+
# slices, concatenates the vectors IN INPUT ORDER, sums input_tokens,
623+
# takes the response identity (id + model) from the FIRST chunk, and
622624
# validates the §4 invariants against the stitched result. When
623625
# len(input) <= 96 this issues a single request. Valid because each
624626
# input's embedding is independent of the others in its batch.

src/openarmature/retrieval/providers/openai.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,13 +446,17 @@ def _parse_chunk(
446446
if isinstance(usage_block, dict)
447447
else None
448448
)
449+
# An empty-string id / model carries no provider identifier (§4: the
450+
# reported identifier is used only when the body carries one), so treat
451+
# "" as absent -> None. For the model that lets the stitch fall back to
452+
# the bound identifier rather than surfacing "".
449453
response_id = body.get("id")
450454
model = body.get("model")
451455
return (
452456
vectors,
453457
input_tokens,
454-
response_id if isinstance(response_id, str) else None,
455-
model if isinstance(model, str) else None,
458+
response_id if isinstance(response_id, str) and response_id else None,
459+
model if isinstance(model, str) and model else None,
456460
body,
457461
)
458462

tests/unit/test_retrieval_provider.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,38 @@ def handler(req: httpx.Request) -> httpx.Response:
445445
assert isinstance(response.raw, dict)
446446

447447

448+
async def test_openai_embed_chunked_usage_is_all_or_nothing() -> None:
449+
# Usage is summed ONLY when every chunk reports it. A 2049-input call
450+
# (2 chunks) where chunk 1 reports prompt_tokens but chunk 2 omits the usage
451+
# block yields usage = null for the WHOLE call, not a confident partial sum:
452+
# the total is unaccountable, and a partial sum would both undercount and
453+
# diverge from the single-request path (which returns null for the same
454+
# unaccountable figure). Guards against the chunked/unchunked inconsistency.
455+
inputs = [f"doc-{i:04d}" for i in range(2049)]
456+
chunk_a = [[i / 10000, 0.5] for i in range(2048)]
457+
body_a = _openai_embed_body(
458+
data=[{"object": "embedding", "index": i, "embedding": v} for i, v in enumerate(chunk_a)],
459+
prompt_tokens=4096,
460+
)
461+
body_b = _openai_embed_body(data=[{"object": "embedding", "index": 0, "embedding": [0.2048, 0.5]}])
462+
del body_b["usage"] # chunk 2 reports no usage block at all
463+
responses = iter([body_a, body_b])
464+
465+
def handler(req: httpx.Request) -> httpx.Response:
466+
assert req.url.path == "/v1/embeddings"
467+
return httpx.Response(200, json=next(responses))
468+
469+
provider = _openai_embed_provider(handler)
470+
try:
471+
response = await provider.embed(inputs)
472+
finally:
473+
await provider.aclose()
474+
475+
assert len(response.vectors) == 2049
476+
# Chunk 2 reported nothing, so the whole-call total is unaccountable -> null.
477+
assert response.usage is None
478+
479+
448480
# --- nonneg_int: warn on a present-but-corrupt usage figure (proposal 0093) --
449481
#
450482
# The shared usage-figure reader distinguishes "not reported" (None -- the

0 commit comments

Comments
 (0)