fix: thought signature handling in gemini chat completion tool call #1181
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe pull request refactors the storage and retrieval of thought signatures in the Gemini provider. Instead of storing signatures in ExtraContent, they are now persisted in ReasoningDetails with encrypted values and optional identifiers. This change affects both the composition of reasoning details during chat processing and the extraction of signatures from received data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (3)📚 Learning: 2025-12-09T17:07:42.007ZApplied to files:
📚 Learning: 2025-12-17T08:44:08.788ZApplied to files:
📚 Learning: 2025-12-19T09:26:54.961ZApplied to files:
🧬 Code graph analysis (3)core/schemas/chatcompletions.go (2)
core/providers/gemini/utils.go (3)
core/providers/gemini/chat.go (3)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/gemini/chat.go (1)
287-295: Bug: Streaming path missing ID assignment for tool call signatures.The non-streaming path (lines 145-151) sets
reasoningDetail.IDwhenpart.FunctionCallis present, but the streaming path does not. This will cause the thought signature lookup inutils.go(line 613) to fail for streaming responses because it searches fortool_call_<callID>.🔎 Proposed fix
// Handle thought signature separately (not part of the switch since it can co-exist with other types) if part.ThoughtSignature != nil { thoughtSig := base64.StdEncoding.EncodeToString(part.ThoughtSignature) - reasoningDetails = append(reasoningDetails, schemas.ChatReasoningDetails{ + reasoningDetail := schemas.ChatReasoningDetails{ Index: len(reasoningDetails), Type: schemas.BifrostReasoningDetailsTypeEncrypted, Signature: &thoughtSig, - }) + } + + // Set ID if this is a tool call signature + if part.FunctionCall != nil { + callID := part.FunctionCall.Name + if part.FunctionCall.ID != "" { + callID = part.FunctionCall.ID + } + reasoningDetail.ID = schemas.Ptr(fmt.Sprintf("tool_call_%s", callID)) + } + + reasoningDetails = append(reasoningDetails, reasoningDetail) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/providers/gemini/chat.gocore/providers/gemini/utils.gocore/schemas/chatcompletions.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
core/schemas/chatcompletions.gocore/providers/gemini/utils.gocore/providers/gemini/chat.go
🧠 Learnings (3)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/chatcompletions.gocore/providers/gemini/utils.gocore/providers/gemini/chat.go
📚 Learning: 2025-12-17T08:44:08.788Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1114
File: core/schemas/chatcompletions.go:224-228
Timestamp: 2025-12-17T08:44:08.788Z
Learning: In core/schemas/chatcompletions.go, ensure the schema structures mirror OpenAI's API specifications exactly. Use the valid values for fields like ChatAudioParameters.Format and ChatAudioParameters.Voice as defined by OpenAI's documentation, and avoid adding additional inline documentation or constants to maintain direct compatibility with OpenAI's API.
Applied to files:
core/schemas/chatcompletions.go
📚 Learning: 2025-12-19T09:26:54.961Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/utils/utils.go:1050-1051
Timestamp: 2025-12-19T09:26:54.961Z
Learning: Update streaming end-marker handling so HuggingFace is treated as a non-[DONE] provider for backends that do not emit a DONE marker (e.g., meta llama on novita). In core/providers/utils/utils.go, adjust ProviderSendsDoneMarker() (or related logic) to detect providers that may not emit DONE and avoid relying on DONE as the sole end signal. Add tests to cover both DONE-emitting and non-DONE backends, with clear documentation in code comments explaining the rationale and any fallback behavior.
Applied to files:
core/providers/gemini/utils.gocore/providers/gemini/chat.go
🧬 Code graph analysis (3)
core/schemas/chatcompletions.go (2)
core/providers/gemini/types.go (1)
Type(783-783)ui/lib/types/logs.ts (1)
Function(152-157)
core/providers/gemini/utils.go (3)
core/schemas/chatcompletions.go (2)
ChatAssistantMessage(658-665)BifrostReasoningDetailsTypeEncrypted(758-758)ui/lib/types/logs.ts (1)
ReasoningDetails(127-134)core/providers/gemini/types.go (1)
Type(783-783)
core/providers/gemini/chat.go (3)
core/schemas/chatcompletions.go (2)
ChatReasoningDetails(763-771)BifrostReasoningDetailsTypeEncrypted(758-758)core/providers/gemini/types.go (2)
Type(783-783)FunctionCall(1202-1212)core/schemas/utils.go (1)
Ptr(16-18)
🔇 Additional comments (4)
core/schemas/chatcompletions.go (2)
719-724: LGTM!The removal of
ExtraContentfromChatAssistantMessageToolCallaligns with the refactor to store thought signatures inReasoningDetailsinstead. The remaining fields are appropriate for tool call representation.
764-764: LGTM!The new optional
IDfield onChatReasoningDetailsenables linking reasoning details to specific tool calls via thetool_call_<callID>format. This is a clean approach for correlating encrypted signatures with their corresponding function calls.core/providers/gemini/utils.go (1)
611-626: LGTM!The logic correctly retrieves thought signatures from
ReasoningDetailsusing thetool_call_<callID>lookup pattern. The implementation properly validates the ID match, type, and signature presence before decoding. The earlybreakafter finding the match is an appropriate optimization.core/providers/gemini/chat.go (1)
136-154: LGTM!The non-streaming path correctly creates the reasoning detail with the encrypted signature and sets the
IDfield totool_call_<callID>when aFunctionCallis present on the same part. This enables proper lookup inutils.goduring message conversion.
Merge activity
|
akshaydeo
deleted the
12-28-fix_thought_signature_handling_in_gemini_chat_completion_tool_call
branch
Summary
Refactored how thought signatures are handled for Gemini tool calls by moving them from
ExtraContentto theReasoningDetailsstructure with proper ID linking.Changes
ExtraContentmapChatReasoningDetailsstruct to link reasoning details to specific tool callstool_call_<callID>) for linking tool calls with their reasoning detailsType of change
Affected areas
How to test
Test Gemini provider with tool calls to ensure thought signatures are properly preserved:
Breaking changes
Security considerations
This change maintains the same security properties for thought signatures but reorganizes how they're stored and retrieved in the data structures.
Checklist