fix(gemini): strip unsupported JSON Schema fields from tool declarations#374
Conversation
Every exec against the Google/Gemini provider failed: Zero emits tool parameter schemas in OpenAI's JSON Schema dialect, which always includes `additionalProperties` (schemaToRuntimeMap writes it unconditionally). Google's functionDeclarations schema doesn't recognize that keyword and 400s the whole request — "Unknown name \"additionalProperties\" … Cannot find field" — so no tool-using call ever reached the model. Sanitize tool parameters in the Gemini mapper (only there — the field is valid and wanted for OpenAI strict mode and harmless for Anthropic): - sanitizeGeminiSchema keeps only the keywords Gemini's Schema type supports (type, description, enum, properties, required, items, anyOf, default, nullable, format, and the min/max/length constraints) via an allowlist, so any future OpenAI-ism or MCP-supplied field can't leak. - Recurses through properties, items, and anyOf so nested schemas are cleaned at every depth, not just the top level. Tests: a unit test on the sanitizer (nesting, legit fields preserved, nil-safe) and an end-to-end test asserting the outgoing request body's functionDeclarations[].parameters carry no additionalProperties while the real schema survives. Bypassing the sanitizer reproduces the reported payload, so the coverage is red-green. Fixes #373
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
WalkthroughAdds a ChangesGemini Tool Schema Sanitization
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant geminiRequest
participant sanitizeGeminiSchema
participant GeminiAPI
Caller->>geminiRequest: build request with tools
geminiRequest->>sanitizeGeminiSchema: tool.Parameters
sanitizeGeminiSchema->>sanitizeGeminiSchema: recursively strip unsupported keys
sanitizeGeminiSchema-->>geminiRequest: sanitized schema
geminiRequest->>GeminiAPI: send functionDeclarations with sanitized Parameters
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/providers/gemini/provider.go (1)
353-368: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAllowlist matches Gemini's documented Schema fields.
Cross-checked against Gemini's Schema object docs —
type,format,nullable,enum,items,properties,required,anyOf,propertyOrdering,pattern,minLength/maxLength,minProperties/maxProperties,exampleare all genuinely supported fields, and the allowlist approach correctly guards against other unsupported keywords (const,propertyNames,exclusiveMinimum/Maximum,prefixItems) that real-world bug reports show Gemini also rejects at arbitrary nesting depths.One note:
oneOf(common for discriminated unions in some JSON Schema generators) isn't in the allowlist, so it's silently dropped rather than preserved/recursed likeanyOf. Likely fine given Zero emitsanyOfper the tests, but worth keeping in mind if a future tool schema source usesoneOf.🤖 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 `@internal/providers/gemini/provider.go` around lines 353 - 368, The Gemini schema allowlist in geminiSchemaFields omits oneOf, so schemas using discriminated unions will lose that keyword during payload filtering. Update the schema handling in provider.go so oneOf is treated consistently with anyOf (preserved or recursively processed where appropriate), while keeping the existing allowlist behavior for unsupported fields.
🤖 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 `@internal/providers/gemini/provider.go`:
- Around line 353-368: The Gemini schema allowlist in geminiSchemaFields omits
oneOf, so schemas using discriminated unions will lose that keyword during
payload filtering. Update the schema handling in provider.go so oneOf is treated
consistently with anyOf (preserved or recursively processed where appropriate),
while keeping the existing allowlist behavior for unsupported fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b1f6d97-6d59-4a9a-be2f-f71e38cdd3b6
📒 Files selected for processing (2)
internal/providers/gemini/provider.gointernal/providers/gemini/provider_test.go
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Verdict: Approve ✅
Fixes a launch-blocking bug — with tools advertised (i.e. essentially every agentic zero exec), Gemini 400-rejects every functionDeclaration on the OpenAI-ism additionalProperties. Checked out, built, vetted, and ran the tests on the branch.
Correctness — solid
- Right scope. Sanitizing in the Gemini mapper only is the correct call —
additionalProperties: falseis wanted for OpenAI strict mode and harmless for Anthropic, so a global strip would be wrong. The base runtime schema is left intact for the other providers. ✔ - Allowlist is complete and accurate. I checked it against Google's Generative-Language
Schematype: the 22 kept keywords match exactly (type/format/title/description/nullable/default/items/minItems/maxItems/enum/properties/propertyOrdering/required/minProperties/maxProperties/minimum/maximum/minLength/maxLength/pattern/example/anyOf). Allowlist (vs denylist) is the right choice — a future OpenAI-ism or MCP-supplied field can't silently leak. ✔ - Recursion is right.
propertiesrecurses into the sub-schema values (not the property names),items/anyOfhandled, non-map values passed through,nil→nil. Confirmed the reported nestedadditionalPropertiesis stripped at depth by the test. ✔ - No leak path.
geminiRequestis the single site that buildsfunctionDeclarations(one completion path atprovider.go:107), so there's no second unsanitized route. ✔
Local verification (branch checked out)
go build ./... clean · go vet ./internal/providers/gemini/... clean · new TestSanitizeGeminiSchemaStripsUnsupportedFields + TestGeminiRequestOmitsAdditionalPropertiesInToolSchema pass · full gemini package green. The E2E test asserting the real outgoing body carries no additionalProperties while the schema survives is exactly the right regression guard.
Nits (non-blocking)
itemsin JSON Schema can be an array (tuple form); only the object form recurses. Immaterial here — Gemini doesn't support tupleitemsanyway, so an array would be equally unusable with or without this PR.- No CHANGELOG entry.
Neither blocks merge. This one is worth prioritizing — Gemini is completely unusable for tool calls without it. Thanks @gnanam1990.
…ons (Gitlawb#374) Every exec against the Google/Gemini provider failed: Zero emits tool parameter schemas in OpenAI's JSON Schema dialect, which always includes `additionalProperties` (schemaToRuntimeMap writes it unconditionally). Google's functionDeclarations schema doesn't recognize that keyword and 400s the whole request — "Unknown name \"additionalProperties\" … Cannot find field" — so no tool-using call ever reached the model. Sanitize tool parameters in the Gemini mapper (only there — the field is valid and wanted for OpenAI strict mode and harmless for Anthropic): - sanitizeGeminiSchema keeps only the keywords Gemini's Schema type supports (type, description, enum, properties, required, items, anyOf, default, nullable, format, and the min/max/length constraints) via an allowlist, so any future OpenAI-ism or MCP-supplied field can't leak. - Recurses through properties, items, and anyOf so nested schemas are cleaned at every depth, not just the top level. Tests: a unit test on the sanitizer (nesting, legit fields preserved, nil-safe) and an end-to-end test asserting the outgoing request body's functionDeclarations[].parameters carry no additionalProperties while the real schema survives. Bypassing the sanitizer reproduces the reported payload, so the coverage is red-green. Fixes Gitlawb#373
3 participants
Problem
Fixes #373. Every
zero execagainst the Google/Gemini provider fails as soon as tools are advertised (i.e. essentially all agentic use). Zero serializes tool parameter schemas in OpenAI's JSON Schema dialect —schemaToRuntimeMapwritesadditionalPropertieson every tool unconditionally — and Google'sfunctionDeclarationsschema doesn't recognize that keyword:The request is 400-rejected for every declaration, so the call never reaches the model. Reported against v0.1.0 with
gemini-2.5-pro.Root cause
internal/agent/loop.go—schemaToRuntimeMapalways emits"additionalProperties"into a tool's runtime parameter map.internal/providers/gemini/provider.go— the mapper passedtool.Parametersverbatim intofunctionDeclarations[].parameters.Fix
Sanitize tool parameters in the Gemini mapper only —
additionalProperties: falseis valid and wanted for OpenAI strict mode and harmless for Anthropic, so this must not be stripped globally.sanitizeGeminiSchemakeeps only the keywords Google's Schema type supports (type,description,enum,properties,required,items,anyOf,default,nullable,format, and the min/max/length constraints) via an allowlist, so any future OpenAI-ism or MCP-supplied field can't silently leak into a Gemini payload.properties,items, andanyOf, so nested schemas are cleaned at every depth — not just the top level.nilparameters staynil(parameterless tools unaffected).Tests
sanitizeGeminiSchema: nested stripping, legitimate fields (type/description/enum/required/default) preserved,nil-safe.functionDeclarations[].parameterscarry noadditionalPropertieswhile the actual schema survives.Full suite
go test ./...passes (73/73 packages); change is isolated to the Gemini provider.Summary by CodeRabbit