Repository navigation
fix(responses): emit the reasoning item on streaming /v1/responses for signature-only thinking - #42871
fix(responses): emit the reasoning item on streaming /v1/responses for signature-only thinking#42871clonylu wants to merge 1 commit into
Conversation
|
| def _flush_thinking_block() -> None: | ||
| nonlocal current_thinking_text_parts, current_signature | ||
| if len(current_thinking_text_parts) > 0 and current_signature: | ||
| # A signed block with empty thinking text still carries replayable reasoning; keep it. |
There was a problem hiding this comment.
Unnecessary source comments This restates the signature check, violating the repository’s source-comment policy. Remove similar comments in streaming_iterator.py before merging.
Context Used: CLAUDE.md (source)
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!
| @@ -0,0 +1,128 @@ | |||
| """Streaming /v1/responses must surface a reasoning item for a SIGNATURE-ONLY thinking block. | |||
There was a problem hiding this comment.
Incorrect regression test placement This bug fix creates a separate test file, violating the requirement to extend the existing mapped test file before merging.
Context Used: CLAUDE.md (source)
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!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…r signature-only thinking Anthropic models return thinking blocks with empty text and the reasoning carried in the signature: Claude Fable 5.1 and Claude Opus 5.5 by default, and Bedrock adaptive thinking with or without an effort. On streaming /v1/responses the chat->Responses bridge opened a reasoning output item only on reasoning_content text (LiteLLMCompletionStreamingIterator._ensure_output_item_for_chunk), and ChunkProcessor.get_combined_thinking_content kept an assembled thinking block only when it had thinking text. Such a response emitted no reasoning item mid-stream and none in response.completed, so a streaming Responses client could not replay the reasoning even though the reasoning tokens were billed. Non-streaming /v1/responses was unaffected. Open the reasoning item when the delta carries a signed or redacted thinking block, and keep a signed block through stream assembly even when its thinking text is empty. Unsigned text-only fragments are still dropped. The reasoning-text path is unchanged.
d9118d9 to
bc9b6f8
Compare
lets-order-some-fries
left a comment
There was a problem hiding this comment.
I hit the same behaviour independently while looking at #42869 and had a reproduction at main 571ada0b0f; I re-ran it against this PR's head bc9b6f8a5c (2026-09-25).
Before, on 571ada0b0f, a signature-only thinking stream gave:
"signature_only/async": {"output_item.added": ["message"], "completed.output": ["message"], "encrypted_content": null}
At this PR's head, same script, sync and async:
"signature_only/async": {"output_item.added": ["reasoning", "message"], "completed.output": ["reasoning", "message"],
"encrypted_content": "[{\"type\":\"thinking\",\"thinking\":\"\",\"signature\":\"SIG_abc123\"}]"}
It looks like a root-cause fix rather than a patch over the symptom: the new _delta_has_signed_thinking_block (litellm/responses/litellm_completion_transformation/streaming_iterator.py:76-79) widens the predicate at :944, and _flush_thinking_block (litellm/litellm_core_utils/streaming_chunk_builder_utils.py:688) no longer requires non-empty thinking text — both sites the bug needed.
Two questions from things I ran:
-
The latch at
streaming_iterator.py:934means only the first chunk withchoicespicks the first item. For a plain Anthropic stream that is fine — offline,CustomStreamWrapperdrops the emptymessage_start/content_block_startchunks, so the signature chunk really is first. But for[text, signature-only thinking, text, stop]I get announcedoutput_item.added = [("message", 0)]whileresponse.completedoutput is[("reasoning", 0), ("message", 1)]; on main both are[("message", 0)], so the index disagreement is new here. Does interleaved thinking need handling too, or is a client rebuilding the response byoutput_indexout of scope? -
The redacted branch (
b.get("data"),streaming_iterator.py:78) has no test although the body says "signed or redacted". I checked it works — redacted-only at head gives[reasoning, message]with the redacted blob inencrypted_content. Would one more parametrization of the new test be worth it?
Separately, and pre-existing rather than yours: the reasoning-close block exists only in __anext__ (:1019-1064); __next__ (:1084) has no counterpart, so sync signature-only streams get output_item.added(reasoning) with no reasoning_summary_text.done / reasoning_summary_part.done / output_item.done. The new test parametrizes sync_mode but asserts only added_item_types[0] and the completed output, so it passes either way. Same on main for the reasoning_content path.
Tests I ran (own worktree, PYTHONPATH confirmed resolving litellm to the worktree):
- The two touched test files:
93 passed. tests/test_litellm/responses/+ the builder test file: head20 failed, 747 passed, main20 failed, 744 passed, identical failure set (test_responses_websocket_all_providers.pyURL tests) — the +3 are this PR's new tests. Four modules deselected for missingfastapi/mcp/websocketslocally.tests/test_litellm/litellm_core_utils/: head109 failed, 3431 passedvs main40 failed, 3500 passed; all 69 extra aretest_tokenizer.pyModuleNotFoundError: No module named 'litellm.rust_bridge._native', i.e. the compiled extension missing from my worktree, not the PR.
I did not test Bedrock's converse streaming shape, and I could not reproduce the end-to-end proxy claim here (no network), so I checked the equivalent path offline through ModelResponseIterator.chunk_parser.
|
Thanks for the fix. Tests moved to tests/unit on main, so this is rebased and superseded by #43414 with your commit kept. Closing |
TLDR
Problem this solves:
/v1/responseson Claude drops signature-only thinking: no reasoning itemHow it solves it:
User Flow
Before:
/v1/responsesrequest withinclude: ["reasoning.encrypted_content"]to a Claude deployment on Bedrock, or to Claude Fable 5.1 or Opus 5.5 on any providermessageitem andresponse.completedoutput is[message], whilereasoning_tokensis non-zeroAfter:
reasoningthenmessage, andresponse.completedoutput is[reasoning, message]withencrypted_contentsetRelevant issues
Fixes #42869
Related: #41362 keeps the same signed empty thinking block on
/v1/messagesrequestsPre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Tests: one regression test in each mapped file,
test_streaming_iterator_transformation.py(sync and async) andtest_streaming_chunk_builder_utils.py. All three fail on unpatchedmainand pass here. The rest oftests/test_litellm/responses/litellm_completion_transformation/and thetest_streaming_chunk_builder_*files give the same pass/fail set with and without this change.scripts/check_type_discipline.pycounts are unchanged in the two library filesDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup: a
litellm --configproxy run from source, with one deployment pointed at a stand-in upstream that streams an Anthropic message with one thinking block and then text, so the thinking block's shape can be set per caseBefore (6dbd65b)
signature-only thinking block (
thinking: "", signature set)response.output_item.addedevents:messageonlyresponse.completedoutput:[message], and the reasoningencrypted_contentis missingthinking block with text (
thinking: "Two plus two is four.", signature set)response.output_item.addedevents:reasoning,messageresponse.completedoutput:[reasoning, message], and the reasoningencrypted_contentis setAfter (bc9b6f8)
signature-only thinking block (
thinking: "", signature set)response.output_item.addedevents:reasoning,messageresponse.completedoutput:[reasoning, message], and the reasoningencrypted_contentis setthinking block with text (
thinking: "Two plus two is four.", signature set)response.output_item.addedevents:reasoning,messageresponse.completedoutput:[reasoning, message], and the reasoningencrypted_contentis setThe stand-in fixes the block shape per case. With real providers, through a proxy on v1.101.0-rc.1 carrying the equivalent change, Claude Fable 5.1 on Vertex AI went from no reasoning item to one the provider accepted on replay in the next turn. Unpatched, real providers show the bug on Bedrock (Claude Opus 4.8, Fable 5.1, Opus 5.5) with or without a reasoning effort, and on Vertex AI whenever no thinking text comes back. The full matrix is in #42869
Type
🐛 Bug Fix
Final Attestation