fix(providers/google): keep media tool results in the Gemini prompt - #58
Merged
Merged
Conversation
The Google provider had no case for media tool results, so a tool that returned an image (with or without accompanying text) emitted no function response at all and Gemini rejected the unpaired function call. Function responses are JSON only and nested functionResponse parts are accepted by Gemini 3+ models only, so send the accompanying text (or a placeholder) as the function response and attach the media as a sibling inline data part of the same turn.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The scanner reported standard library vulnerabilities fixed in Go 1.26.6 and GO-2026-6222 in golang.org/x/image v0.44.0. Keep the module baseline at go 1.26.5 for consumers and move only the scanner toolchain, and take x/image v0.45.0.
ibetitsmike
added a commit
to coder/coder
that referenced
this pull request
Sep 15, 2026
coder/fantasy#58 merged as bb47dc77e6e3, so the replace now points at a commit on the fork's tracked branch instead of the pull request head. The bump also takes golang.org/x/image v0.45.0 through the fork.
ibetitsmike
added a commit
to coder/coder
that referenced
this pull request
Sep 15, 2026
coder/fantasy#57 squash-merged into coder_2_33 as a511353f83eb. The openai provider is byte-identical to the previously pinned PR head f5b3f253c112; the new pin also carries coder/fantasy#58 (Gemini media tool results) and indirect x/image and x/text bumps that fix the govulncheck findings.
ibetitsmike
added a commit
to coder/coder
that referenced
this pull request
Sep 16, 2026
…9230) ## Why The coder/fantasy fork classifies OpenAI reasoning models by name. The pinned version did not know `gpt-6-astra`, so the Responses client omitted the `reasoning` parameter (the reasoning effort picker did nothing) and forwarded `temperature` (quickgen title calls got HTTP 400 `Unsupported parameter: 'temperature'` and retried). Every new model generation would need a fork release to fix this. Admins need a switch in the model config instead. ## What - Bump `charm.land/fantasy` to coder/fantasy `a511353f83eb`, the `coder_2_33` commit that squash-merged coder/fantasy#57 (the pin also carries coder/fantasy#58, Gemini media tool results): gpt-6 and later generations are recognized as reasoning models, and the provider gains `WithReasoningModelFunc` to override the classification. - New tri-state `openai_config.reasoning_model` on `ChatModelCallConfig`: unset keeps fantasy's name heuristics, `true` forces reasoning-model handling (effort and summary sent, temperature/top_p dropped), `false` forces plain sampling. It is applied once at client construction next to `use_responses_api`. The generated model editor shows it for `openai` and `bedrock` providers, since Bedrock Mantle also serves OpenAI-format models; it is ignored for Anthropic models on Bedrock. - Remove the `IsGPT6Astra` transport fallback in `chatopenai.UsesResponsesAPI`; the bumped SDK recognizes gpt-6 itself (`IsGPT6Astra` stays for the effort clamp). - Regenerated API docs, swagger, TypeScript types, and the model options schema; ARCHITECTURE.md documents the flag. ## Validation - New tests: `TestChatModelCallConfig_ReasoningModelRoundTrip`, `TestModelFromConfig_OpenAIReasoningModelOverride` (request body has `reasoning` and no `temperature` when forced on an unknown model; forced off on gpt-5 drops `reasoning` and keeps `temperature`; unset unchanged), `TestAIGatewayModelBedrockReasoningModelOverride` (Bedrock provider path), `TestIsZeroChatModelCallConfigReasoningModel`, and form-logic cases serializing the field for openai and bedrock. Red-green: removing the constructor wiring fails the request-body assertions. - `go test ./coderd/x/chatd/... -count=1` (full), `go test ./codersdk/... -run ChatModel`, `go test ./coderd/ -run 'ChatModel|Chats'`, `golangci-lint run ./coderd/x/chatd/... ./codersdk/... ./coderd/` (0 issues), `make gen` (stable on rerun), Vitest for `src/modules/aiModels` and `src/api/chatModelOptions` (174 tests), Biome, `tsc --noEmit`, `make pre-commit`. - Remote dogfood UAT (Coder Agents) exercised the editor and chat on the dev AI gateway: the switch shows for OpenAI and Bedrock providers only, persists true/false/unset, and `gpt-daybreak-blue-latest` (an alias the SDK does not recognize) sends `reasoning` without `temperature` when forced on and gets OpenAI's temperature 400 when off or unset. UAT also found the new switch colliding with `Use Responses API` at 375px; tri-state switches now take the full row below `sm`. One open observation for follow-up: in Coder chats on `gpt-6-astra` the upstream stream carried no reasoning-summary events even though `reasoning.summary=detailed` was sent, while the same model returns summaries to a standalone fantasy client; not attributed to this change. - Merged `origin/main` (`eceae296e8d`; the only conflict was the ARCHITECTURE.md TODO block, resolved by keeping main's transport-selection prose and adding a TODO for the GPT-6 Astra sentence this PR makes stale) and moved the pin (`553bfb86bf5`). On that head: `go build ./...`, `go vet ./coderd/x/chatd/... ./codersdk/`, `go test ./coderd/x/chatd/ -run 'ReasoningModel|TestAIGateway|Bedrock|ChatModelCallConfig|Quickgen|Transport'`, `go test ./coderd/x/chatd/chatopenai/... ./coderd/x/chatd/chatprovider/... ./coderd/x/chatd/chatdebug/...`, `go test ./codersdk/ -run 'ChatModel|Reasoning'`, and the pre-commit hook on both commits. The `providers/openai` tree at `a511353f83eb` is byte-identical to the previously pinned PR head `f5b3f253c112`. - The local `pre-push` hook fails in this checkout only on unrelated tests (umask-dependent `TestGetModulesArchive`, two untouched site tests), so the push bypassed it; CI is the gate. For the pin-bump push the hook ran once in full (31,276 tests passed; the only 2 failures were `TestGetModulesArchive`, proven to be this checkout's 0600 file modes: passes after normalizing modes and on an untouched `origin/main` worktree) and was then opted out for this worktree per Mike's instruction. coder/fantasy#57 is merged (`a511353f83eb` on `coder_2_33`) and the pin points at it. Related: #29229 (Bedrock `reasoning_summary` coercion). > Xum acted on behalf of @ibetitsmike for this PR.
ibetitsmike
added a commit
to coder/coder
that referenced
this pull request
Sep 17, 2026
Pull coder/fantasy#58 so the Google provider sends media tool results as a text function response plus a sibling inline data part instead of dropping them. Without it, keeping both the text and the image of a mixed MCP tool result would leave Gemini with an unpaired function call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Google provider's prompt conversion has cases for text and error tool results only. A tool result carrying media (
fantasy.ToolResultOutputContentMedia, for example a browser screenshot returned by an MCP tool) emits noFunctionResponsepart at all, so Gemini receives a function call with no paired response and rejects the request. This affects every media tool result, with or without accompanying text.Downstream in coder/coder chatd this blocks keeping both the text and the image of mixed MCP tool results (Playwright's default
[text, image]response): every other provider in this fork already renders media plus text, but a Gemini chat would break on the first screenshot.Fix
Add the media case to
toGooglePrompt. Gemini function responses are JSON only, and nestedfunctionResponse.partsare accepted by Gemini 3+ models only, so the accompanying text (or a placeholder naming the media type) is sent as the function response and the decoded media is attached as a siblinginlineDatapart of the same user turn, which every Gemini generation accepts. Invalid base64 keeps the text response and adds a call warning instead of dropping the pairing. The Vertex ID scrub is applied like the other cases.The
//nolint: unparamontoGooglePromptis removed because warnings are now produced.Validation
providers/google/tool_result_media_test.gocovers image with text, image without text (Vertex ID scrub and placeholder), and invalid base64. Proven red against the previousgoogle.goviago test -overlay(each test fails on the part-count assertion) and green with the fix.go build ./...,go test ./providers/google/ -count=1,gofumpt, andgolangci-lint run ./providers/google/...(v2.0.2) pass.