From ab9918d6e451d377e8d84e8395b7e60d21d3fade Mon Sep 17 00:00:00 2001 From: Sachin Sharma Date: Wed, 23 Sep 2026 09:56:39 +0530 Subject: [PATCH] fix(images): label transcoded image parts with their real media type on the OpenAI wire Resolves the review finding left open on the merged #1738: with imageOptions.outputFormat set to "jpeg", real JPEG bytes reached the OpenAI chat-completions wire inside a data URI labelled image/png. Two independent drops caused it, and each one alone reproduces the bug: - core MessageBuilder rebuilt every image content part from `type` and `image` only, discarding the resolved media type on both buildMessages() and buildMessagesForStream(). The upstream builder emits it as `mimeType` while ImagePart declares `mediaType`, so both are read and the canonical `mediaType` is carried forward. - convertContentForOpenAI never passed the part's media type to imageDataToURL, so every raw-base64 image fell back to its hardcoded image/png default. The vision suite now asserts the wire label as well as the decoded bytes, on generate() and on stream(). stream() serializes to the wire at its own call site, so it gets its own label test: stripping the media type at only that site now fails that one test (it passed all four before) while every generate() test stays green. --- src/lib/core/modules/MessageBuilder.ts | 20 ++++ .../providers/openaiChatCompletionsClient.ts | 10 +- ...ntinuous-test-suite-vision-image-format.ts | 111 ++++++++++++++---- 3 files changed, 117 insertions(+), 24 deletions(-) diff --git a/src/lib/core/modules/MessageBuilder.ts b/src/lib/core/modules/MessageBuilder.ts index fc52d42706..bcd417cb52 100644 --- a/src/lib/core/modules/MessageBuilder.ts +++ b/src/lib/core/modules/MessageBuilder.ts @@ -199,9 +199,19 @@ export class MessageBuilder { }), }; } else if (item.type === "image") { + // Upstream image builders label the resolved format as + // either `mediaType` (canonical ImagePart field) or + // `mimeType` (messageBuilder's convertSimpleImagesToProviderFormat). + // Dropping it here silently reset every transcoded image + // back to the provider's hardcoded PNG default even + // though the transcoded bytes themselves were correct. + const mediaType = (item.mediaType ?? item.mimeType) as + | string + | undefined; return { type: "image" as const, image: (item.image as string) || "", + ...(mediaType && { mediaType }), ...(itemProviderOptions && { providerOptions: itemProviderOptions, }), @@ -350,9 +360,19 @@ export class MessageBuilder { }), }; } else if (item.type === "image") { + // Upstream image builders label the resolved format as + // either `mediaType` (canonical ImagePart field) or + // `mimeType` (messageBuilder's convertSimpleImagesToProviderFormat). + // Dropping it here silently reset every transcoded image + // back to the provider's hardcoded PNG default even + // though the transcoded bytes themselves were correct. + const mediaType = (item.mediaType ?? item.mimeType) as + | string + | undefined; return { type: "image" as const, image: (item.image as string) || "", + ...(mediaType && { mediaType }), ...(itemProviderOptions && { providerOptions: itemProviderOptions, }), diff --git a/src/lib/providers/openaiChatCompletionsClient.ts b/src/lib/providers/openaiChatCompletionsClient.ts index 18c7781405..2350d39b92 100644 --- a/src/lib/providers/openaiChatCompletionsClient.ts +++ b/src/lib/providers/openaiChatCompletionsClient.ts @@ -251,7 +251,15 @@ export const convertContentForOpenAI = ( (part as { image?: unknown; data?: unknown; url?: unknown }).image ?? (part as { data?: unknown }).data ?? (part as { url?: unknown }).url; - const url = imageDataToURL(data); + // ImagePart carries the resolved format as `mediaType` (MessageBuilder + // canonicalizes to it); `mimeType` is accepted for parts that bypass + // that normalization. Without this, a raw base64 payload (as opposed to + // an already-labelled data: URI) always fell back to imageDataToURL's + // hardcoded "image/png" default, mislabeling real JPEG/WebP/etc bytes. + const mediaType = + (part as { mediaType?: string }).mediaType ?? + (part as { mimeType?: string }).mimeType; + const url = imageDataToURL(data, mediaType); if (url) { out.push({ type: "image_url", image_url: { url } }); } diff --git a/test/continuous-test-suite-vision-image-format.ts b/test/continuous-test-suite-vision-image-format.ts index 30c8091353..815effdc76 100644 --- a/test/continuous-test-suite-vision-image-format.ts +++ b/test/continuous-test-suite-vision-image-format.ts @@ -13,21 +13,23 @@ * (`test/helpers/mockChatServer.ts`), not a live provider, so it needs no * credentials and runs unconditionally. * - * ## Why magic bytes, and nothing else + * ## Why magic bytes, primarily * - * Two things about the outbound request are proven unreliable for this - * assertion, so the test decodes the base64 image payload itself and reads - * its first bytes instead of trusting either: + * The outbound request's `data:` URI label used to be unreliable for this + * assertion: every `type:"image"` content part reached the OpenAI wire + * labelled `image/png` regardless of the bytes it actually encoded. Two + * independent drops caused it, and each alone reproduces the bug: core + * `MessageBuilder` rebuilt image parts without their resolved media type, + * and `convertContentForOpenAI` never read it before falling back to + * `imageDataToURL`'s hardcoded default. Both are fixed, so this suite checks + * the decoded bytes (the ground truth for what was actually sent) and, + * separately, that the label matches — on generate() and on stream(), which + * serialize to the wire at different call sites. * - * - **Filenames** carry no format information here at all — the fixture is - * an in-memory Buffer, never a path. - * - **The `data:` URI's declared mime label** is not authoritative. The - * OpenAI chat-completions adapter labels every `type:"image"` content - * part's data URI `image/png` regardless of the bytes it actually - * encodes — confirmed empirically: requesting `outputFormat: "jpeg"` - * produces a wire payload labelled `image/png` whose bytes are real - * JPEG. Asserting on the label would make this suite pass whether or not - * the transcode option did anything. + * - **Filenames** still carry no format information here at all — the + * fixture is an in-memory Buffer, never a path. + * - **Decoding the bytes** is still the primary assertion, since a label + * could in principle be fixed without the underlying bytes being correct. * * PNG magic bytes: `89 50 4E 47`. JPEG magic bytes: `FF D8 FF`. * @@ -69,7 +71,7 @@ async function makeTiffFixture(): Promise { * returns null) so a shape change in the request fails loudly instead of * being read as "no image". */ -function decodedImageBytesFromRequest(body: string): Buffer { +function extractImageDataUrl(body: string): string { const parsed = JSON.parse(body) as { messages?: ChatMessage[] }; const userMessage = parsed.messages?.find((m) => m.role === "user"); assert( @@ -79,19 +81,29 @@ function decodedImageBytesFromRequest(body: string): Buffer { const content = userMessage!.content as ImageUrlPart[]; const imagePart = content.find((p) => p.type === "image_url"); const url = imagePart?.image_url?.url ?? ""; - const commaIndex = url.indexOf(","); assert( - url.startsWith("data:") && commaIndex > -1, + url.startsWith("data:") && url.indexOf(",") > -1, "the captured request's image part was not a data: URI", ); - return Buffer.from(url.slice(commaIndex + 1), "base64"); + return url; } -async function runGenerateAndCapture( +function decodedImageBytesFromRequest(body: string): Buffer { + const url = extractImageDataUrl(body); + return Buffer.from(url.slice(url.indexOf(",") + 1), "base64"); +} + +/** The `data: