fix issue with tool calling using gemini 3.0 pro perview#322
fix issue with tool calling using gemini 3.0 pro perview#322lukemilby wants to merge 1 commit intosipeed:mainfrom
Conversation
…ature from extra content
|
@Zepan Fixes tool calling with Gemini 3.0 Pro Preview. Gemini is a popular provider and tool calling is a core feature. Recommendation: Merge. +47/-16, targeted fix for a widely-used provider. |
nikolasdehor
left a comment
There was a problem hiding this comment.
Good fix for a real breakage with Gemini 3.0 Pro thinking mode. The thought_signature round-trip is necessary per Google's docs — without it, the API returns 400 on subsequent tool call requests.
Suggestions (non-blocking):
-
Duplicate types:
ExtraContent/GoogleExtraContentare defined identically in bothpkg/providers/types.goandpkg/tools/types.go. Consider defining once and importing to avoid drift. -
Future-proofing: Consider using
json.RawMessageforextra_contentto generically round-trip any provider's extra fields without needing struct changes per vendor. -
Tests: This is a critical code path — a unit test parsing a sample Gemini response with
thought_signatureand verifying round-trip would prevent regressions.
LGTM — approving as the fix is correct and urgent.
nikolasdehor
left a comment
There was a problem hiding this comment.
Good fix for Gemini 3.0 Pro thought_signature requirement. The approach of passing ExtraContent through the ToolCall chain is correct.
A few observations:
-
Duplicated types: ExtraContent and GoogleExtraContent are defined in both pkg/providers/types.go and pkg/tools/types.go. This is a maintenance risk -- if Google adds more fields, you need to update both. Consider having one package import from the other, or defining a shared types package.
-
Hardcoded to Google: The ExtraContent struct only has a Google field. If other providers adopt similar patterns, you would need to keep adding top-level fields. An alternative approach would be map[string]interface{} for ExtraContent to make it provider-agnostic, though the struct approach is safer for type-checking.
-
No tests: This PR lacks unit tests for the thought_signature roundtrip. A test that verifies parseResponse correctly extracts thought_signature from a Gemini response and that it survives through to assistantMsg.ToolCalls would increase confidence.
-
Forward compatibility: If Google adds new required fields, the current approach requires a code change each time. Worth considering a more flexible ExtraContent representation for future iterations.
Overall, this addresses a real issue blocking Gemini 3.0 Pro users. The approach is reasonable.
|
|
|
@lukemilby Hi! This Gemini tool calling fix has had no activity for over 2 weeks, so I'm closing it to keep things tidy. Feel free to reopen anytime if it's still relevant. |
issue where gemini 3.0 pro preview would error because google's thought_signature was missing
response I would get