Skip to content

Commit e7e0b29

Browse files
Adopt 0104 retrieval id/error and 0103 chunk cap (#256)
* Adopt 0104 retrieval id/error and 0103 chunk cap 0104: the Jina error mapping now maps a bare 400 to provider_invalid_request instead of the transient provider_unavailable catch-all, aligning it with the TEI / OpenAI / Cohere mappings and avoiding a pointless retry of a request that will not succeed. Jina and Cohere now fold an empty-string response_id to null, since an identifier that correlates nothing is not a present one (extending 0100's malformed-id rule to the empty string); the OpenAI embed mapping already did this. Behavioral at these two edges only; well-formed responses and non-400 errors are unaffected. 0103: OpenAIEmbeddingProvider accepts an optional, test-only chunk_size override of its fixed 2048-input cap (validated positive), so the batch-chunking path can be driven with a small body. Production leaves it unset and the fixed cap applies, so there is no behavior change; the count-based chunking rule (no client-side token estimation) is unchanged. Spec v0.98.0 / v0.99.0 are beyond the current v0.88.0 pin, so both ship ahead of the pin (unit-tested); the conformance fixtures and the manifest entries ride the pin bump. * Note the chunk cap default at the call site
1 parent 9acdd09 commit e7e0b29

5 files changed

Lines changed: 206 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The
2727
- **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.
2828
- **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"]`.
2929
- **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.
30+
- **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.
31+
- **`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.
3032

3133
### Fixed
3234

src/openarmature/retrieval/providers/cohere.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ def _parse_response(
450450
results=scored,
451451
model=model if isinstance(model, str) else self.model,
452452
usage=usage,
453-
response_id=response_id if isinstance(response_id, str) else None,
453+
response_id=response_id if isinstance(response_id, str) and response_id else None,
454454
raw=body,
455455
)
456456

@@ -786,7 +786,13 @@ def _parse_chunk(
786786
response_id = body.get("id")
787787
# The /v2/embed envelope carries an id but no model, so the stitched
788788
# response reports the bound model.
789-
return vectors, input_tokens, response_id if isinstance(response_id, str) else None, None, body
789+
return (
790+
vectors,
791+
input_tokens,
792+
response_id if isinstance(response_id, str) and response_id else None,
793+
None,
794+
body,
795+
)
790796

791797
def _parse_input_tokens(self, body: dict[str, Any]) -> int | None:
792798
"""Extract meta.billed_units.input_tokens, or None.

src/openarmature/retrieval/providers/jina.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,14 @@ def _classify_jina_http_error(resp: httpx.Response) -> LlmProviderError:
140140
return ProviderRateLimit(message or "HTTP 429")
141141
if status == 404:
142142
return ProviderInvalidModel(message or "model not found")
143-
# §8.2 *Errors*: over-length / malformed request (422) ->
144-
# provider_invalid_request (the fail-loud surface, fixture 021). Jina lists
145-
# only 422 here (no 400 / 413, unlike the TEI / OpenAI sibling mappings), so
146-
# a 400 falls through to provider_unavailable per the §8.2 enumeration.
147-
if status == 422:
148-
return ProviderInvalidRequest(message or "HTTP 422")
143+
# §8.2 *Errors* (proposal 0104): a malformed / over-length request (400 /
144+
# 422) -> provider_invalid_request, the fail-loud surface. This aligns Jina
145+
# with the §8.1 / §8.3 / §8.4 mappings, which all map 400 ->
146+
# provider_invalid_request. Before 0104, §8.2 enumerated only 422, so a bare
147+
# 400 fell through to the transient provider_unavailable catch-all and
148+
# invited a pointless retry of a request that will not succeed on retry.
149+
if status in (400, 422):
150+
return ProviderInvalidRequest(message or f"HTTP {status}")
149151
return ProviderUnavailable(message or f"HTTP {status}")
150152

151153

@@ -432,7 +434,7 @@ def _parse_response(
432434
vectors=vectors,
433435
model=model if isinstance(model, str) else self.model,
434436
usage=usage,
435-
response_id=response_id if isinstance(response_id, str) else None,
437+
response_id=response_id if isinstance(response_id, str) and response_id else None,
436438
dimensions=dimensions,
437439
raw=body,
438440
)
@@ -688,7 +690,7 @@ def _parse_response(
688690
results=scored,
689691
model=model if isinstance(model, str) else self.model,
690692
usage=usage,
691-
response_id=response_id if isinstance(response_id, str) else None,
693+
response_id=response_id if isinstance(response_id, str) and response_id else None,
692694
raw=body,
693695
)
694696

src/openarmature/retrieval/providers/openai.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@
5454

5555
# §8.3 *Batch chunking (2048-input cap)*: OpenAI /v1/embeddings accepts at most
5656
# 2048 inputs per request, so an over-cap embed call chunk-and-stitches over
57-
# consecutive <=2048 slices (the §8 general embed rule; a fixed vendor cap like
58-
# Cohere's 96, not a construction-configured chunk_size like TEI). OpenAI also
57+
# consecutive slices of the cap (the §8 general embed rule). The cap is a fixed
58+
# vendor 2048 (like Cohere's 96), not a real construction config like TEI's
59+
# chunk_size; __init__ exposes a TEST-ONLY chunk_size override of it (proposal
60+
# 0103) so a fixture can drive the chunking path with a small body. OpenAI also
5961
# enforces a summed-token ceiling per request, which the count-based rule does
6062
# not address: an over-token chunk still fails loud as provider_invalid_request
6163
# (§8's rule chunks by input count).
@@ -156,6 +158,7 @@ def __init__(
156158
query_prefix: str | None = None,
157159
document_prefix: str | None = None,
158160
populate_caller_metadata: bool = True,
161+
chunk_size: int | None = None,
159162
) -> None:
160163
# base_url is the host root; the provider appends the /v1 routes, so
161164
# a trailing /v1 would produce a doubled /v1/v1 path that 404s (the
@@ -179,6 +182,14 @@ def __init__(
179182
# ``genai_system`` surfaces as gen_ai.system on the embedding span.
180183
self._genai_system = genai_system
181184
self._populate_caller_metadata = populate_caller_metadata
185+
# §8.3's OpenAI input cap is a FIXED vendor 2048, not a construction
186+
# config like TEI's max-client-batch-size. ``chunk_size`` is a TEST-ONLY
187+
# override of that fixed cap (conformance-adapter §5.14, proposal 0103)
188+
# so a fixture can drive the chunk-and-stitch path with a small body;
189+
# production leaves it None and the fixed 2048 applies.
190+
if chunk_size is not None and chunk_size <= 0:
191+
raise ValueError(f"chunk_size must be positive (got {chunk_size})")
192+
self._chunk_size = chunk_size if chunk_size is not None else _OPENAI_EMBED_MAX_INPUTS
182193
self._headers: dict[str, str] = {"Content-Type": "application/json"}
183194
if api_key is not None:
184195
self._headers["Authorization"] = f"Bearer {api_key}"
@@ -343,7 +354,9 @@ async def _embed_one(
343354
return await chunk_and_stitch_embed(
344355
input_strings,
345356
model=self.model,
346-
cap=_OPENAI_EMBED_MAX_INPUTS,
357+
# The fixed vendor 2048 cap unless a test / conformance harness set a
358+
# chunk_size override (0103); see __init__.
359+
cap=self._chunk_size,
347360
embed_chunk=_embed_one,
348361
)
349362

tests/unit/test_retrieval_provider.py

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

447447

448+
async def test_openai_embed_chunk_size_override_drives_chunking() -> None:
449+
# 0103: OpenAI's 2048 cap is fixed, not construction-configurable, so a
450+
# test-only chunk_size override drives the chunk-and-stitch path with a small
451+
# body -- 5 inputs, chunk_size 2 => 3 requests [2, 2, 1], consecutive slices,
452+
# vectors stitched in input order, usage summed, response identity from the
453+
# first chunk. Mirror of fixture 043 case 1 (which rides the pin bump).
454+
inputs = [f"e{i}" for i in range(5)]
455+
chunks = [
456+
[[0.0, 0.5], [0.1, 0.5]],
457+
[[0.2, 0.5], [0.3, 0.5]],
458+
[[0.4, 0.5]],
459+
]
460+
responses = iter(
461+
[
462+
_openai_embed_body(
463+
data=[{"object": "embedding", "index": i, "embedding": v} for i, v in enumerate(c)],
464+
prompt_tokens=2,
465+
)
466+
| {"id": f"chunk-{n}"}
467+
for n, c in enumerate(chunks)
468+
]
469+
)
470+
captured: list[dict[str, Any]] = []
471+
472+
def handler(req: httpx.Request) -> httpx.Response:
473+
captured.append(json.loads(req.content))
474+
assert req.url.path == "/v1/embeddings"
475+
return httpx.Response(200, json=next(responses))
476+
477+
provider = _openai_embed_provider(handler, chunk_size=2)
478+
try:
479+
response = await provider.embed(inputs)
480+
finally:
481+
await provider.aclose()
482+
483+
assert len(captured) == 3
484+
assert [len(b["input"]) for b in captured] == [2, 2, 1]
485+
assert captured[0]["input"] == inputs[0:2]
486+
assert captured[1]["input"] == inputs[2:4]
487+
assert captured[2]["input"] == inputs[4:5]
488+
assert response.vectors == chunks[0] + chunks[1] + chunks[2]
489+
assert response.usage is not None
490+
assert response.usage.input_tokens == 6 # 2 per chunk, summed across 3
491+
assert response.response_id == "chunk-0" # the first chunk's id
492+
493+
494+
def test_openai_embed_chunk_size_must_be_positive() -> None:
495+
# 0103: an override <= 0 is rejected at construction (mirrors TEI's guard).
496+
def _never(_req: httpx.Request) -> httpx.Response: # pragma: no cover - construction raises first
497+
raise AssertionError("must not send when construction rejects chunk_size")
498+
499+
with pytest.raises(ValueError, match="chunk_size must be positive"):
500+
_openai_embed_provider(_never, chunk_size=0)
501+
502+
503+
async def test_openai_embed_chunked_mid_chunk_failure_fails_loud() -> None:
504+
# 0103 case 2: when one chunk of a chunked call fails, the whole call fails
505+
# loud -- no partial stitch, no EmbeddingResponse. 3 inputs, chunk_size 2 =>
506+
# count-chunks [2, 1]; chunk A returns 200, chunk B returns 400 (over-token).
507+
# Both chunk requests reach the wire before the ProviderInvalidRequest raises
508+
# (no client-side token estimation would have split chunk B).
509+
call = 0
510+
511+
def handler(_req: httpx.Request) -> httpx.Response:
512+
nonlocal call
513+
call += 1
514+
if call == 1:
515+
return httpx.Response(
516+
200,
517+
json=_openai_embed_body(
518+
data=[{"object": "embedding", "index": i, "embedding": [0.0, 0.5]} for i in range(2)],
519+
prompt_tokens=2,
520+
),
521+
)
522+
return httpx.Response(400, json={"error": {"message": "input exceeds the token ceiling"}})
523+
524+
provider = _openai_embed_provider(handler, chunk_size=2)
525+
with pytest.raises(ProviderInvalidRequest):
526+
await provider.embed(["a", "b", "c"])
527+
await provider.aclose()
528+
# Both chunk requests reached the wire; the count-chunking did not
529+
# short-circuit or token-sub-chunk chunk B.
530+
assert call == 2
531+
532+
448533
async def test_openai_embed_chunked_usage_is_all_or_nothing() -> None:
449534
# Usage is summed ONLY when every chunk reports it. A 2049-input call
450535
# (2 chunks) where chunk 1 reports prompt_tokens but chunk 2 omits the usage
@@ -1386,6 +1471,29 @@ def handler(req: httpx.Request) -> httpx.Response:
13861471
assert captured[0]["truncate"] == "NONE"
13871472

13881473

1474+
async def test_cohere_embed_empty_response_id_is_null() -> None:
1475+
# 0104: an empty-string id is not a present identifier -> null (it correlates
1476+
# nothing), distinct from 0097's empty-document echo which stays present.
1477+
def handler(_req: httpx.Request) -> httpx.Response:
1478+
return httpx.Response(200, json=_cohere_embed_body(id="", vectors=[[0.1, 0.2]], input_tokens=3))
1479+
1480+
provider = _cohere_embed_provider(handler)
1481+
response = await provider.embed(["x"])
1482+
await provider.aclose()
1483+
assert response.response_id is None
1484+
1485+
1486+
async def test_cohere_rerank_empty_response_id_is_null() -> None:
1487+
# 0104: the empty-string id -> null rule holds on the rerank surface too.
1488+
def handler(_req: httpx.Request) -> httpx.Response:
1489+
return httpx.Response(200, json=_rerank_body(id="", results=[{"index": 0, "relevance_score": 0.9}]))
1490+
1491+
provider = _rerank_provider(handler)
1492+
response = await provider.rerank("q", ["a", "b"])
1493+
await provider.aclose()
1494+
assert response.response_id is None
1495+
1496+
13891497
async def test_cohere_embed_no_usage_object_yields_null_usage() -> None:
13901498
# Cohere reports usage, but a compatible backend that omits meta yields
13911499
# usage = null (§4 -- never fabricate a record or a zero).
@@ -2331,6 +2439,68 @@ def handler(_req: httpx.Request) -> httpx.Response:
23312439
await rprovider.aclose()
23322440

23332441

2442+
@pytest.mark.parametrize("surface", ["embed", "rerank"])
2443+
async def test_jina_400_maps_to_invalid_request(surface: str) -> None:
2444+
# §8.2 *Errors* (proposal 0104): a bare 400 -> provider_invalid_request on
2445+
# BOTH surfaces, aligning Jina with §8.1 / §8.3 / §8.4. Before 0104 it fell
2446+
# through to the transient provider_unavailable catch-all (only 422 was
2447+
# enumerated), inviting a pointless retry of a request that will not succeed.
2448+
def handler(_req: httpx.Request) -> httpx.Response:
2449+
return httpx.Response(400, json={"detail": "Bad request."})
2450+
2451+
with pytest.raises(ProviderInvalidRequest):
2452+
if surface == "embed":
2453+
provider = _jina_embed_provider(handler)
2454+
try:
2455+
await provider.embed(["x"])
2456+
finally:
2457+
await provider.aclose()
2458+
else:
2459+
rprovider = _jina_rerank_provider(handler)
2460+
try:
2461+
await rprovider.rerank("q", ["a", "b"])
2462+
finally:
2463+
await rprovider.aclose()
2464+
2465+
2466+
async def test_jina_embed_empty_response_id_is_null() -> None:
2467+
# 0104: an empty-string id is not a present identifier -> null.
2468+
def handler(_req: httpx.Request) -> httpx.Response:
2469+
return httpx.Response(
2470+
200,
2471+
json={
2472+
"id": "",
2473+
"model": "jina-embeddings-test",
2474+
"usage": {"total_tokens": 8},
2475+
"data": [{"index": 0, "embedding": [0.1, 0.2]}],
2476+
},
2477+
)
2478+
2479+
provider = _jina_embed_provider(handler)
2480+
response = await provider.embed(["x"])
2481+
await provider.aclose()
2482+
assert response.response_id is None
2483+
2484+
2485+
async def test_jina_rerank_empty_response_id_is_null() -> None:
2486+
# 0104: the empty-string id -> null rule holds on the rerank surface too.
2487+
def handler(_req: httpx.Request) -> httpx.Response:
2488+
return httpx.Response(
2489+
200,
2490+
json={
2491+
"id": "",
2492+
"model": "jina-reranker-test",
2493+
"usage": {"total_tokens": 3},
2494+
"results": [{"index": 0, "relevance_score": 0.9}],
2495+
},
2496+
)
2497+
2498+
provider = _jina_rerank_provider(handler)
2499+
response = await provider.rerank("q", ["a", "b"])
2500+
await provider.aclose()
2501+
assert response.response_id is None
2502+
2503+
23342504
async def test_jina_rerank_over_length_422_maps_to_invalid_request() -> None:
23352505
# §8.2 fail-loud: truncation: false makes Jina error on over-length input
23362506
# (422); the mapping surfaces provider_invalid_request.

0 commit comments

Comments
 (0)