fix(models/gemini): emit thought signature as its own reasoning delta#3306
Open
ebarkhordar wants to merge 1 commit into
Open
fix(models/gemini): emit thought signature as its own reasoning delta#3306ebarkhordar wants to merge 1 commit into
ebarkhordar wants to merge 1 commit into
Conversation
GeminiModel packed "text" and "signature" into a single reasoningContent delta. handle_content_block_delta dispatches on key presence, so the "text" arm always won and the signature never reached the aggregated message. The signature is now emitted as its own delta, matching the convention every other provider already follows.
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.
Description
GeminiModel._format_chunkbuilds a singlereasoningContentdelta that carries bothtextandsignaturekeys. The shared aggregator dispatches on key presence, not on value:The
textkey is set unconditionally by the dict literal ingemini.py, so for Gemini theelifarm never runs.
state["signature"]is never set, andhandle_content_block_stopleaves thesignature out of the reasoning block.
The adapter then reads that signature back on the next turn.
_format_request_content_partdoes:which always sees
None, sothought_signature=Noneis always sent back to Gemini. The adapterbase64-encodes the signature on the way in (
gemini.py:401), drops it in aggregation, then looksfor it again on the way out and finds nothing.
Invariant this restores: a thought signature that Gemini issues on a reasoning part survives
stream aggregation, so the signature sent back on the next turn is the one the model issued.
#1703landed exactly this round-trip for thetoolUsepath. This is the same fix for thereasoningTextpath, so signature preservation is consistent within the provider.The fix: emit the signature as its own
contentBlockDelta, which is the convention the otherproviders already follow.
anthropic.py:314emitssignature_deltaas its own single-key delta,and
bedrock.py:1139yields the reasoning text delta and then yields the signature as a separatedelta, which is the same shape this PR gives Gemini. Across every adapter that builds a
reasoningContentdelta (anthropic,bedrock,gemini,llamacpp,openai,openai_responses), Gemini was the only one packing both keys into one delta, which is why theif/elifholds for every other provider.Why not fix the aggregator instead
The one-word change at
streaming.py(eliftoif) also makes the signature survive, and itkeeps the existing tests green. I measured it rather than assume:
handle_content_block_deltareturns a single
typed_event, so the signature branch overwrites it and theReasoningTextStreamEventis lost. Reasoning text would stop reaching streaming consumers.Measured on
8a292d9, same input (a thought part withtext="test reason",thought_signature=b"abc"):ReasoningTextStreamEventmaineliftoifinstreaming.pyI chose the adapter-only seam because it touches no shared aggregation code and so has no
cross-provider blast radius. If you would rather fix the dispatch in
streaming.pyand make itemit multiple events, that is your call and I am happy to redo it that way.
One open question
If Gemini can attach a complete
thought_signatureto more than one part inside a single reasoningblock,
state["signature"] += ...concatenates them. Feeding two signed thought parts through mybranch produces
'czE=czI=', which is two base64 values joined rather than one signature. I couldnot verify whether Gemini actually does that, because I made no live API calls, so I am flagging it
rather than guessing. On
mainthat same input yields no signature at all, so this is not a casethat works today.
Related Issues
None. I found this by reading the adapter against the aggregator, and did not open a separate issue
for it since the repro and the fix are both here. Related merged work: #1703.
Documentation PR
Not needed. No public API or documented behavior changes.
Type of Change
Bug fix
Testing
All offline, no API keys, in a clean
python:3.12-slimDocker container against8a292d9, withimportlib.util.find_spec(...).originchecked on both sides to confirm which copy ran.test_stream_response_reasoning_signature_survives_aggregation, which drives the realmodel.stream()through the realprocess_streamand asserts three things: the aggregatedreasoningTextcarries the exact signature, theReasoningTextStreamEventis still emitted (thisassertion is what fails on the
eliftoifalternative above), and feeding the aggregatedmessage back through
_format_request_content_partreproduces the original signature bytes.gemini.pyhunks and kept thenew test, and it fails on unfixed source. It passes on the branch.
test_stream_response_reasoningandtest_stream_response_reasoning_and_text, whichasserted the combined-delta chunk shape. They passed while the aggregated message was wrong,
which is why this survived.
thought with signature, signed thought followed by plain text, empty-text thought with
signature). Output is identical except the signed-thought path.
hatch test tests: 4421 passed, 5 skipped, 2 failed. Both failures are intests/strands/storage/test_local_file_storage.pyand are not from this change: they failidentically on pristine
8a292d9, because my container runs as root and root ignores thechmodthose two tests use to force a write error.
hatch fmt --linter --check:ruff checkpasses.ruff format --checkreports both changedfiles already formatted.
mypyreports no issues across 223 source files, though I ran it with--follow-imports=silentbecause the default run exhausts memory on my machine, so that leg isnot byte-for-byte what CI runs.
main.What I did not verify: I made no live Gemini API calls, so everything above is about this SDK's own
round-trip, not about how Google's service responds to a request with a missing thought signature. I
also ran the unit suite on Python 3.10 only, not the full 3.10 to 3.14 matrix, so I did not run
hatch run prepareend to end.hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
I used an AI coding assistant on this change. I reproduced the bug, ran every check quoted above
myself, and can explain and defend each line.