fix(openai-chat): deliver tool-result images to vision models - #912
Conversation
|
✅ PR quality gates passed This pull request now targets The title was left unchanged. The pull request has been marked ready for review again. |
📝 WalkthroughWalkthroughThe ChangesTool-result image forwarding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ToolResult
participant OpenAIChatAdapter
participant OpenAIChatRequest
ToolResult->>OpenAIChatAdapter: Provide text and image content
OpenAIChatAdapter->>OpenAIChatRequest: Emit text-only tool message with [image]
OpenAIChatAdapter->>OpenAIChatRequest: Emit follow-up user vision message with image_url parts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@docs-site/src/content/docs/reference/adapters.md`:
- Around line 34-36: Add the tool-result image behavior to the corresponding
openai-chat sections in both translated adapter pages: document that the
text-only role:"tool" message retains a short [image] marker, while the actual
images are delivered as image_url parts in one follow-up user vision message
after the tool round closes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 839db9c6-9a30-4061-84de-2b79886cba9c
📒 Files selected for processing (5)
docs-site/src/content/docs/ja/reference/adapters.mddocs-site/src/content/docs/ko/reference/adapters.mddocs-site/src/content/docs/reference/adapters.mdsrc/adapters/openai-chat.tstests/openai-chat-tool-result-images.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a719509eaf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const parts: unknown[] = []; | ||
| for (const p of content) { | ||
| if (p.type !== "image") continue; | ||
| parts.push({ type: "image_url", image_url: { url: p.imageUrl, ...(p.detail ? { detail: p.detail } : {}) } }); |
There was a problem hiding this comment.
Guard empty tool-result image URLs
When a tool output contains a malformed input_image with an empty image_url (the tool-output parser currently accepts any string here, including the image_url: "", file_id: "..." shape that user input omits), this new carrier forwards {"image_url":{"url":""}} to the chat provider. Vision-capable OpenAI-compatible endpoints reject malformed image_url parts, so a bad tool image that previously degraded to the [image] marker can now fail the whole request; skip non-usable URLs before adding carrier parts.
AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
Images in a tool_result were flattened to a literal "[image]" marker, so vision-capable routed models (kimi-code, DeepSeek, ...) described images they never saw. role:"tool" content is text-only on chat providers, so images now ride in a follow-up user vision message released once the tool round closes, mirroring the Google and Kiro adapters. Models in noVisionModels still use the vision sidecar. Fixes lidge-jun#888
a719509 to
9c8a0c5
Compare
The tool-output parser accepts any string for image_url, including
the empty file_id shape, and a {"url":""} carrier part would fail
the whole request where the "[image]" marker degrades safely.
The first commit only updated the en, ja, and ko adapter pages.
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 `@docs-site/src/content/docs/ru/reference/adapters.md`:
- Around line 37-39: Update the image forwarding descriptions in
docs-site/src/content/docs/ru/reference/adapters.md lines 37-39 and
docs-site/src/content/docs/zh-cn/reference/adapters.md lines 33-34 to state that
direct image_url forwarding applies only to vision-capable models; explicitly
document that models in noVisionModels continue using the vision sidecar path.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 78619e20-7ec9-41d1-8bba-b4ab1e34299f
📒 Files selected for processing (4)
docs-site/src/content/docs/ru/reference/adapters.mddocs-site/src/content/docs/zh-cn/reference/adapters.mdsrc/adapters/openai-chat.tstests/openai-chat-tool-result-images.test.ts
| - **Изображения из результатов инструментов** отправляются отдельным последующим user-сообщением | ||
| (части `image_url`) после закрытия раунда инструментов, так как содержимое `role:"tool"` может | ||
| быть только текстом; маркер `[image]` остаётся в сообщении инструмента как якорь. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify direct image forwarding by model capability.
Both localized pages describe direct image_url forwarding as universal. Models listed in noVisionModels continue to use the vision sidecar.
docs-site/src/content/docs/ru/reference/adapters.md#L37-L39: State that directimage_urlforwarding applies to vision-capable models; document the sidecar path fornoVisionModels.docs-site/src/content/docs/zh-cn/reference/adapters.md#L33-L34: Add the samenoVisionModelssidecar qualification.
📍 Affects 2 files
docs-site/src/content/docs/ru/reference/adapters.md#L37-L39(this comment)docs-site/src/content/docs/zh-cn/reference/adapters.md#L33-L34
🤖 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 `@docs-site/src/content/docs/ru/reference/adapters.md` around lines 37 - 39,
Update the image forwarding descriptions in
docs-site/src/content/docs/ru/reference/adapters.md lines 37-39 and
docs-site/src/content/docs/zh-cn/reference/adapters.md lines 33-34 to state that
direct image_url forwarding applies only to vision-capable models; explicitly
document that models in noVisionModels continue using the vision sidecar path.
Source: Path instructions
lidge-jun
left a comment
There was a problem hiding this comment.
Approving. Verified on a merged scratch tree against dev at fa51fce54.
Why this is the right fix for #888. The reporter's route was traced end to end: Claude Code sends an Anthropic tool_result → src/claude/inbound.ts:96-113 preserves the images as input_image → src/responses/parser.ts:547-555 produces an internal toolResult with structured image parts → Kimi OAuth (src/providers/registry.ts:712-741) and Kimi API-key (:1428-1444) both resolve to openai-chat, which flattened it through contentPartsToText. That flattening is the [image] placeholder in the report.
Carrying the images on a following user message is also the correct shape rather than a workaround — the Chat Completions schema accepts only strings or text parts in a tool message, and this repository already does the same thing elsewhere: src/adapters/google.ts:196-208 puts inline_data beside the functionResponse in a user turn, and src/adapters/kiro.ts:511-531 uses a corresponding user carrier.
Evidence.
- Merge onto current
devis clean; 7 files, +207/−1. - Focused suite: 5 pass / 0 fail (31 assertions).
- Ablation — with
src/adapters/openai-chat.tsreverted todevand the tests kept: 4 fail / 1 pass, and the survivor is exactly the image-free control. The test is not tautological. bun x tsc --noEmitexit 0.tests/openai-chat-eof.test.ts+openai-chat-hardening.test.ts+responses-parser.test.ts: 74 pass / 0 fail, confirming the finish-less-EOF handling from #896 is untouched. This mattered because #880 rewrote ~256 lines of the same file an hour before; the hunks land in the right regions after that refactor.- Full CI green on
d0a525d7.
Checked and cleared, for the record. Vision gating ordering is correct — the sidecar substitutes or fail-closed strips before adapter.buildRequest, and carriesImages() includes toolResult (src/vision/index.ts:187-194), so a noVisionModels target never reaches the new carrier with a raw image. No openai-chat provider in the registry enforces role alternation, so the assistant → tool → user(carrier) → user sequence is safe; Kiro is the only alternation-enforcing transport and it has its own adapter. Nothing post-processes what messagesToChatFormat returns.
One follow-up, not blocking. The five translated doc bullets describe direct image_url forwarding without noting that noVisionModels models get sidecar-generated text instead. Worth a sentence later.
Thanks for a clean fix with a test that actually drives the path.
Summary
Fixes #888. Images in a tool result were flattened to the literal string
"[image]", so vision models like kimi-k3 never saw the actual image and made up a description (Claude Code sends Read and screenshot output this way).role:"tool"messages can only carry text on chat providers, so the adapter now re-sends the image parts in a user vision message once the tool round closes, the same approach the Google and Kiro adapters already use. Models innoVisionModelsstill go through the vision sidecar.Also added a line to the adapter docs. The Japanese, Korean, Russian, and Chinese versions were done with AI translation, so a quick check from native speakers would help.
Verification
bun run test(includes the newtests/openai-chat-tool-result-images.test.ts),typecheck,lint:gui,privacy:scan."[image]".Checklist
Notes
Rebased onto current
dev(was accidentally based on thedev/mainrelease-promotion point for v2.10.0). Source commit before the rebase: a719509.Summary by CodeRabbit
New Features
[image]marker.Documentation