fix: gemini openai through signature compatibility - #4810
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds shared stripping logic for thought-signature suffixes, applies it to OpenAI chat and responses request conversion, centralizes the Gemini separator constant, and ensures Gemini function-call parts always carry a non-nil thought signature field. ChangesThought Signature Stripping
Sequence Diagram(s)sequenceDiagram
participant Bifrost as Bifrost request
participant OpenAIChat as ConvertBifrostMessagesToOpenAIMessages
participant Utils as core/providers/utils
participant OpenAI as OpenAI message
Bifrost->>OpenAIChat: assistant/tool call IDs
OpenAIChat->>Utils: StripThoughtSignature(callID)
Utils-->>OpenAIChat: base ID
OpenAIChat->>OpenAI: emit sanitized ToolCallID / ToolCalls
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
e175200 to
4e6b674
Compare
9ac2c54 to
af0b891
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/openai/utils.go`:
- Around line 44-74: Only strip verified Gemini thought signatures by updating
the shared StripThoughtSignature helper to require a decodable base64url payload
after the ThoughtSignatureSeparator instead of stripping any ID containing the
separator. Then ensure both the ChatToolMessage/ChatAssistantMessage handling in
openaiMessages conversion and the responses converter continue to call that
validated helper so non-Gemini IDs like report_ts_backup are left unchanged.
In `@core/providers/utils/utils.go`:
- Around line 47-51: StripThoughtSignature is too aggressive because it removes
any callID containing ThoughtSignatureSeparator, which collapses raw IDs like
search_ts_a and search_ts_b into the same value. Update this helper to strip
only positively identified Gemini-added thought signatures, not arbitrary IDs
that merely contain "_ts_", and make sure the OpenAI converters that rely on
StripThoughtSignature continue to preserve distinct upstream call IDs. Use the
existing ThoughtSignatureSeparator and StripThoughtSignature symbols to locate
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fcb619d-47a1-4be5-b829-2e6dbdc5c1f3
📒 Files selected for processing (8)
core/providers/gemini/responses.gocore/providers/gemini/types.gocore/providers/openai/chat_test.gocore/providers/openai/responses.gocore/providers/openai/responses_test.gocore/providers/openai/utils.gocore/providers/utils/utils.gocore/providers/utils/utils_test.go
4e6b674 to
a7d71e7
Compare
a7d71e7 to
0069e87
Compare
ee2940f to
f9dae1e
Compare
0069e87 to
1b6a465
Compare
Merge activity
|
The base branch was changed.
1b6a465 to
afebdf9
Compare
* upstream/dev: feat(mcp): add per-MCP-server tool execution timeout (maximhq#4472) fix: billing on failed responses stream requests anthropic and bedrock (maximhq#4842) fix: gemini openai through signature compatibility (maximhq#4810) fix: cancelled state in logs (maximhq#4831) fix: perplexity responses api compatibility (maximhq#4813) docs: clarify two-layer token refresh behavior and disabled-client refresh token expiry (maximhq#4849) fix: skip background token refresh for disabled/unconfigured MCP clients and guarantee non-nil logger in sync workers (maximhq#4848)

Summary
When a conversation switches from Gemini to OpenAI, Gemini embeds its
thoughtSignatureinto tool call IDs using the format<baseID>_ts_<signature>. OpenAI rejectscall_idvalues exceeding 64 characters, causing cross-provider conversation continuations to fail. This PR strips the embedded signature before sending IDs to OpenAI, while leaving the caller's input untouched so a subsequent Gemini turn can still recover the signature.Changes
ThoughtSignatureSeparator(_ts_) as a shared constant incore/providers/utilsand aStripThoughtSignaturehelper that returns the base ID by cutting at the separator.thoughtSignatureSeparatorconstant now references the sharedThoughtSignatureSeparatorinstead of a local string literal.ConvertBifrostMessagesToOpenAIMessages), assistant tool call IDs and tool resulttool_call_idvalues are stripped of any embedded signature. Structs are cloned before modification to avoid mutating the caller's input.ToOpenAIResponsesRequest),ResponsesToolMessage.CallIDis stripped the same way, also via a shallow clone.ThoughtSignature, it is now assignedskipThoughtSignatureValidatorto prevent downstream validation errors during cross-provider routing.StripThoughtSignatureedge cases, the chat converter stripping behavior (including immutability of the original input), and the Responses converter stripping behavior.Type of change
Affected areas
How to test
go test ./core/providers/openai/... ./core/providers/gemini/... ./core/providers/utils/...The new tests verify:
StripThoughtSignaturecorrectly handles IDs with and without embedded signatures.call_idfields without mutating the original request.Breaking changes
Security considerations
No auth, secrets, or PII implications. The stripping is deterministic and only affects tool call ID formatting on the wire.
Checklist
docs/contributing/README.mdand followed the guidelines