fix(gemini): strip thought_signature when falling back to Gemma (#36907) - #49218
fix(gemini): strip thought_signature when falling back to Gemma (#36907)#49218hbentel wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the predicate and preserving the copy-on-write contract.
Problems
- Current
maindeliberately restored the opposite predicate atagent/transports/chat_completions.py:128: commit63ddd022a203e48ba0e4617f41c2661f3415e69ecalls the Gemini-only behavior a reverted change requiring its own justification. Commite7648d59129ab1709ed111eca3b1d5f11408adacthen restoredgemma-3-27bkeep-coverage attests/agent/transports/test_chat_completions.py:110so a future narrowing fails loudly. This needs maintainer resolution against the claimed Gemma API behavior. - The shared predicate also controls
AIAgent._sanitize_tool_calls_for_strict_api()(run_agent.py:5585-5588), but this PR tests onlyChatCompletionsTransport.
Suggested changes
- Add a parity regression for the
run_agent.pysanitizer with a Gemma target, including preservation of canonical history. - Retain provider-backed evidence for the asserted Gemma
400 INVALID_ARGUMENTbefore reversing the intentionally restored current-main contract.
Automated hermes-sweeper review.
| """ | ||
| m = str(model or "").lower() | ||
| return "gemini" in m or "gemma" in m | ||
| return "gemini" in m |
There was a problem hiding this comment.
This shared predicate is also used by AIAgent._sanitize_tool_calls_for_strict_api() (run_agent.py:5585-5588), including the direct summary request path. Please add a run-agent parity regression for a Gemma target as well as this transport-level test.
d5115d4 to
4c7aae9
Compare
|
Addressed both review points. Provider-backed justification for reversing the deliberate I've flipped the deliberately-restored Parity coverage for the second consumer of the predicate. The shared Rebased on current |
…Research#36907) A gemini-3-flash thinking-mode tool call carries extra_content.google.thought_signature. When a cross-model fallback (or subagent / mid-session model switch) routes that tool-call history to gemma-4-31b-it, the chat-completions transport left the field in place and Gemma returned HTTP 400 INVALID_ARGUMENT — Gemma does not use the Gemini-3 thinking format. _model_consumes_thought_signature() decided "keep" on "gemini" in m OR "gemma" in m, so any gemma-* model matched and the stale signature leaked. The docstring already said the field should be kept "only when the target model is itself Gemini-family" — Gemma isn't, so the "gemma" term was the bug. Narrow it to "gemini" so Gemma (and any non-Gemini model that inherited stale Gemini extra_content) strips the field. The Gemini native adapter reads extra_content on the response side only, so request-side stripping here does not affect genuine Gemini replay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4c7aae9 to
e0a9698
Compare
|
Rebased onto current |
Problem
A
gemini-3-flashthinking-mode tool call attachesextra_content.google.thought_signature. When a fallback chain (or subagent / mid-session model switch) routes that tool-call history to a Google model that does not use the Gemini-3 thinking format — e.g.gemma-4-31b-it— the request 400s:Repro: primary
gemini-3-flashmakes a tool call (emitsextra_content), then fails retriably so fallback activates togemma-4-31b-it; the first fallback turn 400s. Also reproducible via any cross-provider replay of Gemini-3 tool-call history through a non-Gemini-3 model.Root cause
convert_messages()inagent/transports/chat_completions.pydecides whether to strip the Gemini-specificextra_contentvia_model_consumes_thought_signature(), which returned:So any
gemma-*model matched and the stale thought_signature was kept — but Gemma rejects the unknown field. The function's own docstring says the field should be kept "only when the target model is itself Gemini-family"; Gemma is a separate model family that doesn't use the Gemini-3 thinking format, so the"gemma"term was the bug.Fix
Narrow the predicate to genuine Gemini models:
Now Gemma (and any non-Gemini model that inherited stale Gemini
extra_contentearlier in a mixed-provider session) strips the field, while real Gemini-3 targets still replay it. The Gemini native adapter readsextra_contenton the response side only, so request-side stripping here does not affect legitimate Gemini replay.Test plan
test_convert_messages_strips_extra_content_for_gemma—gemma-4-31b-itandgoogle/gemma-3-27bstrip the signature (original message list untouched).test_convert_messages_keeps_extra_content_for_geminito assert keep forgemini-3-pro/google/gemini-3-pro-preview/gemini-3-flash(no longer asserts the buggy gemma-keeps behavior).scripts/run_tests.sh tests/agent/transports/test_chat_completions.py— 82/82 pass.Closes #36907
🤖 Generated with Claude Code