fix(anthropic): split mixed stream chunks by payload kind - #35289
Conversation
Greptile SummarySplits mixed Anthropic stream chunks into ordered, single-payload pieces.
Confidence Score: 5/5This follow-up appears safe to merge. No blocking failure remains within the eligible follow-up-review scope.
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py | Adds payload-kind splitting and reasoning normalization while preserving terminal usage and selected passthrough behavior. |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_streaming_iterator_first_delta.py | Adds focused sync and async regressions for mixed reasoning, text, tool-use, continuation, multi-choice, and usage behavior. |
Reviews (3): Last reviewed commit: "fix(anthropic): keep continuation and mu..." | Re-trigger Greptile
| pieces = tuple(copy.deepcopy(chunk) for _ in present_groups) | ||
| for index, (piece, group) in enumerate(zip(pieces, present_groups)): | ||
| copied_delta = piece.choices[0].delta | ||
| fields = {field: value for field in group if (value := getattr(copied_delta, field, None))} | ||
| role = getattr(copied_delta, "role", None) if index == 0 else None | ||
| piece.choices[0].delta = Delta(role=role, **fields) | ||
| for extra_choice in piece.choices[1:]: | ||
| extra_choice.delta = Delta() | ||
| return pieces |
There was a problem hiding this comment.
🟡 Several tool calls arriving in one collapsed chunk are merged into a single broken tool call
All tool calls riding on a collapsed chunk are put on one split piece (("tool_calls",) group at litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py:102) instead of one piece per call, so a response with two or more tool calls reaches the client as a single tool call whose arguments are two JSON objects glued together.
Impact: When a provider returns the whole answer in one chunk with multiple tool calls, the client sees only the first tool's name/id and unparseable arguments, so those tool calls fail.
Mechanism: single tool_use block built from tool_calls[0] with all arguments concatenated
Fake-streamed providers collapse an entire non-streaming response into one chunk (MockResponseIterator at litellm/llms/base_llm/base_model_iterator.py:204-221), so delta.tool_calls can hold several parallel calls. _split_by_payload_kind groups them all onto one piece, and the translator then derives the block header from choices[0].delta.tool_calls[0] only (litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py:1385-1405) while concatenating every tool's function.arguments into a single partial_json (litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py:1450-1456). Result: one content_block_start of type tool_use with the first tool's id/name, followed by an input_json_delta like {"a":1}{"b":2}. The per-tool-call block splitting in _should_start_new_content_block only fires across chunks, never within one. Emitting one piece per tool call (mirroring the existing per-kind loop) would make each tool_use block well-formed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified pre-existing rather than introduced here. Running the exact shape (one collapsed chunk, two named tool_calls) through the wrapper at the merge-base and at this head gives byte-identical output: one tool_use block named from tool_calls[0] with the arguments concatenated ('{"city":"NY"}{"tz":"EST"}') on both sides. The gluing happens in the translator (block header from tool_calls[0], every call's arguments accumulated into one partial_json), which pure tool chunks hit without ever entering the new split, so splitting per call here would not fix it either. The right fix is per-call block emission in the wrapper or translator, tracked separately rather than folded into this PR
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c4b2cc1. Configure here.
The mixed-chunk split cleared only the fields it knew about on each deep-copied piece, so any other payload riding the chunk survived on both pieces: tool_calls were emitted as two tool_use blocks with the same id, thinking_blocks on the text piece emitted duplicated thinking into a text block while dropping the answer text, and chunks whose reasoning arrived only as thinking_blocks never split at all Rebuild each piece's delta from scratch with exactly one payload kind (reasoning, text, tool calls), ordered to match native Anthropic block order. Fresh Delta construction keeps unset attributes deleted, which matters because the translators branch on hasattr, and prevents future Delta fields from riding along on every piece
…it signature-less thinking once Adversarial verification against the merge-base found three shapes where the payload-kind split changed behavior beyond its target: a mixed chunk carrying a tool argument continuation was torn into a truncated block plus a fabricated one, a multi-choice chunk lost its secondary choices' payload, and a signature-less thinking_blocks piece inherited the non-empty block start body so accumulators collected the thinking twice Continuation and multi-choice chunks now pass through the splitter untouched, matching the merge-base byte for byte, and signature-less thinking_blocks pieces are normalized to reasoning_content so the block start opens empty and the thinking text is emitted exactly once
c4b2cc1 to
bc12ede
Compare
|
@greptileai please review the current head bc12ede |
| return pieces | ||
|
|
||
| @staticmethod | ||
| def _normalize_reasoning_fields(fields: "dict[str, Any]") -> "dict[str, Any]": |
There was a problem hiding this comment.
🟡 New helper uses a banned loose dictionary type instead of a precise type
The new reasoning-normalization helper is typed with the coarse dict[str, Any] signature (_normalize_reasoning_fields at litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py:160), which the repository's coding guidelines explicitly forbid.
Impact: The change ships with weaker type information than the project requires, so mistakes in the data passed through it are not caught automatically.
Rule violated and where the loose type flows
CLAUDE.md (referenced as mandatory by AGENTS.md) states: "Fully typed; no Any or coarse types like dict[str, Any] or just dict. Every function parameter must be strongly typed". _normalize_reasoning_fields both accepts and returns dict[str, Any], and its result is splatted into Delta(role=role, **fields) at litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py:156, so no type checker can verify that the keys/values match Delta's reasoning/text/tool parameters. A TypedDict (e.g. with optional reasoning_content, thinking_blocks, content, tool_calls keys) or returning a small frozen dataclass consumed explicitly would satisfy the rule.
Prompt for agents
CLAUDE.md forbids coarse types such as dict[str, Any]. `_CombinedChunkSplitter._normalize_reasoning_fields` in litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py is annotated `(fields: dict[str, Any]) -> dict[str, Any]`, and its return value is splatted into `Delta(role=role, **fields)`, so the payload shape is completely untyped at the boundary. Consider modelling the per-piece payload explicitly (e.g. a TypedDict with the optional keys reasoning_content / thinking_blocks / content / tool_calls, or a frozen dataclass that the caller turns into explicit Delta keyword arguments) so the splat is type-checked.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correction: the PR merged at bc12ede before this cleanup landed, and the follow-up was dropped as style-level; the merged code keeps the dict passing helper. Verified behavior-neutral either way over a 720 shape differential
| fields = {field: value for field in group if (value := getattr(copied_delta, field, None))} | ||
| fields = _CombinedChunkSplitter._normalize_reasoning_fields(fields) |
There was a problem hiding this comment.
🟡 Split logic rebinds a local variable, against the project's no-mutation convention
The per-piece payload value is built and then immediately overwritten by reassigning the same local name (fields = _CombinedChunkSplitter._normalize_reasoning_fields(fields) at litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py:154), which the repository's coding guidelines forbid.
Impact: The new code deviates from the required style for this repository, making it harder to review and maintain.
Rule text and a conforming shape
CLAUDE.md (mandatory via AGENTS.md) lists under coding conventions: "No mutation; don't reassign variables, global or local." Line 153 builds fields from the copied delta and line 154 rebinds the same name to the normalized result. Composing in one expression, e.g. normalized_fields = _CombinedChunkSplitter._normalize_reasoning_fields({field: value for field in group if (value := getattr(copied_delta, field, None))}), keeps a single binding.
| fields = {field: value for field in group if (value := getattr(copied_delta, field, None))} | |
| fields = _CombinedChunkSplitter._normalize_reasoning_fields(fields) | |
| normalized_fields = _CombinedChunkSplitter._normalize_reasoning_fields( | |
| {field: value for field in group if (value := getattr(copied_delta, field, None))} | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correction: the PR merged at bc12ede before this cleanup landed, and the follow-up was dropped as style-level; the rebinding remains in the merged code. Verified behavior-neutral either way over a 720 shape differential
|
@greptileai please review the current head 1e2c9c2 |
TLDR
Problem this solves:
How it solves it:
Relevant issues
Fixes #33224. Adopts and supersedes #34701 by @Napuh, whose two commits are preserved with his authorship; his split of reasoning and text is kept and extended to cover tool calls and thinking_blocks
Linear ticket
Resolves LIT-5019
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Live proxy legs, real Gemini behind the adapter, no mocks. Proxy launched from this branch (worktree import asserted), captured at the tree-identical pre-rebase commit f004001149; the rebase onto current staging changed no file in this PR
The forced tool call leg returns tool_use at index 0 with a single input_json_delta, the plain text leg returns text at index 0, the non-streaming leg returns 200, and all four write correct spend rows to Postgres (0.003516 / 0.000203 / 0.000630 / 0.000312 USD)
Real providers do not emit the collapsed mixed shape on demand (it arises from fake-streamed responses and vLLM-style parser boundary chunks), so the defect differential drives the real
AnthropicStreamWrapperin process with the collapsed chunks from the bug reports. Before = dbdfe4a (adopted #34701 unchanged), after = c4b2cc1:The reasoning-plus-text shapes from #34701 itself are byte-identical before and after, and @Napuh verified that base split against a live GLM-5.2 vLLM backend in #34701 (comment)
A second adversarial verification pass (lensed investigators plus reproduce-or-reject verifiers against the genuine merge-base module, real provider chunk shapes, and a real anthropic SDK accumulator) found three shapes where the first iteration of the split diverged beyond its target; commit bc12ede closes them and adds a pinning test per shape (each fails at d6b3608, passes at bc12ede). A mixed chunk carrying a tool argument continuation (function.name None) now passes through unsplit, so the in-flight tool_use block keeps assembling valid JSON exactly as on the merge-base. A multi-choice chunk passes through unsplit, so secondary choices' payload is neither dropped nor repeated. A signature-less thinking_blocks piece is normalized to reasoning_content, so the synthesized block start opens empty and SSE accumulators collect the thinking exactly once instead of twice via the seeded start body
Type
🐛 Bug Fix
Changes
_split_mixed_reasoning_and_textdeep-copied the mixed chunk and cleared only the one field it knew about on each piece, so any other payload riding the chunk survived on both pieces:tool_callsproduced two tool_use blocks with the same id,thinking_blockson the text piece emitted duplicated thinking into a text block while dropping the answer text, and chunks whose reasoning arrived only asthinking_blocksnever matched the predicate and never splitThe replacement
_split_by_payload_kindbuilds one piece per payload kind present (reasoning =reasoning_contentandthinking_blockstogether, text =content, tools =tool_calls), ordered thinking then text then tool_use to match native Anthropic block order. Each piece's delta is a freshly constructedDeltacarrying only its own kind, mirroring_clear_later_replay_slice_metadata:Deltadeletes unset reasoning attributes and the translators branch onhasattr, so clearing fields on a copy would resurrect them as attribute-presentNones, and a futureDeltafield could otherwise ride along on every piece. Secondary choices on split pieces get empty deltas so multi-choice chunks cannot leak payload either.reasoning_contentandthinking_blocksstay on one piece so a signature-carrying thinking chunk keeps its signature suppression, and_splitstill runs first, peelingfinish_reasonand usage onto the finish chunkRegression tests cover the three defect shapes (tool call emitted once with full ordering asserted, sync and async; thinking_blocks-only mixed chunk splits; both reasoning fields keep the text) plus usage emitted exactly once on a mixed finish chunk. All fail on the adopted #34701 code and pass with the fix
Behavior changes
Mixed chunks now emit one block per payload kind in thinking, text, tool_use order; previously the tool_use block was duplicated and thinking or text could be mislabeled or dropped. The split deliberately skips tool argument continuations and multi-choice chunks, which behave byte-identically to the merge-base. Signature-less thinking_blocks payloads on split pieces are emitted through the reasoning_content branch, so their thinking block opens with an empty start body instead of a pre-seeded one. Delta fields that neither streaming translator reads (
annotations,reasoning_items, delta-levelprovider_specific_fields) no longer ride along on split pieces; this is unobservable in emitted events today and prevents the next such field from reintroducing the duplication classFinal Attestation