fix: content_block_delta type mismatch when reasoning and content arrive in one chunk - #33938
Conversation
…out (BerriAI#31632) * fix(prometheus): bound per-request budget metric emission with a timeout Wrap the per-request budget-metric gather in asyncio.wait_for so a slow Redis or DB lookup cannot consume the whole LoggingWorker watchdog and get the success-logging event cancelled. On timeout the emission is skipped in isolation; budget gauges are still refreshed by the periodic cron. The timeout is configurable via PROMETHEUS_BUDGET_METRICS_PER_REQUEST_TIMEOUT and defaults to 5.0 seconds, falling back to the default on an invalid value instead of raising * fix(prometheus): reject non-finite and non-positive budget-metrics timeout env float() accepts 0, negatives, nan and inf, which bypass the fallback: a value <= 0 makes asyncio.wait_for time out immediately and skip every per-request emission, and inf reintroduces the unbounded wait the timeout was meant to bound. Validate the parsed value is finite and greater than zero before using it, otherwise fall back to the default
When a guardrail blocks a post-call response, the synthetic violation response reported hard-coded zero usage, discarding the token usage the upstream call had already consumed. Fix the root cause rather than re-counting tokens: - Add an optional `original_response` field to ModifyResponseException. - The unified guardrail's post-call success hook attaches the blocked LLM response to the exception. - The /v1/messages and OpenAI-format (/v1/chat/completions, /v1/completions) block handlers report `original_response.usage` directly. Pre-call blocks never invoked the LLM, so usage is zero. Mock-based tests cover the helper (returns original usage / zero), the success hook attaching original_response, and the endpoint reporting it end-to-end. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ock (BerriAI#31389) Streaming moderation improvements for the unified guardrail post-call streaming iterator hook: - streaming_buffer_until_moderated: withhold all chunks until end-of-stream moderation passes, then release the original response (clean) or only the block message (blocked) -- the original content is never delivered on a block. Snapshot chunks with a shallow list() copy (end-of-stream builds a separate assembled response; chunks aren't mutated in place). - Clean Anthropic SSE on block: synthesize a well-formed termination sequence instead of a bare data: {"error": ...} blob that truncates the stream. Provider-specific synthesis lives in AnthropicMessagesHandler via build_block_sse_chunks (format-agnostic routing stays in the hook). - Mid-stream blocks continue the in-progress message (close open content block, append block message, terminate) rather than emitting a second message_start, which clients reject. Standalone envelope only when no chunks were sent (buffered path). - ModifyResponseException imported under TYPE_CHECKING + locally at runtime to avoid a module-level cyclic import. Adds regression tests for buffering (content withheld on block) and mid-stream continuation (single message_start). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… content-rewriting guardrails - _standalone_block_chunks and _block_continuation_chunks now read real token usage from ModifyResponseException.original_response instead of hardcoding zero, matching the non-streaming _blocked_response_usage path. Shared helper moved to guardrail_translation/utils.py. - streaming_buffer_until_moderated is now forced off when the guardrail has mask_response_content=True, since buffered replay releases the withheld original chunks verbatim -- unsafe for a guardrail that rewrites content (e.g. PII masking). - Fix inverted streaming-flag precedence comment.
…-of-stream detection _check_streaming_has_ended assumed responses_so_far held ModelResponse objects with .choices, but for the Responses API the accumulated chunks are raw SSE event dicts, causing an AttributeError on every call
…emitting a broken stream When the initial LLM call inside MCPEnhancedStreamingIterator fails (e.g. an invalid previous_response_id -> provider 400 'No tool output found for function call ...'), the proxy returned HTTP 200 and the stream emitted the pre-generated mcp_list_tools discovery events with no response.created before them. That violates the Responses API streaming contract and crashes SDK stream accumulators (openai-node: "expected 'response.created' event, got response.mcp_list_tools.in_progress"). - aresponses_api_with_mcp now makes the initial call eagerly, before any SSE bytes are written, and re-raises the stashed failure so the client gets a real 4xx/5xx with the provider error body. - If a creation failure still surfaces during iteration, the stream emits a single terminal 'error' event instead of discovery events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…low-up failures When tool execution failed as a batch, the stream proceeded to a follow-up call carrying function_call items with no outputs — rejected by the provider with 'No tool output found for function call ...' — and when the follow-up call itself failed, the stream simply ended with no terminal event. In both cases the client received HTTP 200 and a stream that looks like a truncated success: tool events, then silence. - Stash tool-execution and follow-up failures on the iterator. - Skip the doomed follow-up call entirely after a tool-execution failure. - Emit a single terminal OpenAI-style 'error' stream event carrying the mapped failure instead of ending silently. Builds on the initial-call failure handling from the previous commit (shares the _stream_error stash and _make_stream_error_event helper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A request that explicitly asks for MCP tools via server_url litellm_proxy/... but resolves none of them (the API key/team has no access to the MCP server via allow_all_keys=false and no object-permission grant, the server name does not exist, or allowed_tools matches nothing) was silently sent to the model with no tools. The model then hallucinates, and the only trace is a list_mcp_tools spend log with status success and an empty response — the request looks healthy end to end while being completely broken. Raise a 400 BadRequestError naming the requested server URLs and the likely causes instead. Guard scope: - Mixed requests are exempt: with other (function) tools present, the request proceeds using those tools, matching the previous fallback behaviour. - Opt-out via litellm.reject_empty_mcp_resolved_tools = False (default True, per maintainer guidance). The auth-header pass-through test in tests/mcp_tests now resolves a dummy tool, since its purpose is header propagation, not zero-tool behaviour. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rop orphaned tool events on batch failure Review feedback (Greptile on BerriAI#32579): - The terminal error event was numbered sequence_number=1, out of order after tool-execution events. __anext__ now tracks the highest sequence_number that passed through the stream and the error event is numbered after it. - A batch tool-execution failure queued mcp_call.in_progress events that never received a terminal per-item event. Those queued events are now dropped; the terminal error event carries the failure instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Drop test_initial_call_success_does_not_emit_error_event: the tool-call happy path (test_tool_call_happy_path_emits_no_error_event) already guards against false-positive error events and exercises more of the changed code (tool-exec + follow-up success paths). - Drop the stream=True parametrization on the zero-resolved-tools guard: the guard runs before the stream/non-stream branch in aresponses_api_with_mcp, so both cases hit identical code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removed MCP server configuration for deepwiki.
…cp_gateway_failure_handling fix(responses): fail loudly on MCP gateway failures (initial call, mid-stream, zero resolved tools)
…ive in one chunk Some vLLM reasoning parsers flush the tail of reasoning_content bundled with the first token of content in the same streaming chunk at the reasoning/answer boundary. AnthropicStreamWrapper's content-block-type detection (_should_start_new_content_block) and its delta emission (translate_streaming_openai_response_to_anthropic) disagreed on which payload is dominant in that case: the former picks content (text), the latter picks reasoning (thinking_delta). The result is a thinking_delta emitted into a block the client was already told is text, which strict Anthropic SSE clients (e.g. Claude Code) reject as "Content block is not a thinking block" - and since the corrupted turn gets replayed as conversation history, every subsequent turn fails the same way until the context is cleared. Generalize the existing _CombinedChunkSplitter (previously only handling content+finish_reason combined chunks, e.g. fake-streamed providers) to also split a combined reasoning+content chunk into two single-payload chunks, reasoning first.
Greptile SummaryThis PR fixes a
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the splitter that pre-processes chunks before the stream wrapper, existing fake-stream behavior is preserved, and the new regression test drives the exact failure scenario end-to-end. The fix is well-contained: No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py | Generalized _CombinedChunkSplitter._split to produce one chunk per payload (reasoning, content/tool_calls, finish_reason) instead of always two; helper statics extracted for reuse; logic is consistent between _is_combined (detection) and _split (production) |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_streaming_iterator_combined_chunk.py | Existing test updated to assert the corrected three-way split; new assertions are strictly stronger than the old ones and all prior passing test cases remain unchanged |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_streaming_iterator_reasoning_content_chunk.py | New regression test replays the captured vLLM boundary-chunk shape through the full AnthropicStreamWrapper, asserts zero delta/block-type mismatches, and verifies both payloads survive as separate deltas; no real network calls |
Reviews (1): Last reviewed commit: "fix: content_block_delta type mismatch w..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a
Confidence Score: 4/5Safe to merge; the change is tightly scoped to the splitter pre-processing layer and does not touch the main translation or state-machine logic in AnthropicStreamWrapper. The logic change is small and well-isolated: only _CombinedChunkSplitter._is_combined and _split are rewritten, and the three-way split correctly mirrors what the downstream pair already expects. Tests cover both split mechanics and end-to-end SSE ordering. The two findings are a stale inline comment and a bare dict subscript in the test helper that produces an unhelpful KeyError for unknown delta types — neither affects correctness. No files require special attention; the existing-test update correctly strengthens rather than weakens coverage.
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py | Generalized _CombinedChunkSplitter._split to handle reasoning+content co-arrival at the vLLM boundary, correctly producing 3 single-payload chunks instead of 2; minor stale inline comment in __init__ |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_streaming_iterator_combined_chunk.py | Existing combined-chunk test updated to expect correct 3-way split for reasoning+content+finish chunks; new assertions are more granular and accurate than before |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_streaming_iterator_reasoning_content_chunk.py | New regression test exercising the full AnthropicStreamWrapper with a boundary chunk that bundles reasoning tail + first answer token; no real network calls |
Comments Outside Diff (1)
-
litellm/llms/anthropic/experimental_pass_through/adapters/streaming_iterator.py, line 208-211 (link)The
__init__comment still describes the old (narrower) behavior of_CombinedChunkSplitter— splitting only content+finish_reason pairs into two chunks. It no longer reflects the generalized logic that handles reasoning+content co-arrival and can produce up to three chunks.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "fix: content_block_delta type mismatch w..." | Re-trigger Greptile
| expected_block_type = { | ||
| "text_delta": "text", | ||
| "thinking_delta": "thinking", | ||
| "signature_delta": "thinking", | ||
| "input_json_delta": "tool_use", | ||
| }[delta_type] |
There was a problem hiding this comment.
The helper uses a bare dict subscript for
delta_type lookup, so any delta type not in the four-entry map (e.g. compaction_delta) raises an unhelpful KeyError rather than a descriptive assertion failure. Using .get() with a fallback and an explicit assert makes failures much easier to diagnose if a new event type slips into this test's stream.
| expected_block_type = { | |
| "text_delta": "text", | |
| "thinking_delta": "thinking", | |
| "signature_delta": "thinking", | |
| "input_json_delta": "tool_use", | |
| }[delta_type] | |
| expected_block_type = { | |
| "text_delta": "text", | |
| "thinking_delta": "thinking", | |
| "signature_delta": "thinking", | |
| "input_json_delta": "tool_use", | |
| }.get(delta_type) | |
| assert expected_block_type is not None, ( | |
| f"Unknown delta type {delta_type!r}; extend the map above if intentional" | |
| ) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Closing: litellm_internal_staging's _CombinedChunkSplitter already generalizes to reasoning+content chunks, so this specific fix is now redundant. |
Relevant issues
Linear ticket
Pre-Submission checklist
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Proxy config: self-hosted vLLM backend with a reasoning-capable Qwen model behind an
openai/deployment, Claude Code pointed at the proxy's/v1/messagesendpointCaptured live from the failing deployment before the fix. The vLLM reasoning parser flushed the tail of
reasoning_contentbundled with the first character of the answer in one streaming chunk.AnthropicStreamWrapperopened the new content block astext(block-type detection prefers content) but emitted athinking_deltainto it (delta emission prefers reasoning), producing this SSE sequence:Claude Code (a strict Anthropic SSE client) rejects this as "Content block is not a thinking block". Because the corrupted assistant turn gets replayed as conversation history, every subsequent turn in the same context fails the same way until the context is cleared.
After the fix, hitting the same backend with a prompt that forces a reasoning/answer transition, and reducing the captured event stream to (index -> declared block type) then checking every delta against its block's declared type:
block types opened:
{0: "text", 1: "thinking", 2: "text"}(empty leading text block, then thinking, then the answer), zero delta/block-type mismatches across all 462 deltasType
🐛 Bug Fix
Changes
_CombinedChunkSplitterpreviously only split a chunk that combined response content with afinish_reason(needed for fake-streamed providers like Vertex AI Gemma:predict, which collapse the whole response into one chunk). It did not split a chunk that combinedreasoning_content/thinking_blockswithcontentin the same delta, which some vLLM reasoning parsers do at the reasoning/answer boundary.That combined shape hit two functions that disagree on which payload is dominant:
_should_start_new_content_block(content-block-type detection) checkscontentbeforereasoning_contentand pickstext, whiletranslate_streaming_openai_response_to_anthropic(delta emission) checksreasoning_contentfirst and picksthinking_delta. The mismatch means athinking_deltalands on a block the client was told istext.Generalized
_CombinedChunkSplitterto split on any combination of {reasoning, content/tool_calls, finish_reason} rather than only {content, finish_reason}, so each downstream chunk carries exactly one payload and both functions agree on its type. Existing content+finish_reason behavior is unchanged; a chunk carrying reasoning+content+finish_reason now splits into three single-payload chunks instead of two.Added a regression test (
test_streaming_iterator_reasoning_content_chunk.py) that replays the captured chunk shape (reasoning tail bundled with content) through the fullAnthropicStreamWrapperand asserts no delta ever targets a block of the wrong declared type, and that both payloads survive as separate deltas. Updated the existing combined-chunk splitter test that asserted the old (incorrect) two-way split behavior for a reasoning+content+finish chunk to assert the corrected three-way split.