Uh oh!
There was an error while loading. Please reload this page.
Read an explicit null sequence as an empty one - #4
Conversation
Several OpenAI-compatible endpoints spell "no tool calls" as "tool_calls": null rather than by omitting the key; Mistral-family models do it on every plain-text completion. #[serde(default)] covers only an absent key, so such a body failed the whole decode with "invalid type: null, expected a sequence" and a model that simply answered in prose surfaced as a transport fault. Deserialize every sequence the provider sends through Option, the same tolerance deserialize_arguments already applies to a tool call's arguments: choices, tool_calls on both the unary and streaming paths, the Responses API's output/content/summary, and the model listing. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughOpenAI response collection fields now deserialize explicit JSON ChangesOpenAI null collection handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This localized change makes explicit null sequence fields decode as empty collections, with targeted tests and passing checks; no actionable merge-blocking risk remains, so it is merge-ready after normal checks. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0140 · 85,077 in / 1,741 out · 20,745 cached (24%) · deepseek/deepseek-v4-flash, openrouter/openai/text-embedding-3-small, z-ai/glm-5.2 · 302 embedded
critique: $0.0031 · 33,958 in / 365 out · 0 cached (0%) · deepseek/deepseek-v4-flash
security: $0.0070 · 32,603 in / 433 out · 17,145 cached (53%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
tests: $0.0011 · 12,537 in / 114 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0026 · 4,821 in / 534 out · 3,600 cached (75%) · z-ai/glm-5.2
How this change flows2 changed behaviours across 15 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 43 further behaviours left out to keep the diagram readable. flowchart LR
n0["...400_unions_with_existing_baseline_degrade<br/>changed"]:::changed
n1["ChatCompletionChunk<br/>changed"]:::changed
n2["...ne_knobs_default_to_supported_wire_shapes"]:::impacted
n3["translates_request_to_openai_json_shape"]:::impacted
n4["with_response_format"]:::impacted
n5["ingest"]:::impacted
n6["model"]:::impacted
n7["translate_request"]:::impacted
n0 -->|calls| n4
n0 -->|tests| n4
n2 -->|calls| n4
n2 -->|tests| n4
n2 -->|calls| n6
n2 -->|tests| n6
n2 -->|calls| n7
n2 -->|tests| n7
n3 -->|calls| n4
n3 -->|tests| n4
n3 -->|calls| n6
n3 -->|tests| n6
n3 -->|calls| n7
n3 -->|tests| n7
n5 -->|uses| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
What changed
Every sequence field an OpenAI-compatible provider can send is now deserialized through
Option<Vec<_>>, so an explicitnullreads as an empty list instead of failing the decode.#[serde(default)]covers an absent key and nothing else. A key present with the valuenullstill reaches theVecvisitor and fails the whole response withinvalid type: null, expected a sequence.That is not hypothetical. Mistral-family endpoints spell "the model did not call a tool" as
"tool_calls": nullon every plain-text completion:{"choices":[{"index":0,"finish_reason":"stop", "message":{"role":"assistant","tool_calls":null,"content":"Hi!"}}]}Any agent role sitting on such a rung could not complete a single turn — the model answered perfectly and the caller saw a transport fault. Found on a Lean-specialised rung (
labs-leanstral-1-5), where a sub-agent delegation failed 100% of the time withserialization error: invalid type: null, expected a sequence.This is the same tolerance
deserialize_argumentsalready applies one level down to a tool call'sarguments, and for the same reason recorded in its doc comment: one provider's unexpected spelling of "nothing" must not fail the decode of everything around it.Fields covered:
choicesandmessage.tool_calls(unary),choicesanddelta.tool_calls(streaming), the Responses API'soutput/content/summary, and the model listing'sdata.Public API or behavior changes
None to the API. One behavior change, deliberate: a body carrying
"choices": nullnow fails withModel("openai response contained no choices")rather than a serde error — the provider fact the caller can act on, rather than the transport being named for a body it read perfectly. Pinned by a test.Validation
cargo test --dochas 5 pre-existing failures insrc/embeddings/— present onmainbefore this branch and untouched by it.Three new tests:
a_null_tool_calls_array_is_read_as_no_tool_calls,a_null_choices_array_is_read_as_no_choices,a_null_tool_calls_delta_is_read_as_no_fragments.Summary by CodeRabbit
Bug Fixes
nullfor optional list fields.Tests