diff --git a/devlog/_plan/260905_external_image_roundtrip/000_plan.md b/devlog/_plan/260905_external_image_roundtrip/000_plan.md new file mode 100644 index 0000000000..8c3e09a673 --- /dev/null +++ b/devlog/_plan/260905_external_image_roundtrip/000_plan.md @@ -0,0 +1,68 @@ +# External-client image round trips + +## Loop specification + +- Class: C3 protocol compatibility repair; spec-satisfaction, no optimization race. +- Trigger: external clients report missing screenshots through OpenAI routes. +- Goal: preserve supported image bytes, URLs, ordering and detail across translation. +- Non-goals: no new uploader, provider settings, auth changes, live-service restart, + release, image synthesis, or unrelated adapter refactor. +- Verifier: standalone converter/parser/adapter body inspection, TypeScript, exact-head + GitHub CI. ALL local test suites are forbidden by the user, including focused suites. +- Stop: reviewed two-layer stack merged bottom-up to dev with green CI and ancestry. +- Memory: this unit and the session-bound goalplan/ledger. +- Outcomes: DONE only with proof; external dependencies may be BLOCKED/NEEDS_HUMAN; + unsafe expansion is UNSAFE. No implementation-success claim from docs-only work. +- Scope: this managed checkout, read-only Aside official docs, GitHub stack/CI/admin + merge. Maximum four concurrent agents; reassess after 90 minutes; no token cap set. +- Escalation: reclaim a lane after two distinct failed agents; any delegated writes + must be planned with disjoint paths before B. No production credentials in artifacts. + +## Measured baseline and hypotheses + +H1: normal user images disappear in Responses serialization. Falsifier: compare the +synthetic URL in final request JSON. REJECTED: direct Chat converter -> parseRequest -> +canonical forward buildRequest preserves the input_image, as does openai-chat. +H2: Chat ingress drops image metadata or tool-result images. Falsifier: compare role:user +and role:tool with identical image_url parts. CONFIRMED: user detail is absent and tool +output becomes just `Read this`. Source: src/chat/inbound.ts:80 and :284. +H3: external route/native forwarding or Claude ingress drops otherwise preserved +images. Independent read-only investigation pending; don't assume a token count alone +identifies a serializer. Plain Claude image and tool-result paths have dedicated mapping. + +Baseline command: standalone `bun -e` importing chat/inbound, responses/parser, +openai-responses, openai-chat and createTranslatorBudget. Exit 0; direct source imports +observe the actual owners, no bun:test import and no network. User image retained in +both wire formats; identical tool image absent from both. Original typecheck could not +resolve bun-types in this fresh worktree; frozen-lockfile dependency install (scripts +disabled) completed, with no manifest/lock edits. CI remains the test-suite authority. + +No-code options: do nothing leaves demonstrated loss; deletion/configuration cannot +restore discarded payloads. Reuse userContentToBlocks and existing downstream image +serialization. Do not add a generic image helper or patch correct Responses code. + +## Dependency-ordered roadmap + +1. wp0: docs-only roadmap and independent audit (this cycle). +2. wp1 / 010: preserve Chat image detail and structured tool output; lower PR to dev. +3. wp2 / 020: cross-protocol wire regressions and public contract; child PR to lower + branch, then CI/review/admin-merge bottom-up, retarget child and verify again. + +Existing placement is reused: src/chat/, tests/responses/, public reference/proxy-formats, +structure/04_transports-and-sidecars.md. No new package, runtime module, or config. +The user explicitly requested stacking; the upper layer consumes the corrected +converter and protects the integrated contract independently of unit-level assertions. + +## Continuity + +Roadmap audit: independent gpt-6-astra high reviewer returned GO-WITH-FIXES, +two medium findings. Both folded: exact no-suite typecheck/push commands and actual +Claude converter export. Direct node tsc exits 0. Standalone reproduction at +`.tmp/external-image-probe.ts` exits 1 before production edits with imageRetained=false +and detailRetained=false. No local suite ran. Aside opened official Chat docs confirming +image_url.url and nested auto/low/high detail; Anthropic tool-result docs confirm +nested image content. No upload handler is necessary for data URLs. + +Roadmap initially recorded against freshly fetched origin/dev. Never claim the whole +reported model-specific outage fixed merely because a converter fix lands. Preserve +the negative result for ordinary user images in the final report. diff --git a/devlog/_plan/260905_external_image_roundtrip/010_chat_image_parts.md b/devlog/_plan/260905_external_image_roundtrip/010_chat_image_parts.md new file mode 100644 index 0000000000..64e5db9ff3 --- /dev/null +++ b/devlog/_plan/260905_external_image_roundtrip/010_chat_image_parts.md @@ -0,0 +1,55 @@ +# Chat image-part foundation + +Depends on wp0. One full PABCD cycle, one lower PR. + +## MODIFY src/chat/inbound.ts + +Keep imageUrlFromPart's current object/string URL support. In userContentToBlocks, +extend the input_image push with detail from the nested image_url object, or the part +for the already-supported string shorthand. Preserve only auto/low/high detail. + +```diff +- blocks.push({ type: "input_image", image_url: imageUrl }); ++ const detail = isRec(raw.image_url) ? raw.image_url.detail : raw.detail; ++ blocks.push({ type: "input_image", image_url: imageUrl, ++ ...(detail === "auto" || detail === "low" || detail === "high" ? { detail } : {}) }); +``` + +For role:tool, reuse the existing content converter, retaining the original text-only +string behavior when no valid image is present. Responses function_call_output accepts +input_text/input_image, not input_video; don't newly forward video tool blocks. + +```diff +- const output = typeof msg.content === "string" ? msg.content : contentToText(msg.content); ++ const blocks = userContentToBlocks(msg.content); ++ const output = blocks.some(part => part.type === "input_image") ++ ? blocks.filter(part => part.type === "input_text" || part.type === "input_image") ++ : contentToText(msg.content); +``` + +Keep output_text tool parts supported: extend the reusable converter's text recognition +to output_text (already accepted by contentToText) so mixed arrays lose no old text. +Field chain: nested Chat detail -> input_image.detail -> parser image.detail -> existing +Chat image_url.detail; raw Responses preserves detail. No new type/enum/config. + +## MODIFY tests/responses/chat-completions-endpoint.test.ts + +Add converter-level cases beside the existing conversion tests: +- user image: remote/data URL, nested detail, no detail, string shorthand; +- tool image: function_call plus mixed text/image output retains exact order; +- image-only tool output stays a nonempty array; +- text-only string/array and invalid image keep existing text behavior; +- mixed output_text/image preserves text; video does not enter function output. +Use explicit expected objects, not converter-derived expectations. No removed assertions. + +## Acceptance and delivery + +Repeat the baseline standalone invocation: tool output must now contain input_image and +both final wire bodies must contain the synthetic URL; user detail must survive. Run +`node node_modules/typescript/bin/tsc --noEmit` (not a suite), add tests but execute +them only in CI. Review source and tests, commit, `git push --no-verify origin +codex/external-image-parts`, and open templated PR to dev. The user explicitly forbids +local suites; the installed pre-push hook runs package.json prepush including the full +suite, so that hook must be bypassed for this authorized push. No persistent hook +configuration change. Existing large files +are extended narrowly to avoid an unrelated split. No new exports or upload handler. diff --git a/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md b/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md new file mode 100644 index 0000000000..7baf039379 --- /dev/null +++ b/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md @@ -0,0 +1,60 @@ +# External wire contract and stack delivery + +Depends on wp1 and its corrected Chat converter. One full PABCD cycle. + +## MODIFY tests/responses/openai-responses-passthrough.test.ts + +Import real chatCompletionsToResponsesBody, anthropicToResponsesBody, +parseRequest, and createOpenAIChatAdapter wrapped with the +existing withTestTranslatorBudget. Add a table-driven regression for each ingress: +Chat user image, Chat tool screenshot (depends on wp1), Claude user image, Claude +tool_result image. Use data and HTTPS URL fixtures, two ordered images, and image-only +tool output. Build each through public API-key Responses, canonical ChatGPT forward, +and Chat adapter; assert exact image payloads in the actual serialized body, original +input immutability, and tool call/result adjacency. Add orphan tool-result case using +the existing repair path; don't modify production adapters unless evidence demands it. +Do not claim these body tests prove upstream model OCR or live route selection. + +## MODIFY docs-site/src/content/docs/reference/proxy-formats.md + +After the Chat intro add: + +```diff ++ Image URLs and base64 data URLs use Chat `image_url` content parts. Translation ++ preserves supported `detail` values (`auto`, `low`, `high`). OpenCodex also accepts ++ image-bearing tool-result arrays as a compatibility extension: Responses routes ++ retain structured output, while Chat adapters send tool images in a following user ++ message because the upstream Chat tool role is text-only. Plain text results remain ++ strings. Native passthrough follows its upstream contract. +``` + +No locale currently contradicts this additive contract; inspect sibling translated +sections before deciding whether an amendment is needed. Document no model entitlement. + +## MODIFY tests/responses/chat-completions-endpoint.test.ts + +Reuse mockDualWireUpstream (line 113) and dualWireConfig (line 2764), beside the +existing Chat-to-Responses HTTP regression (line 2834). POST a user image with high +detail and a paired tool screenshot to mock/grok-4.5; consume the stream and assert +one captured /responses body with unchanged ordered image parts. This is real HTTP +route proof in CI, not real-model OCR or canonical account authentication. + +## MODIFY structure/04_transports-and-sidecars.md + +Add one short paragraph beside the Chat inbound responsibility: its converter owns +detail and tool-image preservation; adapters own target-specific image placement. +Retain all existing transport/security/sidecar policy. + +## Acceptance / delivery + +Run `node node_modules/typescript/bin/tsc --noEmit`; public documentation build in CI +or local build (not tests); focused +regressions and full OS suites in GitHub CI only. A fresh independent patch audit +checks each wire assertion and absence of secret/logging changes. Publish child branch +codex/external-image-wire-contract against the open lower branch using `git push +--no-verify` (same explicit no-local-suite override as 010). Record admin bypass +authorization in both PR bodies; merge lower only when exact-head full CI is green, +don't delete parent branch. Prefer merge commits to preserve stack ancestry; retarget +the child to dev and refresh CI/review. If squash is used, restack and reverify its new +HEAD before merge. Fetch origin/dev and prove both merge SHAs ancestors. Archive this +unit only after the completed outcome is public. No restart/deployment is authorized. diff --git a/docs-site/src/content/docs/reference/proxy-formats.md b/docs-site/src/content/docs/reference/proxy-formats.md index 754d51a212..39251da8ba 100644 --- a/docs-site/src/content/docs/reference/proxy-formats.md +++ b/docs-site/src/content/docs/reference/proxy-formats.md @@ -197,6 +197,14 @@ non-empty `messages` array. It translates system, user, assistant, and tool mess Responses items; translates function tools, tool choice, images, reasoning effort, and supported response formats; runs the normal Responses routing pipeline; then translates the result back. +Image URLs and base64 data URLs use Chat `image_url` content parts. Translation preserves +supported `detail` values (`auto`, `low`, `high`). On translated routes, OpenCodex also accepts +image-bearing tool-result arrays as a compatibility extension: Responses routes retain structured +output, while the `openai-chat` adapter sends tool images in a following user message because Chat +tool content is text-only. Other downstream adapters own provider-specific placement. Plain text +results remain strings. Native passthrough follows its upstream contract; image support still depends +on the selected model and provider configuration. + Reasoning is part of that translation. `reasoning_effort` (or `reasoning.effort`) becomes internal `reasoning.effort`. Because the Responses parser hides thinking unless `reasoning.summary` is set and is not `none`, Chat Completions requests that ask for an diff --git a/src/chat/inbound.ts b/src/chat/inbound.ts index db3b41d12e..43024ca812 100644 --- a/src/chat/inbound.ts +++ b/src/chat/inbound.ts @@ -73,13 +73,18 @@ function userContentToBlocks(content: unknown): Rec[] { continue; } if (!isRec(raw)) continue; - if ((raw.type === "text" || raw.type === "input_text") && typeof raw.text === "string") { + if ((raw.type === "text" || raw.type === "input_text" || raw.type === "output_text") && typeof raw.text === "string") { blocks.push({ type: "input_text", text: raw.text }); continue; } const imageUrl = imageUrlFromPart(raw); if (imageUrl) { - blocks.push({ type: "input_image", image_url: imageUrl }); + const detail = isRec(raw.image_url) ? raw.image_url.detail : raw.detail; + blocks.push({ + type: "input_image", + image_url: imageUrl, + ...(detail === "auto" || detail === "low" || detail === "high" ? { detail } : {}), + }); continue; } const videoUrl = videoUrlFromPart(raw); @@ -275,7 +280,10 @@ export function chatCompletionsToResponsesBody(raw: unknown): Rec { : typeof msg.tool_use_id === "string" ? msg.tool_use_id : ""; if (!callId) throw new ChatCompletionsRequestError("tool messages require tool_call_id"); - const output = typeof msg.content === "string" ? msg.content : contentToText(msg.content); + const blocks = userContentToBlocks(msg.content); + const output = blocks.some(part => part.type === "input_image") + ? blocks.filter(part => part.type === "input_text" || part.type === "input_image") + : contentToText(msg.content); input.push({ type: "function_call_output", call_id: callId, output }); break; } diff --git a/tests/responses/chat-completions-endpoint.test.ts b/tests/responses/chat-completions-endpoint.test.ts index 7ca64ecdc6..4a4a5a5a12 100644 --- a/tests/responses/chat-completions-endpoint.test.ts +++ b/tests/responses/chat-completions-endpoint.test.ts @@ -238,6 +238,82 @@ test("chatCompletionsToResponsesBody maps messages/tools/system", () => { expect(input.some(i => i.type === "function_call_output" && i.call_id === "call_1")).toBe(true); }); +describe("chatCompletionsToResponsesBody image parts", () => { + test.each([ + { part: { type: "image_url", image_url: "https://example.com/image.png" }, expected: { type: "input_image", image_url: "https://example.com/image.png" } }, + { part: { type: "image_url", image_url: "https://example.com/image.png", detail: "low" }, expected: { type: "input_image", image_url: "https://example.com/image.png", detail: "low" } }, + { part: { type: "image_url", image_url: { url: "https://example.com/image.png" } }, expected: { type: "input_image", image_url: "https://example.com/image.png" } }, + ])("preserves user image shorthand and omitted detail: %j", ({ part, expected }) => { + const body = chatCompletionsToResponsesBody({ model: "mock/test-model", messages: [{ role: "user", content: [part] }] }); + expect(body.input).toEqual([{ type: "message", role: "user", content: [expected] }]); + }); + + test.each(["auto", "low", "high"])("preserves user image detail %s", detail => { + const url = "https://example.com/screenshot.png"; + const body = chatCompletionsToResponsesBody({ + model: "mock/test-model", + messages: [{ role: "user", content: [{ type: "image_url", image_url: { url, detail } }] }], + }); + expect(body.input).toEqual([{ type: "message", role: "user", content: [ + { type: "input_image", image_url: url, detail }, + ] }]); + }); + + test("retains ordered tool screenshots and legacy text without forwarding video", () => { + const url = "data:image/png;base64,aGVsbG8="; + const body = chatCompletionsToResponsesBody({ + model: "mock/test-model", + messages: [ + { role: "assistant", tool_calls: [{ id: "call_image", type: "function", function: { name: "screenshot", arguments: "{}" } }] }, + { role: "tool", tool_call_id: "call_image", content: [ + { type: "text", text: "before" }, + { type: "image_url", image_url: { url, detail: "high" } }, + { type: "output_text", text: "after" }, + { type: "video_url", video_url: "https://example.com/video.mp4" }, + { type: "image_url", image_url: "https://example.com/second.png", detail: "low" }, + ] }, + ], + }); + expect(body.input).toEqual([ + { type: "function_call", call_id: "call_image", name: "screenshot", arguments: "{}" }, + { type: "function_call_output", call_id: "call_image", output: [ + { type: "input_text", text: "before" }, + { type: "input_image", image_url: url, detail: "high" }, + { type: "input_text", text: "after" }, + { type: "input_image", image_url: "https://example.com/second.png", detail: "low" }, + ] }, + ]); + expect(() => parseRequest(body)).not.toThrow(); + }); + + test("keeps image-only tool results structured and ignores unsupported detail", () => { + const body = chatCompletionsToResponsesBody({ + model: "mock/test-model", + messages: [{ role: "tool", tool_call_id: "call_image", content: [ + { type: "image_url", image_url: { url: "https://example.com/first.png" } }, + { type: "image_url", image_url: { url: "https://example.com/second.png", detail: "invalid" } }, + ] }], + }); + expect(body.input).toEqual([{ type: "function_call_output", call_id: "call_image", output: [ + { type: "input_image", image_url: "https://example.com/first.png" }, + { type: "input_image", image_url: "https://example.com/second.png" }, + ] }]); + }); + + test.each([ + { content: "plain", expected: "plain" }, + { content: [{ type: "text", text: "one" }, { type: "output_text", text: "two" }], expected: "one\ntwo" }, + { content: [{ type: "image_url", image_url: { url: "" } }, { type: "text", text: "kept" }], expected: "kept" }, + { content: [], expected: "" }, + ])("preserves image-free tool output as a string: %j", ({ content, expected }) => { + const body = chatCompletionsToResponsesBody({ + model: "mock/test-model", + messages: [{ role: "tool", tool_call_id: "call_text", content }], + }); + expect(body.input).toEqual([{ type: "function_call_output", call_id: "call_text", output: expected }]); + }); +}); + describe("chatCompletionsToResponsesBody service_tier", () => { test("preserves a caller-supplied service_tier", () => { const body = chatCompletionsToResponsesBody({