-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[agent] fix: send Google structured output on the generateContent wire and map Anthropic parallel=false #4536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -789,6 +789,37 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte | |
| : {}), | ||
|
|
||
| async buildRequest(parsed: OcxParsedRequest) { | ||
| // Structured-output admission runs FIRST, before messagesToGeminiFormat writes | ||
| // lastInjectedCallIds/lastReasoningReplayScope: a refused request must not leave | ||
| // adapter-scoped replay state pointing at call ids that never went out. These | ||
| // refusals are local and precede any fetch, and carry no request content, schema | ||
| // body, URL or credential. | ||
| const requestedTextFormat = parsed.options.textFormat; | ||
| if (requestedTextFormat) { | ||
| if (provider.googleMode === "cloud-code-assist") { | ||
| // Not implemented or verified by opencodex for the Cloud Code Assist envelope, | ||
| // including Claude models served through it. This is not a claim that the | ||
| // upstream cannot do it — silence would return unconstrained prose as success, | ||
| // which is the failure this fix exists to remove. | ||
| throw new Error( | ||
|
Comment on lines
+799
to
+804
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For routed Responses or Chat requests selecting Google, this change now enforces schemas on AI Studio/Vertex and locally rejects Cloud Code Assist and image-capable models, but AGENTS.md reference: src/AGENTS.md:L29-L29 Useful? React with 👍 / 👎. |
||
| "google cloud-code-assist structured output is not implemented by opencodex — " | ||
| + "remove response_format or route this model through AI Studio or Vertex", | ||
| ); | ||
| } | ||
| if (isImageCapableModel(parsed.modelId)) { | ||
| // An image-output model is configured with responseModalities; constraining the | ||
| // same turn to JSON text is contradictory. Say so rather than dropping the schema. | ||
| throw new Error( | ||
| "google image-capable models cannot combine image output with structured output — " | ||
| + "remove response_format or select a text model", | ||
| ); | ||
| } | ||
| if (requestedTextFormat.type === "json_schema" && !requestedTextFormat.schema) { | ||
| // Downgrading a malformed json_schema to bare JSON mode would silently drop the | ||
| // constraint the caller asked for. | ||
| throw new Error("google structured output requires text.format.schema for type json_schema"); | ||
| } | ||
| } | ||
| const routedModelId = provider.googleMode === "cloud-code-assist" | ||
| ? resolveAntigravityEffortWireModel( | ||
| parsed.modelId, | ||
|
|
@@ -846,6 +877,21 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte | |
| if (!generationConfig.thinkingConfig && isImageCapableModel(parsed.modelId)) { | ||
| generationConfig.responseModalities = ["TEXT", "IMAGE"]; | ||
| } | ||
| // Structured output travels in generationConfig on generateContent itself. | ||
| // responseJsonSchema takes ordinary JSON Schema (lowercase types), which is what | ||
| // options.textFormat.schema already holds; responseSchema would require Gemini's | ||
| // uppercase typed Schema form, and the docs require omitting it when | ||
| // responseJsonSchema is used. The response type does not change — the model | ||
| // returns text containing the conforming JSON — so response parsing is untouched. | ||
| // The tool-parameter sanitizer is deliberately NOT applied: it narrows a schema | ||
| // to the function-declaration subset and would corrupt a valid output schema. | ||
| const textFormat = parsed.options.textFormat; | ||
| if (textFormat) { | ||
| generationConfig.responseMimeType = "application/json"; | ||
| if (textFormat.type === "json_schema" && textFormat.schema) { | ||
| generationConfig.responseJsonSchema = textFormat.schema; | ||
| } | ||
| } | ||
| if (Object.keys(generationConfig).length > 0) body.generationConfig = generationConfig; | ||
|
|
||
| const method = parsed.stream ? "streamGenerateContent" : "generateContent"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,3 +47,27 @@ mismatched, and standalone results become marked text instead of unpaired functi | |
| Representable data-URL images remain sibling `inline_data` parts in either case. | ||
|
|
||
| > Decision record: [ADR-0058](../decisions/ADR-0058-google-tool-result-adjacency-repair.md) | ||
| ## Structured output on generateContent | ||
|
|
||
| A caller's Responses `text.format` reaches the Gemini wire as | ||
| `generationConfig.responseMimeType: "application/json"` plus, for `json_schema`, | ||
| `generationConfig.responseJsonSchema` carrying the schema unchanged. | ||
|
Comment on lines
+50
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This commit changes the AGENTS.md reference: structure/AGENTS.md:L49-L50 Useful? React with 👍 / 👎. |
||
| `responseJsonSchema` takes ordinary JSON Schema with lowercase type names, which is | ||
| the shape `options.textFormat.schema` already holds; `responseSchema` takes Gemini's | ||
| uppercase typed `Schema` form and is omitted when `responseJsonSchema` is used. The | ||
| response type is unchanged — the model returns text containing conforming JSON — so | ||
| response parsing is untouched. | ||
|
|
||
| The schema is carried verbatim. `sanitizeGeminiToolParameters` narrows a schema to | ||
| the function-declaration subset and must never be applied to a caller-authored output | ||
| schema. `compileGenerationConfig` in `google-wire-compiler.ts` is a whitelist, so | ||
| both keys are listed there as well; setting them in the adapter alone would drop them | ||
| before the wire. | ||
|
|
||
| Three cases refuse explicitly rather than dropping the constraint silently: | ||
| cloud-code-assist, which opencodex does not implement or verify for this field | ||
| (including Claude models served through that envelope — this is not a claim about | ||
| what the upstream can do); an image-capable model, whose `responseModalities` | ||
| configuration contradicts JSON-constrained text; and a `json_schema` format carrying | ||
| no schema, which would otherwise downgrade to bare JSON mode. An image-capable model | ||
| with no structured-output request keeps its existing `responseModalities` behavior. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| /** | ||
| * Audit F4 (2026-09-14): `options.parallelToolCalls === false` had no Anthropic | ||
| * consumer. The caller asked for one tool call at a time and the request went out | ||
| * unconstrained. | ||
| * | ||
| * Anthropic carries that intent as `disable_parallel_tool_use` nested INSIDE | ||
| * `tool_choice`. Per the tool-use docs its per-mode meaning is: | ||
| * auto -> at most one call; any -> exactly one; tool -> exactly one; | ||
| * none -> tool use already off, so the flag is irrelevant. | ||
| * The old code also emitted tool_choice only when an explicit choice was set, so a | ||
| * request carrying only parallel_tool_calls:false emitted nothing at all — the | ||
| * implicit default has to be stated for the flag to have somewhere to live. | ||
| * | ||
| * The flag constrains the model's OUTPUT, not execution order. | ||
| */ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { createAnthropicAdapter } from "../../../src/adapters/anthropic"; | ||
| import type { OcxParsedRequest, OcxProviderConfig, OcxTool } from "../../../src/types"; | ||
|
|
||
| const provider = { adapter: "anthropic", baseUrl: "https://api.anthropic.com", apiKey: "sk-x", authMode: "apiKey" } as unknown as OcxProviderConfig; | ||
|
|
||
| const TOOL = { name: "lookup", description: "Look something up", parameters: { type: "object", properties: {} } } as OcxTool; | ||
|
|
||
| async function toolChoiceOf(options: Record<string, unknown>, withTools = true): Promise<Record<string, unknown> | undefined> { | ||
| const parsed = { | ||
| modelId: "anthropic/claude-sonnet-4.5", | ||
| stream: false, | ||
| options, | ||
| context: { messages: [{ role: "user", content: "hi", timestamp: 0 }], ...(withTools ? { tools: [TOOL] } : {}) }, | ||
| } as unknown as OcxParsedRequest; | ||
| const { body } = await createAnthropicAdapter(provider).buildRequest(parsed); | ||
| const parsedBody = JSON.parse(typeof body === "string" ? body : JSON.stringify(body)) as { tool_choice?: Record<string, unknown> }; | ||
| return parsedBody.tool_choice; | ||
| } | ||
|
|
||
| describe("F4 parallel=false maps onto nested disable_parallel_tool_use", () => { | ||
| test("implicit auto is synthesized so the intent has somewhere to live", async () => { | ||
| expect(await toolChoiceOf({ parallelToolCalls: false })) | ||
| .toEqual({ type: "auto", disable_parallel_tool_use: true }); | ||
| }); | ||
|
|
||
| test("an explicit auto carries the flag", async () => { | ||
| expect(await toolChoiceOf({ toolChoice: "auto", parallelToolCalls: false })) | ||
| .toEqual({ type: "auto", disable_parallel_tool_use: true }); | ||
| }); | ||
|
|
||
| test("required maps to any and carries the flag", async () => { | ||
| expect(await toolChoiceOf({ toolChoice: "required", parallelToolCalls: false })) | ||
| .toEqual({ type: "any", disable_parallel_tool_use: true }); | ||
| }); | ||
|
|
||
| test("a named tool choice carries the flag", async () => { | ||
| const choice = await toolChoiceOf({ toolChoice: { name: "lookup" }, parallelToolCalls: false }); | ||
|
|
||
| expect(choice).toMatchObject({ type: "tool", disable_parallel_tool_use: true }); | ||
| expect(choice!.name).toBe("lookup"); | ||
| }); | ||
|
|
||
| test("allowed-tools auto and required both carry the flag", async () => { | ||
| // The IR shape is { allowedTools, mode } (src/types/tools.ts:294-299), which | ||
| // isAllowedToolChoice detects by the allowedTools key. | ||
| expect(await toolChoiceOf({ toolChoice: { allowedTools: ["lookup"], mode: "auto" }, parallelToolCalls: false })) | ||
| .toEqual({ type: "auto", disable_parallel_tool_use: true }); | ||
| expect(await toolChoiceOf({ toolChoice: { allowedTools: ["lookup"], mode: "required" }, parallelToolCalls: false })) | ||
| .toEqual({ type: "any", disable_parallel_tool_use: true }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("F4 cases that must not change", () => { | ||
| test("none stays bare — tool use is already off, so the flag is irrelevant", async () => { | ||
| expect(await toolChoiceOf({ toolChoice: "none", parallelToolCalls: false })).toEqual({ type: "none" }); | ||
| }); | ||
|
|
||
| test("no tools on the wire means no tool_choice at all", async () => { | ||
| expect(await toolChoiceOf({ parallelToolCalls: false }, false)).toBeUndefined(); | ||
| }); | ||
|
|
||
| test("parallel unset is byte-identical to today", async () => { | ||
| expect(await toolChoiceOf({ toolChoice: "auto" })).toEqual({ type: "auto" }); | ||
| expect(await toolChoiceOf({})).toBeUndefined(); | ||
| }); | ||
|
|
||
| test("parallel true never attaches the flag", async () => { | ||
| expect(await toolChoiceOf({ toolChoice: "auto", parallelToolCalls: true })).toEqual({ type: "auto" }); | ||
| expect(await toolChoiceOf({ parallelToolCalls: true })).toBeUndefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
scripts/AGENTS.md:25-26makesbun run privacy:scanandbun run prepushconditional. This mapping-only change does not handle privacy-sensitive data, release, packaging, dependency, or cross-platform tooling concerns. The layout is consumed byscripts/test-layout/schema.tsandscripts/test-layout/move.ts, so focused validation andbun run typecheckremain applicable.Complete the required validation for
scripts/test-layout/layout.json:211.Obtain the required explicit security review, run a focused test or probe for the mapping, and run
bun run typecheck. Runbun run privacy:scanonly when the change handles configuration, credentials, requests, logs, or account data. Runbun run prepushonly for release, packaging, dependency, or cross-platform tooling changes. Report any platform-specific validation that was not executed.🤖 Prompt for AI Agents