Tolerate a Chutes stream chunk that omits id - #984
Conversation
Reproduced against production: streaming zai-org/GLM-5.1-FP8 with a large
input context falls back to Chutes, whose decrypted frames omit `id`. The
shared ChatCompletionChunk requires it, so a single missing scalar killed an
otherwise healthy stream:
data: {"error":{"message":"Failed to perform completion: Chutes stream
chunk parse: missing field `id` at line 1 column 184", ...}}
data: [DONE]
Delivered as HTTP 200 with the error in-band and [DONE] after it, so a client
keying on [DONE] recorded a successful empty completion. At 96K the same
request returns a clean 400, so this path is a defect rather than a limit.
The fix is contained to the Chutes boundary. ChatCompletionChunk is shared by
every provider; making `id` optional there would weaken validation fleet-wide
and let a genuinely malformed chunk from any backend parse silently. Instead
the frame is deserialized to a Value, a missing `id` is filled, and the typed
chunk is built from that. Unknown fields already survive via the flatten map -
verified by test, since DeepSeek emits `prompt_text` at 400K context.
`raw_bytes` are unchanged and asserted byte-identical: those bytes are
signature-relevant and must pass through verbatim.
Handles the consequence: extract_inference_id_from_chunk previously hashed
whatever it was given, so an empty id produced the *same* inference id for
every affected stream - worse than none, because it looks valid and would
corrupt attestation lookups. It now returns Option<Uuid> and absent stays
absent, matching how Inference-Id is already omitted when a stream fails
before its first chunk. This inverts an existing unit test that asserted the
old behaviour; the assertion was encoding the bug.
Verified: cargo clippy -p inference_providers -p services -p api --all-targets
-D warnings clean; cargo test -p inference_providers --lib chutes 112 passed;
cargo test -p api --lib extract_inference_id 4 passed. E2E tests were not run -
they require PostgreSQL and a dstack/TEE socket, neither available here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fm9f3maaY2wd6yHH8FEjR5
Review —
|
Review · Status🟩 CompletedIronLoop completed the review and posted it to GitHub. ResultRun detailsAutomatic trigger · attempt 1 of 3 · completed in 25m 59s |
|
✅ OpenCodeReview: Review failed: 0 finding(s); 2 of 2 selected item(s) failed. |
There was a problem hiding this comment.
Review · Summary
Two issues remain in the new id-less Chutes stream path.
Findings: 🔴 High 1 · 🟠 Medium 1
Code-specific findings are attached to the diff.
Validation
- ✅ Chutes E2EE stream unit tests — 12 focused parser and stream tests passed.
- ✅ Inference-ID extraction unit tests — 4 focused API tests passed.
Review details
- Run:
0050226e-1a1d-4965-a06b-f4a281690d85 - Attempts: 1
| if let Some(object) = chunk_json.as_object_mut() { | ||
| object | ||
| .entry("id") | ||
| .or_insert_with(|| serde_json::Value::String(String::new())); |
There was a problem hiding this comment.
🔴 High · Keep id-less chunks synchronized after sanitization
The placeholder is inserted only in the local value used to build chunk; raw_bytes remains id-less. The downstream Chutes rewrite sanitizes that raw JSON but cannot deserialize it back into ChatCompletionChunk, so it retains the original chunk. The normal attested streaming path reserializes that stale chunk for usage shaping, which can re-expose fields the Chutes allowlist removed (such as prompt_token_ids and prompt_sha256) and lose the canonical model rewrite. Normalize the value used by downstream typed rebuilds too, or replace the parsed chunk after sanitization.
| if let Some(object) = chunk_json.as_object_mut() { | ||
| object | ||
| .entry("id") | ||
| .or_insert_with(|| serde_json::Value::String(String::new())); |
There was a problem hiding this comment.
🟠 Medium · Do not use an empty provider id as a signature key
For a default attestation-supported stream, usage shaping enables gateway signing. If the first accepted Chutes chunk has no id, its parsed id is "", and the stream tail stores gateway signatures under that key. The signature table upserts on (chat_id, signing_algo), so each affected completion overwrites the prior completion's signatures and cannot retain a per-completion signature. Skip storage when the provider id is absent, or mint and return a unique gateway identifier.
PierreLeGuen
left a comment
There was a problem hiding this comment.
Containing the tolerance to the Chutes boundary is the right call, but raw_bytes still omit id, so downstream re-derives (rewrite_sse_event_model, think extraction) silently no-op and unsanitized Chutes internals reach clients; the empty id also flows on as a real chat id, collapsing the usage idempotency key to…
Blocking findings:
crates/inference_providers/src/attested/chutes/e2ee_stream.rs:92— The missingidis filled only in the localchunk_jsonused to build the typed chunk; the emittedraw_bytes(data. Impact: For exactly the id-less streams this PR unblocks,rewrite_sse_event_modelnever replacesev.chunk`, so the parsed… Fix: Do not leave the frame id-less downstream.
Checks: cargo +1.92.0 test -p inference_providers --lib chutes — 112 passed, 0 failed, 1 ignored (matches the PR's stated result); cargo +1.92.0 test -p api --lib extract_inference_id — 4 passed, 0 failed, including the deliberately inverted test_extract_inference_id_from_chunk_empty_id; cargo +1.92.0 clippy -p inference_providers -p api --all-targets -- -D warnings — clean
Two review findings, both about consequences downstream of the parse. High: the id placeholder was inserted only into the value used to build the typed chunk, so raw_bytes stayed id-less. rewrite_sse_event_model reads raw_bytes, sanitizes against the Chutes allowlist and applies the canonical model rewrite, then rebuilds the typed chunk from it - and that rebuild failed on an id-less frame, so it kept the ORIGINAL chunk. The attested path then reserializes that stale chunk for usage shaping, which can re-expose fields the allowlist strips (prompt_token_ids, prompt_sha256) and loses the canonical model rewrite. The tolerance could therefore leak provider internals on exactly the streams it unblocks. Medium: an empty id became the signature key. Gateway signatures upsert on (chat_id, signing_algo), so every affected completion overwrote the previous one's row and no per-completion signature survived. Both are fixed by minting a stable synthetic id per response session - `chutes-gateway-<uuid>` - used when the frame omits `id` or carries an empty one. It is stable across every chunk of a stream so chat_id grouping, sticky routing and signature storage all behave; unique per completion so signature rows cannot collide; and visibly synthetic so it cannot be mistaken for a provider id. Raw and typed representations are both built from the value carrying that id, so every downstream round-trip through raw_bytes succeeds. Re-serialization is deliberately conditional. serde_json here has no preserve_order feature, so Value::Object is a BTreeMap and a round-trip sorts keys alphabetically. Re-serializing unconditionally would have changed the wire bytes of every Chutes frame, not just the id-less ones this PR exists to fix. Frames that already carry an id are emitted byte-for-byte as before, guarded by a regression test that fails against the unconditional version. Verified: cargo clippy -p inference_providers --all-targets -D warnings clean; cargo test -p inference_providers --lib chutes 116 passed, 0 failed. The sanitization regression test fails without the fix. E2E not run - needs PostgreSQL and a dstack/TEE socket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fm9f3maaY2wd6yHH8FEjR5
|
Both findings addressed in 🔴 High — raw and typed chunks now derive from the same valueYou were right that the divergence was the bug. The placeholder was inserted only into the value used to build the typed chunk, so Both representations are now built from the value carrying the id, so every downstream round-trip through 🟠 Medium — minted a stable per-stream id instead of an empty stringTook the "mint a unique gateway identifier" option, since it fixes both findings at once.
One thing my own verification caught before this landedThe first version re-serialized every frame. Re-serialization is now conditional: frames that already carry an id are emitted byte-for-byte as before. Guarded by a byte-identity test written to fail against the unconditional version. Verification
E2E not run locally — needs PostgreSQL and a dstack/TEE socket; both tests panic at setup on |
PierreLeGuen
left a comment
There was a problem hiding this comment.
Normalizing a missing or empty id at the Chutes E2EE boundary is correct and coherent. One non-blocking gap: a JSON-null id is not normalized.
Optional follow-ups:
crates/inference_providers/src/attested/chutes/e2ee_stream.rs:94— The new normalization treats a JSON-nullidas present. Fix: Fold null into the replaced set so absent, empty, and null normalize identically.
Checks: cargo +1.92.0 clippy -p inference_providers -p services -p api --all-targets -- -D warnings — clean, no warnings (a parallel attempt in a second environment aborted on a read-only…; cargo +1.92.0 test -p inference_providers --lib — 424 passed, 0 failed, 1 ignored; cargo +1.92.0 test -p inference_providers --lib chutes — 116 passed, 0 failed (includes the new e2ee_stream tests)
Review follow-up. The match treated Value::Null as a present id:
None => true,
Some(Value::String(id)) => id.is_empty(),
Some(_) => false, // <- "id": null landed here
so a frame carrying `"id": null` got no synthetic id, and the next statement -
from_value::<ChatCompletionChunk> - then failed, because ChatCompletionChunk::id
is a String and cannot deserialize from null. That reproduced the exact
"Chutes stream chunk parse" error this PR exists to prevent, just via a
different frame shape than the one found in production.
Absent, empty and null now normalize identically. Re-serialization stays
conditional, so frames carrying a real id are still emitted byte-for-byte.
Verified: cargo clippy -p inference_providers --all-targets -D warnings clean;
cargo test -p inference_providers --lib chutes 117 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fm9f3maaY2wd6yHH8FEjR5
|
@PierreLeGuen follow-up addressed — a JSON-null You were right that it was a gap rather than a nicety: Re-serialization stays conditional, so frames carrying a real id are still emitted byte-for-byte identical — the byte-identity test is unchanged and green. Verified: |
Split out of #952 so the status fix and this parser fix can be reviewed separately.
The bug
Reproduced against production. Streaming
zai-org/GLM-5.1-FP8, varying only the input context size:x-serving-providernear[DONE]Above a context threshold the request falls back to Chutes, whose decrypted frames omit
id.ChatCompletionChunkrequires it, so one missing scalar kills an otherwise healthy stream atchutes/e2ee_stream.rs:88.That the 96K path already returns a clean 400 is what makes this a defect rather than a limit — the system can reject properly; this path instead falls back to a provider it cannot parse.
Why the fix is contained to the Chutes boundary
ChatCompletionChunkis shared by every provider. Makingidoptional there would weaken validation fleet-wide and let a genuinely malformed chunk from any backend parse silently. So the frame is deserialized to aValue, a missingidis filled, and the typed chunk is built from that.models.rsis untouched.Unknown fields already survive via the flatten map — covered by a new test, since DeepSeek emits an extra
prompt_textfield at 400K context. Provider chunk shapes genuinely diverge: DeepSeek at 300K returned"id":"c678a193…", at 400K"id":"chatcmpl-6ce9a499…"plus"prompt_text":null.raw_bytesare asserted byte-identical to the input — those bytes are signature-relevant and must pass through verbatim.The part worth a careful look
extract_inference_id_from_chunkpreviously hashed whatever it was given, so an emptyidproduced the same inference id for every affected stream — worse than having none, because it looks valid and would corrupt attestation lookups. It now returnsOption<Uuid>, and absent stays absent, matching howInference-Idis already omitted when a stream fails before its first chunk.This inverts an existing unit test —
test_extract_inference_id_from_chunk_empty_idasserted that an empty id should still yield a non-nil UUID. That assertion was encoding the bug. Flagging it rather than letting it pass as a routine test edit.Tests added
idparses, yields a usable chunk, andraw_bytesare byte-identicalidyields no inference id rather than a hash of the empty stringVerification
cargo clippy -p inference_providers -p services -p api --all-targets -- -D warnings— cleancargo test -p inference_providers --lib chutes— 112 passed, 0 failedcargo test -p api --lib extract_inference_id— 4 passed, 0 failedE2E tests were not executed locally — they require PostgreSQL and a dstack/TEE socket; both tests panic at setup on
Failed to derive signing keys from dstack. CI is the judge there.Interaction with #952
Independent changes, but both touch
extract_inference_id_from_chunkand its call sites. Whichever merges second needs a trivial merge in that function. #952 makes a pre-first-event error return a real HTTP status; this PR makes the stream not fail in the first place.Suggested acceptance test post-merge
Against staging, varying only context size on
zai-org/GLM-5.1-FP8: 128K/200K should stream normally instead of returning 200 + in-band error; 96K should still return 400; healthy control unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fm9f3maaY2wd6yHH8FEjR5