Repository navigation
fix(chatcmpl): clear pending thinking blocks when flushing an assistant message - #4089
Merged
Merged
Conversation
…nt message Signed thinking blocks belong to the assistant turn that produced them. A reasoning item that is not directly followed by its assistant message kept them pending across an intervening user turn, so they were prepended to a later assistant message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Converter.items_to_messageskeeps two pieces of cross-item reasoning state.pending_reasoning_contentis cleared on flush, with a comment explaining why (prevents stale reasoning_content from contaminating later turns). Its siblingpending_thinking_blocksis not — it is only cleared where it is consumed, at the assistant-message and function-call branches.So when a reasoning item is not directly followed by its own assistant message, its signed thinking blocks stay pending across an intervening user turn and are prepended to a later assistant message. Anthropic validates
signatureagainst the turn the block belongs to, so the next request either fails validation or replays a previous turn's private reasoning back to the model.This is reachable without anything contrived:
message_to_output_itemsdrops the message item when the content is empty, so a Claude turn with extended thinking but no visible text produces only a reasoning item — the next history entry is the user's next message. It affects the paths that setpreserve_thinking_blocks=Truefor Claude (litellm_model.py,any_llm_model.py).Round-tripping a two-turn history through
message_to_output_items→items_to_messagesonmain:Turn 1's thinking block is attached to turn 2's assistant message. With this change the same history produces
{"role": "assistant", "content": "a2"}.Fix
flush_assistant_messagenow clearspending_thinking_blocksalongsidepending_reasoning_content. The single call site that deliberately preserves reasoning state across a flush — themaybe_response_output_messagebranch, where a reasoning item legitimately belongs to the assistant message that follows it — still passes the opt-out, so that case is unchanged. The keyword is renamed fromclear_pending_reasoning_contenttoclear_pending_reasoningsince it now covers both pieces of state; it is a parameter of a local closure, not public API.Test plan
Added
test_thinking_blocks_do_not_leak_across_an_intervening_user_turnto the existingtests/models/test_anthropic_thinking_blocks.py, asserting the later assistant message carries no thinking block from the earlier turn. It fails onmain(the assistant content is the thinking block plus the text instead of the text) and passes with this change.tests/models/test_anthropic_thinking_blocks.py: 6 passed with the fix; 1 failed / 5 passed without it.tests/models: 605 passed. Converter/litellm/reasoning-related selection: 341 passed.ruff checkandruff format --checkclean on both files;mypyreports no errors in either changed file.Verification-script note:
.agents/skills/code-change-verification/scripts/run.shcould not run here becausemakeis not installed on this machine, so I ran the underlying commands directly. The full parallel suite shows 38 pre-existing failures with the change and 39 on a stashed baseline — all xdist/tracing flakes intesting_processor.py, unrelated to this diff.Issue number
None — self-reported, found while reading the converter.
Checks
.agents/skills/code-change-verification/scripts/run.sh— could not:makeunavailable locally (see Test plan); ran the equivalent commands directly/reviewbefore submitting this PR — n/aAI assistance was used in preparing this change; I reviewed it, reproduced the leak and verified the fix locally.