[fix]: Anthropic - include_server_side_tool_invocations now reaches the Gemini declaration-drop gate - #5935
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Anthropic Responses request converter promotes ChangesAnthropic tool opt-in propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/anthropic/responses_test.go (1)
403-425: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the mixed-tool regression in this test.
Lines 405-409 contain no
tools, so the test cannot detect whether Anthropic ingress preserves both a function tool and a server-side tool. Convert a request containing both tool types throughToGeminiResponsesRequestand assertfunctionDeclarations,googleSearch, andToolConfig.IncludeServerSideToolInvocations.Also add cases for explicit
false, an absent field, and a directboolinExtraParams. These cases cover the converter branches and preserve the distinction betweennilandfalse. The existingcore/providers/gemini/gemini_test.gotest covers the downstream builder, but it does not cover this Anthropic ingress boundary.As per coding guidelines, Go behavior changes should have deterministic, table-driven coverage.
🤖 Prompt for 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. In `@core/providers/anthropic/responses_test.go` around lines 403 - 425, Expand TestAnthropicIngressLiftsServerSideToolOptIn into deterministic table-driven coverage for mixed function and server-side tools, converting each case through ToGeminiResponsesRequest and asserting functionDeclarations, googleSearch, and ToolConfig.IncludeServerSideToolInvocations. Include explicit true, explicit false, absent, and direct bool ExtraParams cases, preserving nil versus false semantics and validating both tool types survive Anthropic ingress.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@core/providers/anthropic/responses_test.go`:
- Around line 403-425: Expand TestAnthropicIngressLiftsServerSideToolOptIn into
deterministic table-driven coverage for mixed function and server-side tools,
converting each case through ToGeminiResponsesRequest and asserting
functionDeclarations, googleSearch, and
ToolConfig.IncludeServerSideToolInvocations. Include explicit true, explicit
false, absent, and direct bool ExtraParams cases, preserving nil versus false
semantics and validating both tool types survive Anthropic ingress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 798f344e-a83d-48ed-a353-6d1044364eac
📒 Files selected for processing (2)
core/providers/anthropic/responses.gocore/providers/anthropic/responses_test.go
…he Gemini declaration-drop gate (#5935) * [fix]: anthropic - lift include_server_side_tool_invocations onto the typed param * [fix]: anthropic - compare raw token directly for server-side tool opt-in --------- Co-authored-by: Akshay Deo <akshay@akshaydeo.com>
…he Gemini declaration-drop gate (maximhq#5935) * [fix]: anthropic - lift include_server_side_tool_invocations onto the typed param * [fix]: anthropic - compare raw token directly for server-side tool opt-in --------- Co-authored-by: Akshay Deo <akshay@akshaydeo.com>
Summary
On
anthropic/v1/messages,include_server_side_tool_invocations: truewas silently ignored: the field is unregistered on the Anthropic schema, so it lands in ExtraParams as raw JSON, while the Gemini declaration-drop gate (added in #5576) reads the typedParams.IncludeServerSideToolInvocations. Combining a server-side tool with function tools on a Gemini-routed request therefore dropped the function declarations even with the opt-in set.Changes
ToBifrostResponsesRequestliftsinclude_server_side_tool_invocationsfrom ExtraParams onto the typed parameter, decoding thejson.RawMessagecapture (a plain type assertion never matches, which is why the flag was invisible)Type of change
Affected areas
How to test
cd core && go test ./providers/anthropic/ -run TestAnthropicIngressLiftsServerSideToolOptIn -vBreaking changes
If yes, describe impact and migration instructions.
Related issues
Closes #5679
Security considerations
None.
Checklist
docs/contributing/README.mdand followed the guidelines