fix(responses): keep images in tool results instead of dropping them - #2117
fix(responses): keep images in tool results instead of dropping them#2117GhostFlying wants to merge 2 commits into
Conversation
convertToolMessageWithType only copied `text` parts when converting an
internal tool message back to a Responses API function_call_output. An
`image_url` part was skipped, and a result that carried nothing but an
image then fell through to the empty-string fallback, so the model
received `output: ""` — a successful-looking but blank tool result it
cannot distinguish from a tool that genuinely returned nothing.
The Responses schema allows text, image and file content in both
function_call_output and custom_tool_call_output (they share the
FunctionAndCustomToolCallOutput union), and the sibling paths already
handle this: convertUserMessage in this same file maps image_url to
input_image, and the Anthropic outbound transformer preserves images in
tool_result blocks. Only this converter dropped them.
`detail` is always sent: InputImageContent, which a custom_tool_call_output
content array resolves to, requires it, while the function_call_output param
schema makes it optional. Both document "auto" as the default.
Observed with Codex CLI, whose view_image tool returns a single base64
image with no text: every view of a rendered image arrived at the model as
an empty tool result, and the model confabulated a description of an image
it never received.
Two existing cases asserted the drop ("only text extracted", "no text
parts"); they now assert that the image survives. Added coverage for an
image-only result, a custom tool output with an image, and the serialised
wire shape.
Still unfixed, and unchanged here: part kinds this API cannot express in a
tool result (input_audio, video_url, document) are dropped silently, and an
all-dropped result is still indistinguishable from an empty one. Reporting
that needs a logger this pure converter has no access to.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTool-result conversion for the Responses API now preserves ChangesTool output conversion
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Greptile SummaryTool-result images are now preserved in Responses API output arrays.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| llm/transformer/openai/responses/outbound_convert.go | Preserves tool-result images and explicitly defaults image detail, resolving the previously reported custom-tool serialization issue. |
| llm/transformer/openai/responses/outbound_convert_test.go | Covers image-only and mixed tool results, custom outputs, default detail, and serialized output-array shape. |
Reviews (2): Last reviewed commit: "fix(responses): default tool image detai..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@llm/transformer/openai/responses/outbound_convert.go`:
- Around line 326-331: Default missing image detail to "auto" in the outbound
image conversion block that builds each input_image Item, while preserving
explicitly provided details. Update the expectations at
llm/transformer/openai/responses/outbound_convert_test.go:142-145, 208-215, and
281-284 to require Detail set to "auto" for omitted details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88437236-7ec1-4965-ae5a-d2f5ebc9a260
📒 Files selected for processing (2)
llm/transformer/openai/responses/outbound_convert.gollm/transformer/openai/responses/outbound_convert_test.go
The bug
convertToolMessageWithType(outbound_convert.go:301) only copiestextparts when turning an internalrole:"tool"message back into a Responsesfunction_call_output. Animage_urlpart is skipped, and a tool result that carried nothing but an image then falls through to the empty-string fallback a few lines below:So the model receives
{"type":"function_call_output","call_id":"...","output":""}— a successful, empty tool result, indistinguishable from a tool that genuinely returned nothing.Every other step of the path is already correct, which is what makes this a one-function bug:
convertContentItemToPart(inbound.go)input_image→image_urlpartllm.Message{Role:"tool"}MultipleContentconvertToolMessageWithTypeconvertUserMessagein the same file has acase "image_url"branch, and the Anthropic outbound transformer preserves tool-result images viaconvertToAnthropicTrivialContent(including data-URL → base64 source). Only this converter drops them.Spec basis
From the official spec (
openai/openai-openapi,main):FunctionCallOutputItemParam.output—oneOfa string, or "An array of content outputs (text, image, file) for the function tool call" whose items areInputTextContentParam | InputImageContentParamAutoParam | InputFileContentParam.FunctionToolCallOutput.output/CustomToolCallOutput.output— "Can be a string or an list of output content.", itemsFunctionAndCustomToolCallOutput=InputTextContent | InputImageContent | InputFileContent.Images in a tool result are a documented, first-class part of the API for both
function_call_outputandcustom_tool_call_output.Why this went unnoticed
Two things hide it:
input_image, which works. Only a tool that reads an image itself hits the broken field.Reproduction
Client: Codex CLI 0.144.3 →
/v1/responses, modelgpt-5.5. Codex'sview_imagetool returns a single base64 image with no text, i.e. exactly the shape that degrades to"".Payload: N solid squares of one colour (N random 2..7, colour random of 6), so a correct answer cannot be guessed — 1/36 by chance. Same PNG generator in all arms.
input_image(codex-i)view_image→function_call_output(before)view_image→function_call_output(after this patch)Before the patch the wrong answers were stereotyped ("3 red" twice out of three) — no visual information reached the model at all. Also reproduced 0/3 on
gpt-5.6-solbefore, 2/2 after.In a real agent loop the effect was worse than a wrong colour: the model called
view_imagethree times per run, received""each time, and then wrote confident descriptions of a rendered figure it had never seen — including describing a deliberately blank image as readable.The change
text-onlyifbecomes aswitch, withcase "image_url"→input_image, mirroringconvertUserMessage.detailis now always populated (lo.FromPtrOr(p.ImageURL.Detail, "auto")). This matters because of an asymmetry in the request schemas: acustom_tool_call_outputcontent array resolves toInputImageContent, whoserequiredis["type","detail"], whilefunction_call_outputresolves toInputImageContentParamAutoParam, wheredetailis optional. Both documentautoas the default, so sending it explicitly is valid on either side and avoids branching on the item type.Tests
TestConvertToolMessagegains: image-only result, image + unsupported parts, acustom_tool_call_outputcarrying an image. Two new tests assert the serialised wire shape (thatoutputis an array of content parts, and that a custom tool output's image carriesdetail).Two existing assertions changed, since they pinned the drop:
"tool message with multiple content - mixed types (only text extracted)"expectedtext/image/textto collapse totext/text; it now asserts the image survives in place, and is renamed accordingly."tool message with multiple content but no text parts"expected an image to become""; it now asserts the image survives, and is renamed.go test ./...in thellmmodule: 34 packages ok.gofmt/go vetclean.Deliberately not in this PR
MessageContentFromLLM(openai/outbound_convert.go:217) is role-agnostic and passesimage_urlstraight into tool messages, butChatCompletionRequestToolMessage.contentsays "For tool messages, only typetextis supported" — so a strict upstream may 400. Whether to drop, annotate or reject there is a design call, and the compatibility surface is different, so it seemed wrong to bundle it here. Happy to open a separate issue.input_audio/video_url/documentin a tool result still vanish, and an all-dropped result still degrades to"". Surfacing that properly needs a logger or an error return, which this pure converter has no access to — a separate change. The audio-only case is kept as a test with a comment so the gap is explicit rather than looking intentional.input_file. The spec allows it in the output array, but the ResponsesItemtype has no file fields at all (neither inbound nor outbound), so it is a symmetric feature gap rather than an asymmetric drop.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
input_imagewithdetaildefaulting to"auto".Bug Fixes