Skip to content

fix(streaming): preserve unsigned thinking blocks in stream_chunk_builder - #33035

Open
abhay-codes07 wants to merge 1 commit into
BerriAI:litellm_oss_daily_2026_07_10from
abhay-codes07:fix/streaming-thinking-blocks-no-signature
Open

fix(streaming): preserve unsigned thinking blocks in stream_chunk_builder#33035
abhay-codes07 wants to merge 1 commit into
BerriAI:litellm_oss_daily_2026_07_10from
abhay-codes07:fix/streaming-thinking-blocks-no-signature

Conversation

@abhay-codes07

Copy link
Copy Markdown

Relevant issues

Fixes #33034

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes lint / format / unit tests locally
  • My PR's scope is as isolated as possible; it only solves 1 specific problem

Screenshots / Proof of Fix

Reasoning-content data loss during streamed-response assembly, reproducible offline (no keys/network).

Before (base 3d63eda) — unsigned thinking text is dropped:

thinking_blocks (unsigned stream): None

After (this PR) — text preserved; signed/redacted paths unchanged:

UNSIGNED: [{'type': 'thinking', 'thinking': 'Let me reason about this.'}]
SIGNED  : [{'type': 'thinking', 'thinking': 'reasoning', 'signature': 'sig123'}]
REDACTED: [{'type': 'redacted_thinking', 'data': 'abc'}]

New regression test (fails on base, passes here) + existing thinking tests still green:

tests/test_litellm/litellm_core_utils/test_streaming_chunk_builder_utils.py  17 passed
  (incl. test_get_combined_thinking_content_preserves_unsigned_block)

Type

🐛 Bug Fix

Changes

ChunkProcessor.get_combined_thinking_content required both accumulated text and a signature to emit a thinking block, so a block whose signature never arrived (truncated stream, or a provider without per-block signatures) lost all its reasoning text. Emit the block whenever text exists and attach the signature only when present. signature is Optional on ChatCompletionThinkingBlock, and the send-side prompt transforms already drop unsignable blocks before replaying them, so this does not risk invalid replay. Signed and redacted paths are unchanged.


cc @ishaan-jaff @krrish-berri-2 — isolated data-loss fix in stream assembly with a regression test. Thanks!

…lder

ChunkProcessor.get_combined_thinking_content only emitted a thinking block when both accumulated text AND a signature were present. The per-block signature arrives only at the end of a thinking block, so any block whose signature never lands (a stream truncated mid-thinking by max_tokens or cancellation, or a provider that emits thinking text without per-block signatures) had all of its accumulated reasoning text silently discarded, and thinking_blocks came back as None.

signature is optional on ChatCompletionThinkingBlock, and the Anthropic streaming handler itself emits thinking blocks with text and no signature, so the assembled response must keep the text. Emit the block whenever text exists and attach the signature only when present. The send-side prompt transforms already drop unsignable blocks before replaying them, so this does not risk invalid replay. Signed and redacted paths are unchanged; adds a regression test.
Copilot AI review requested due to automatic review settings July 13, 2026 04:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a silent data-loss bug in ChunkProcessor.get_combined_thinking_content where accumulated thinking text was dropped whenever no signature arrived — as happens with a truncated stream or a provider that emits thinking without per-block signatures. The condition guarding _flush_thinking_block previously required both text and a signature; the fix relaxes that to text-only and attaches the signature only when present, matching the Optional[str] typing on ChatCompletionThinkingBlock.

  • Core fix (streaming_chunk_builder_utils.py): _flush_thinking_block now emits a block as long as there is accumulated text; signature is added conditionally. Signed and redacted paths are unchanged.
  • Regression test (test_streaming_chunk_builder_utils.py): A new test verifies that unsigned thinking text is preserved after assembly, failing on the base commit and passing here. Existing signed/interleaved/redacted tests continue to pass.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-scoped, and strictly additive: callers that previously received unsigned thinking blocks as None will now receive the accumulated text instead, with no impact on signed or redacted paths.

The one-line condition change in _flush_thinking_block is straightforward, the TypedDict already declares signature as Optional[str], and the production caller in main.py uses the same chunk-filtering pattern that the new test exercises. No existing test expectations were weakened.

No files require special attention.

Important Files Changed

Filename Overview
litellm/litellm_core_utils/streaming_chunk_builder_utils.py Fixes silent data loss in _flush_thinking_block by emitting unsigned thinking blocks; signature is attached only when present, matching the Optional[str] typing on ChatCompletionThinkingBlock.
tests/test_litellm/litellm_core_utils/test_streaming_chunk_builder_utils.py Adds a focused regression test for the unsigned-thinking-block bug; mirrors the same chunk-filtering pattern used by the production caller in main.py and by the existing interleaved-block test.

Reviews (1): Last reviewed commit: "fix(streaming): preserve unsigned thinki..." | Re-trigger Greptile

@abhay-codes07

Copy link
Copy Markdown
Author

Heads up: the red auth-and-jwt check here is a pre-existing failure on the litellm_oss_daily_2026_07_10 base (the test_can_key_call_model_wildcard_access/test_can_team_access_model bedrockz/* cases), unrelated to this change. Root-caused and fixed separately in #33031; this PR's own changes are fully covered and green. Once #33031 lands in the base, this check goes green on re-run.

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants