Repository navigation
feat(devin): pass user and tool-result images to the wire #4513
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 |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # 000 — Devin 이미지 패스스루 | ||
|
|
||
| - 단위: `260913_devin_image_passthrough` | ||
| - 세션: `01a0985e-ce1a-7d12-81b9-c2e93a2bce67` (HOTL, cxc-loop) | ||
| - 기준: `origin/dev` | ||
|
|
||
| ## 증상 | ||
|
|
||
| 사용자가 Codex composer에 이미지를 붙여넣고 devin/swe-2에 보냈더니 턴이 0초에 죽었다. | ||
| 이미지가 전달되지 않아 tesseract OCR로 우회하려던 상황이었다. | ||
|
|
||
| ## 원인 — 세 층이 겹쳐서 | ||
|
|
||
| 와이어 계층(`src/adapters/devin/cloud-direct/chat.ts`)은 이미 멀티모달이다. | ||
| `ContentPart`에 `{type:"image", mimeType, base64Data, caption}`이 있고 | ||
| `encodeImageData`(:189-196)가 이를 `ChatMessagePrompt` 필드 #10 `ImageData` | ||
| `{#1 base64_data, #2 mime_type, #3 caption}`로 인코딩한다. extension.js 대조 검증 완료. | ||
|
|
||
| 그런데 매핑 계층(`src/adapters/devin.ts`)이 이미지를 버린다. | ||
|
|
||
| | 함수 | 줄 | 하는 일 | | ||
| |---|---|---| | ||
| | `textFromParts` | :205-209 | `type:"text"`만 뽑아 문자열로 반환 — 이미지는 빈 문자열 기여 | | ||
| | `mapOneMessage` (user) | :293-294 | 텍스트만 남기고 `if (!text) return undefined` — **이미지만 있는 메시지가 통째로 사라짐** | | ||
| | `toolResultText` | :211-214 | 같은 방식으로 툴 결과의 이미지도 버림 | | ||
|
|
||
| 사용자의 스크린샷에서 data: URI가 텍스트 첨부로 보인 것은 UI 표시이고, 실제로는 | ||
| `OcxImageContent`(`types/request.ts:189-195`)의 `imageUrl`이 data: URL로 들어온다. | ||
| 매핑이 그것을 인식하지 못하고 텍스트 추출에서 빈 문자열을 얻어 메시지를 드롭한다. | ||
|
|
||
| ## 수정 — `src/adapters/devin.ts` | ||
|
|
||
| ### 1) NEW: `mapOcxContentToWire` 헬퍼 | ||
|
|
||
| ```ts | ||
| import type { ContentPart } from "./devin/cloud-direct/chat"; | ||
|
|
||
| /** | ||
| * Convert inbound content parts to the wire shape the encoder accepts. | ||
| * | ||
| * The wire layer is already multimodal (ChatMessagePrompt field #10 ImageData), | ||
| * but every image was discarded here: textFromParts returned text-only strings, | ||
| * and a message whose only content was an image was dropped entirely. A data: | ||
| * URL carries everything field #10 needs; a remote https URL cannot be inlined | ||
| * without a fetch, so it stays as an explicit text reference rather than | ||
| * pretending the model can see a picture it cannot. Video has no Devin field. | ||
| */ | ||
| function mapOcxContentToWire(content: string | OcxContentPart[] | undefined): string | ContentPart[] { | ||
| if (typeof content === "string" || !Array.isArray(content)) return content ?? ""; | ||
| const out: ContentPart[] = []; | ||
| for (const part of content) { | ||
| if (part.type === "text" && part.text) out.push({ type: "text", text: part.text }); | ||
| else if (part.type === "image") { | ||
| const m = part.imageUrl.match(/^data:([^;]+);base64,(.+)$/); | ||
| if (m) out.push({ type: "image", mimeType: m[1]!, base64Data: m[2]! }); | ||
| else out.push({ type: "text", text: "[image url: " + part.imageUrl + "]" }); | ||
| } | ||
| } | ||
| return out; | ||
| } | ||
| ``` | ||
|
|
||
| ### 2) MODIFY: `mapOneMessage` user/developer 분기 | ||
|
|
||
| ```ts | ||
| // before | ||
| const text = textFromParts(message.content).trim(); | ||
| if (!text) return undefined; | ||
| return { role: ..., content: text }; | ||
|
|
||
| // after | ||
| const content = mapOcxContentToWire(message.content); | ||
| // 텍스트 없이 이미지만 있는 메시지도 유효하다 — 드롭하면 안 된다. | ||
| if (typeof content === "string" ? !content.trim() : content.length === 0) return undefined; | ||
| return { role: ..., content }; | ||
| ``` | ||
|
|
||
| ### 3) MODIFY: 툴 결과 | ||
|
|
||
| ```ts | ||
| // before | ||
| content: toolResultText(message), | ||
|
|
||
| // after — 오류 접두사는 유지하되, 이미지가 있으면 ContentPart[]로 넘긴다 | ||
| const wireContent = mapOcxContentToWire(message.content); | ||
| content: message.isError | ||
| ? (typeof wireContent === "string" ? "ERROR: " + wireContent | ||
| : [{ type: "text", text: "ERROR:" }, ...wireContent]) | ||
| : wireContent, | ||
| ``` | ||
|
|
||
| ## NEW: tests/providers/devin-image-passthrough.test.ts | ||
|
|
||
| | 케이스 | 기대 | | ||
| |---|---| | ||
| | data: URL 이미지 파트가 ContentPart image로 변환 | `{type:"image", mimeType:"image/png", base64Data:"iVBOR..."}` | | ||
| | 이미지만 있는 user 메시지가 드롭되지 않음 | items에 존재 | | ||
| | 텍스트 + 이미지 혼합 | 순서 보존 | | ||
| | https URL 이미지 | 텍스트 참조로 남음 | | ||
| | 툴 결과의 이미지 | ContentPart[]로 전달 | | ||
| | 툴 결과 오류 + 이미지 | ERROR 접두사 유지 | | ||
| | 와이어 인코딩 | buildGetChatMessageRequestForTests가 field #10을 냄 | | ||
|
|
||
| ## 레이아웃 등록 | ||
|
|
||
| - `scripts/test-layout/layout.json` explicit → providers | ||
| - `tests/fixtures/test-layout-expected.json` | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import type { AdapterEvent, OcxAssistantMessage, OcxContentPart, OcxMessage, Ocx | |
| import { namespacedToolName } from "../types"; | ||
| import type { IncomingMeta, ProviderAdapter } from "./base"; | ||
| import { streamChatEvents, allocateCascadeId, CloudChatError, type ChatHistoryItem, type ToolDef } from "./devin/cloud-direct"; | ||
| import type { ContentPart } from "./devin/cloud-direct/chat"; | ||
| import { getCachedCatalog } from "./devin/cloud-direct/catalog"; | ||
| import { collapseDevinModelUid } from "./devin/live-models"; | ||
| import { buildNonOpenAIToolCatalogNudgeForTools } from "./tool-catalog-nudge"; | ||
|
|
@@ -208,9 +209,31 @@ function textFromParts(content: string | OcxContentPart[] | undefined): string { | |
| return content.map((part) => (part.type === "text" ? part.text : "")).filter(Boolean).join("\n"); | ||
| } | ||
|
|
||
| function toolResultText(message: OcxToolResultMessage): string { | ||
| const body = textFromParts(message.content); | ||
| return message.isError ? ("ERROR: " + body) : body; | ||
| /** | ||
| * Convert inbound content parts to the multimodal shape the wire encoder accepts. | ||
| * | ||
| * The wire layer already carries images (ChatMessagePrompt field #10 ImageData), | ||
| * but every image was discarded at this boundary: textFromParts returned a | ||
| * text-only string and a message whose only content was an image was dropped | ||
| * entirely, which is why a pasted screenshot killed the turn and the only | ||
| * workaround was running OCR before sending. A data: URL carries everything | ||
| * field #10 needs; a remote https URL cannot be inlined without a fetch, so it | ||
| * stays as an explicit text reference rather than pretending the model can see | ||
| * a picture it cannot. Video has no Devin field and is skipped. | ||
| */ | ||
| function mapOcxContentToWire(content: string | OcxContentPart[] | undefined): string | ContentPart[] { | ||
|
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 changes the image-handling contract for AGENTS.md reference: src/AGENTS.md:L11-L11 Useful? React with 👍 / 👎. |
||
| if (typeof content === "string" || !Array.isArray(content)) return content ?? ""; | ||
| const out: ContentPart[] = []; | ||
| for (const part of content) { | ||
| if (part.type === "text" && part.text) { | ||
| out.push({ type: "text", text: part.text }); | ||
| } else if (part.type === "image") { | ||
| const m = part.imageUrl.match(/^data:([^;]+);base64,(.+)$/); | ||
| if (m) out.push({ type: "image", mimeType: m[1]!, base64Data: m[2]! }); | ||
| else out.push({ type: "text", text: `[image url: ${part.imageUrl}]` }); | ||
|
Comment on lines
+231
to
+233
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.
When an image is remote, or when its data URL is not recognized by this narrower regex—for example, base64 containing a newline, which the shared Useful? React with 👍 / 👎.
Comment on lines
+231
to
+233
Contributor
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Support parameterized data URLs.
Parse the full header and pass only the base media type to 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| function assistantToolCalls(message: OcxAssistantMessage): Array<{ id: string; name: string; arguments: string }> { | ||
|
|
@@ -290,9 +313,12 @@ export function mapOcxMessagesToDevin(parsed: OcxParsedRequest): ChatHistoryItem | |
|
|
||
| function mapOneMessage(message: OcxMessage): ChatHistoryItem | undefined { | ||
| if (message.role === "user" || message.role === "developer") { | ||
| const text = textFromParts(message.content).trim(); | ||
| if (!text) return undefined; | ||
| return { role: message.role === "developer" ? "system" : "user", content: text }; | ||
| const content = mapOcxContentToWire(message.content); | ||
| // An image with no caption text is a complete user message on its own. | ||
| // Dropping it — which is what the text-only extraction did — is why a | ||
| // pasted screenshot killed the turn before the model ever saw anything. | ||
| if (typeof content === "string" ? !content.trim() : content.length === 0) return undefined; | ||
| return { role: message.role === "developer" ? "system" : "user", content }; | ||
| } | ||
| if (message.role === "assistant") { | ||
| const toolCalls = assistantToolCalls(message); | ||
|
|
@@ -309,9 +335,15 @@ function mapOneMessage(message: OcxMessage): ChatHistoryItem | undefined { | |
| }; | ||
| } | ||
| if (message.role === "toolResult") { | ||
| const wireContent = mapOcxContentToWire(message.content); | ||
| const toolContent = message.isError | ||
| ? (typeof wireContent === "string" | ||
| ? `ERROR: ${wireContent}` | ||
| : [{ type: "text", text: "ERROR:" } as ContentPart, ...wireContent]) | ||
| : wireContent; | ||
| return { | ||
| role: "tool", | ||
| content: toolResultText(message), | ||
| content: toolContent, | ||
| tool_call_id: message.toolCallId, | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { mapOcxMessagesToDevin } from "../../src/adapters/devin"; | ||
| import { buildGetChatMessageRequestForTests } from "../../src/adapters/devin/cloud-direct/chat"; | ||
| import type { OcxMessage, OcxParsedRequest } from "../../src/types"; | ||
|
|
||
| const dataUrl = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg"; | ||
|
|
||
| function parsedWith(messages: OcxMessage[]): OcxParsedRequest { | ||
| return { | ||
| context: { | ||
| provider: "devin", | ||
| model: "swe-2", | ||
| systemPrompt: [], | ||
| tools: [], | ||
| messages, | ||
| }, | ||
| options: {}, | ||
| } as unknown as OcxParsedRequest; | ||
| } | ||
|
|
||
| describe("user image passthrough", () => { | ||
| test("a data: URL image part becomes a wire image with mime and base64", () => { | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "user", | ||
| content: [ | ||
| { type: "text", text: "이거 읽을수 있어?" }, | ||
| { type: "image", imageUrl: dataUrl }, | ||
| ], | ||
| }])); | ||
| const user = items.find(i => i.role === "user")!; | ||
| expect(Array.isArray(user.content)).toBe(true); | ||
| const parts = user.content as Array<Record<string, unknown>>; | ||
| expect(parts[0]).toEqual({ type: "text", text: "이거 읽을수 있어?" }); | ||
| expect(parts[1]).toEqual({ type: "image", mimeType: "image/png", base64Data: "iVBORw0KGgoAAAANSUhEUg" }); | ||
| }); | ||
|
|
||
| test("an image-only user message is not dropped", () => { | ||
| // This is the reported failure: a pasted screenshot with no caption killed | ||
| // the turn at 0s because the text-only extraction produced an empty string | ||
| // and the whole message was discarded. | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "user", | ||
| content: [{ type: "image", imageUrl: dataUrl }], | ||
| }])); | ||
| expect(items.filter(i => i.role === "user")).toHaveLength(1); | ||
| }); | ||
|
|
||
| test("a remote https image stays as an explicit text reference", () => { | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "user", | ||
| content: [{ type: "image", imageUrl: "https://example.com/pic.png" }], | ||
| }])); | ||
| const user = items.find(i => i.role === "user")!; | ||
| expect(user.content).toEqual([{ type: "text", text: "[image url: https://example.com/pic.png]" }]); | ||
| }); | ||
| }); | ||
|
Comment on lines
+21
to
+56
Contributor
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Assert tool-result part order
🤖 Prompt for AI Agents |
||
|
|
||
| describe("tool-result image passthrough", () => { | ||
| test("a tool result carrying an image keeps it", () => { | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "toolResult", | ||
| toolCallId: "call_1", | ||
| content: [ | ||
| { type: "text", text: "screenshot captured" }, | ||
| { type: "image", imageUrl: dataUrl }, | ||
| ], | ||
| } as unknown as OcxMessage])); | ||
| const tool = items.find(i => i.role === "tool")!; | ||
| const parts = tool.content as Array<Record<string, unknown>>; | ||
| expect(parts.some(p => p.type === "image" && p.base64Data === "iVBORw0KGgoAAAANSUhEUg")).toBe(true); | ||
| }); | ||
|
|
||
| test("an error tool result still carries the ERROR prefix alongside images", () => { | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "toolResult", | ||
| toolCallId: "call_1", | ||
| isError: true, | ||
| content: [{ type: "image", imageUrl: dataUrl }], | ||
| } as unknown as OcxMessage])); | ||
| const tool = items.find(i => i.role === "tool")!; | ||
| const parts = tool.content as Array<Record<string, unknown>>; | ||
| expect(parts[0]).toMatchObject({ type: "text", text: "ERROR:" }); | ||
| expect(parts.some(p => p.type === "image")).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("the wire encoder receives the image on field #10", () => { | ||
| test("a user image produces an ImageData submessage in the request frame", () => { | ||
| const items = mapOcxMessagesToDevin(parsedWith([{ | ||
| role: "user", | ||
| content: [{ type: "image", imageUrl: dataUrl }], | ||
| }])); | ||
| const buf = buildGetChatMessageRequestForTests({ | ||
| apiKey: "k", | ||
| modelUid: "swe-2-medium", | ||
| messages: items, | ||
| cascadeId: "c1", | ||
| sessionId: "s1", | ||
| requestId: 1n, | ||
| triggerId: "t1", | ||
| } as never); | ||
| // ImageData: field 10 (tag 0x52), containing base64 (field 1, string) | ||
| // and mime_type (field 2, string). The base64 payload is present. | ||
| expect(buf.includes(Buffer.from("iVBORw0KGgoAAAANSUhEUg"))).toBe(true); | ||
| expect(buf.includes(Buffer.from("image/png"))).toBe(true); | ||
| }); | ||
| }); | ||
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
Run the focused test-layout checks and typecheck.
AGENTS.md:215-224requires focused checks for the changed subsystem. Runbun test tests/test-layout.test.ts tests/test-layout-tooling.test.tsandbun run typecheck. Theprivacy:scancondition inscripts/AGENTS.md:25does not apply becauselayout.jsonis a test-layout mapping and does not handle credentials, requests, logs, or account data.🤖 Prompt for AI Agents