fix(gemini): preserve JSON Schema tool parameters - #1259
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe Gemini tool converter now sends input schemas through ChangesGemini JSON Schema tool support
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change preserves complete JSON Schema tool parameters in Gemini requests without changing the public usage contract. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ArkTypeWireTest
participant ArkTypeToolWireRoute
participant GeminiTextAdapter
participant GeminiJsonSchemaToolMount
ArkTypeWireTest->>ArkTypeToolWireRoute: request provider=gemini
ArkTypeToolWireRoute->>GeminiTextAdapter: configure Gemini model and test ID
GeminiTextAdapter->>GeminiJsonSchemaToolMount: send function declaration
GeminiJsonSchemaToolMount->>GeminiJsonSchemaToolMount: validate parametersJsonSchema
GeminiJsonSchemaToolMount-->>ArkTypeWireTest: return Schema accepted
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The implementation directly addresses issue [
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit cfeeb82
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-snippets
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-compaction
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-llmgateway
@tanstack/ai-lovable
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-octane
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-perplexity
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-upstash-box
@tanstack/ai-sandbox-vercel
@tanstack/ai-skills
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vercel-gateway
@tanstack/ai-vertex
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
@tanstack/svelte-ai-devtools
commit: |
harshlocham
left a comment
There was a problem hiding this comment.
The E2E failure appears to be in an existing spec rather than the new ArkType coverage.
tests/provider-tool-dispatch-wire.spec.ts still asserts that aimock's /v1/_requests journal contains function.parameters for the google_search tool. After this change, the Gemini converter intentionally sends the schema via parametersJsonSchema, while aimock's Gemini handler only copies parameters into the journal. As a result, the recorded function becomes:
{
type: 'function',
function: {
name: 'google_search',
description: 'Run an application function'
}
}I can reproduce this with:
pnpm run build:all
CI=1 pnpm --filter @tanstack/ai-e2e test:e2e -- tests/provider-tool-dispatch-wire.spec.tsThe existing name assertion still provides useful coverage that the tool isn't being swallowed into { googleSearch: ... }, while the new ArkType mount already covers the richer schema keywords.
I think the existing matcher should be updated to stop asserting function.parameters here (or, alternatively, inspect parametersJsonSchema through the raw mount). I wouldn't recommend sending both fields just to satisfy aimock, since FunctionDeclaration does not allow both.
|
Thanks for the PR, @kolaworld! 🙌 @tombeckenham will take a look. Automated pre-review checks
Automated triage — a human review follows. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
The converter now sends tool schemas via `parametersJsonSchema`, which
aimock's Gemini handler never records (it copies only `parameters` into
its `/v1/_requests` journal). Two consequences, both fixed here.
`provider-tool-dispatch-wire` asserted the journal's `function.parameters`,
so it failed deterministically after the switch. Drop that matcher and the
comment describing the old SDK-side upper-casing; the surviving
`name: 'google_search'` check is what the spec relies on to prove the tool
was not swallowed into `{googleSearch: {...}}`.
The new Gemini case in `arktype-tool-wire` asserted only `ok: true`, which
the route returns unless the SDK throws. Had the raw mount stopped matching,
the request would fall through to aimock's native Gemini handler, return
200, and the test would pass while proving nothing. Assert on the mount's
`Schema accepted` reply instead, which it emits only after verifying the
request carried `parametersJsonSchema` with `unit.const` and no `parameters`.
Verified: pointing the route at a model the mount does not intercept fails
on the new assertion while `ok: true` still comes back.
Claude-Session: https://claude.ai/code/session_01PG6uRwgdsxF9vrj973KsKX
tombeckenham
left a comment
There was a problem hiding this comment.
Approving. The fix is correct and I verified it against the live Gemini API rather than only against mocks.
Root cause
convertToolsToProviderFormat sent tool schemas on FunctionDeclaration.parameters. @google/genai's tTool routes that field through processJsonSchema, which upper-cases type and copies unrecognised keywords through verbatim — so const arrived on parameters, whose proto has no such member, and the API rejected the request before generation. The SDK only bypasses that path when the schema carries $schema, and the core layer strips $schema (packages/ai/src/activities/chat/tools/schema-converter.ts:22), so every tool took the lossy path. Sending parametersJsonSchema is the right fix — the SDK never reads or rewrites it, and it matches what the Gemini realtime client already does.
Verification
Agent-written repro against the real API on gemini-3.5-flash-lite, run on clean main and on this branch:
- main —
400 Invalid JSON payload received. Unknown name "const" at 'tools[0].function_declarations[0].parameters.properties[1].value' - this branch — passes
I also confirmed live that a zod-shaped schema (additionalProperties: false, nullable anyOf, enum, arrays) and the no-inputSchema default (required: []) both still work, since those paths had no coverage and were plausible regressions.
convertToolsToProviderFormat has a single call site (adapters/text.ts:877) shared by the streaming and non-streaming paths, so both are covered. ai-vertex re-exports from ai-gemini and inherits the fix; changesets bumps it as a dependent, so the single @tanstack/ai-gemini patch entry is correct.
Test changes I pushed
Two test-hygiene issues, fixed in cfeeb826f:
-
provider-tool-dispatch-wire.spec.tsasserted aimock's journalledfunction.parameters. aimock copies onlyparametersinto its journal, so after this change that assertion could never hold — it was the one deterministic E2E failure. Dropped the matcher and the now-false comment about SDK-side upper-casing; the survivingname: 'google_search'check is what proves the tool wasn't swallowed into{googleSearch: {...}}. -
The new Gemini case asserted only
ok: true, which the route returns unless the SDK throws. Had the raw mount ever stopped matching, the request would fall through to aimock's native handler, return 200, and the test would pass while proving nothing. It now asserts on the mount'sSchema acceptedreply. I verified this has teeth by repointing the route at a model the mount doesn't intercept:ok: truestill came back, and only the new assertion caught it.
CI is green — 634 passed, 0 failed. The 15 flaky are pre-existing 30s timeouts in embedding / tts / multi-turn-structured that hit a different provider set on each run and are unrelated to this change.
Follow-up (not blocking)
Structured output has the same bug class: adapters/text.ts:188 and :861 send output schemas via config.responseSchema, the same narrow Schema dialect through the same conversion, so a const in an output schema still fails. responseJsonSchema exists as a provider option but is typed Schema, so it would need widening too. Worth a separate issue.
Optional polish left alone: the new unit field sits next to a pre-existing units? and reads as a typo (scale would be clearer), and the OpenRouter HTTPClient / adapter are still constructed on the Gemini path.
Fixes #1258
🎯 Changes
FunctionDeclaration.parametersJsonSchemainstead of the narrowerparametersfield.constvalues produced by ArkType and other Standard Schema libraries.@google/genaiFunctionDeclarationtype, matching the existing Gemini Realtime implementation.@tanstack/ai-gemini. Docs are unchanged because public usage and the documented contract remain the same.Alternatives considered:
@copilotkit/aimock. Rejected because the project would need to carry a local dependency patch. The upstream aimock Gemini converter reads onlyparameters, notparametersJsonSchema. No matching upstream tracker issue is currently filed.✅ Checklist
pnpm run test:pr, or these tests do not apply to this pull request.docs/for this change, or this change is not user-facing.pnpm changeset), or this PR does not change a published package.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests