Skip to content

Commit 6188438

Browse files
committed
refactor(files): gate every vision passthrough on model-supported media types
Review found two passthroughs that still handed the model bytes it cannot decode. The sharp-load-failure branch returned raw HEIF, and the already-small-enough branch returned raw AVIF, TIFF, BMP or ICO — all of which isImageFileType accepts and no vision model does. Gating all three on the existing MODEL_SUPPORTED_IMAGE_MIME_TYPES subsumes the ad-hoc isHeifContainer re-sniff, and re-encoding an unsupported format falls out of the resize ladder that was already there. Also drop two constants that were pure indirection (a one-use alias for 'image/jpeg', and a quality value identical to heic-convert's default), trim the oversized comments, log successful transcodes so the ratio is visible in prod, and replace a detection test that could not fail.
1 parent 1257e7b commit 6188438

3 files changed

Lines changed: 46 additions & 44 deletions

File tree

apps/sim/lib/copilot/vfs/file-reader.ts

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ import { recordFileRead } from '@/lib/copilot/request/metrics'
1414
import { markSpanForError } from '@/lib/copilot/request/otel'
1515
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
1616
import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
17+
import { isHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic'
1718
import {
18-
HEIC_TRANSCODE_MEDIA_TYPE,
19-
isHeifContainer,
20-
transcodeHeicToJpeg,
21-
} from '@/lib/uploads/server/heic'
22-
import { isImageFileType, resolveEffectiveMimeType } from '@/lib/uploads/utils/file-utils'
19+
isImageFileType,
20+
MODEL_SUPPORTED_IMAGE_MIME_TYPES,
21+
resolveEffectiveMimeType,
22+
} from '@/lib/uploads/utils/file-utils'
2323

2424
// Lazy tracer (same pattern as lib/copilot/request/otel.ts).
2525
function getVfsTracer() {
@@ -121,7 +121,9 @@ async function prepareImageForVision(
121121
error: toError(err).message,
122122
})
123123
span.setAttribute(TraceAttr.CopilotVfsSharpLoadFailed, true)
124-
const fitsWithoutSharp = sourceBuffer.length <= MAX_IMAGE_READ_BYTES
124+
const fitsWithoutSharp =
125+
MODEL_SUPPORTED_IMAGE_MIME_TYPES.has(detectedType) &&
126+
sourceBuffer.length <= MAX_IMAGE_READ_BYTES
125127
span.setAttribute(
126128
TraceAttr.CopilotVfsOutcome,
127129
fitsWithoutSharp ? 'passthrough_no_sharp' : 'rejected_no_sharp'
@@ -142,15 +144,10 @@ async function prepareImageForVision(
142144
return null
143145
})
144146

145-
/**
146-
* sharp is always tried first — its libvips reads every format we accept
147-
* except HEVC-coded HEIF, and it is roughly an order of magnitude faster
148-
* than the WebAssembly decoder. Only bytes it cannot read at all are worth
149-
* a transcode. This is the same libvips-preferred / libheif-fallback
150-
* layering PhotoPrism uses, and it is deliberately capability-based:
151-
* choosing the decoder from the container brand would send AV1-coded
152-
* `mif1` files down the slow path even though sharp handles them natively.
153-
*/
147+
// sharp first: its libvips reads everything we accept except HEVC-coded
148+
// HEIF, and it is ~10x faster than the WASM decoder. Capability-based
149+
// rather than brand-based, so AV1-coded `mif1` — which sharp handles
150+
// natively — does not get sent down the slow path.
154151
let buffer = sourceBuffer
155152
let mediaType = detectedType
156153
let metadata = await readMetadata(sourceBuffer)
@@ -159,18 +156,17 @@ async function prepareImageForVision(
159156
const transcoded = await transcodeHeicToJpeg(sourceBuffer)
160157
if (transcoded) {
161158
buffer = transcoded
162-
mediaType = HEIC_TRANSCODE_MEDIA_TYPE
159+
mediaType = 'image/jpeg'
163160
metadata = await readMetadata(transcoded)
164161
}
165162
}
166163

167164
if (!metadata) {
168165
span.setAttribute(TraceAttr.CopilotVfsMetadataFailed, true)
169-
// HEIF that neither decoder could read is genuinely unreadable. Passing
170-
// those bytes through would hand the model a format it cannot decode,
171-
// which it then describes as empty rather than reporting as broken.
166+
// Bytes the model cannot decode are worse than no image: it describes
167+
// them as empty rather than reporting them as broken.
172168
const passthroughViable =
173-
!isHeifContainer(buffer) && buffer.length <= MAX_IMAGE_READ_BYTES
169+
MODEL_SUPPORTED_IMAGE_MIME_TYPES.has(mediaType) && buffer.length <= MAX_IMAGE_READ_BYTES
174170
span.setAttribute(
175171
TraceAttr.CopilotVfsOutcome,
176172
passthroughViable ? 'passthrough_no_metadata' : 'rejected_no_metadata'
@@ -185,11 +181,15 @@ async function prepareImageForVision(
185181
[TraceAttr.CopilotVfsInputHeight]: height,
186182
})
187183

188-
const needsResize =
184+
// A format the model cannot decode has to be re-encoded even when it is
185+
// already small enough — the ladder below emits JPEG or WebP, both of
186+
// which it accepts.
187+
const needsReencode =
188+
!MODEL_SUPPORTED_IMAGE_MIME_TYPES.has(mediaType) ||
189189
buffer.length > MAX_IMAGE_READ_BYTES ||
190190
width > MAX_IMAGE_DIMENSION ||
191191
height > MAX_IMAGE_DIMENSION
192-
if (!needsResize) {
192+
if (!needsReencode) {
193193
span.setAttributes({
194194
[TraceAttr.CopilotVfsResized]: false,
195195
[TraceAttr.CopilotVfsOutcome]: CopilotVfsOutcome.PassthroughFitsBudget,

apps/sim/lib/uploads/server/heic.test.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { isHeifContainer } from '@/lib/uploads/server/heic'
5+
import { isHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic'
66

77
/** An ISO-BMFF header: 4-byte box size, the `ftyp` marker, then the major brand. */
88
function ftypHeader(brand: string): Buffer {
@@ -37,11 +37,11 @@ describe('isHeifContainer', () => {
3737
)
3838
})
3939

40-
it('rejects a RIFF container whose brand offset would otherwise collide', () => {
41-
const webp = Buffer.alloc(16)
42-
webp.write('RIFF', 0, 'ascii')
43-
webp.write('WEBP', 8, 'ascii')
44-
expect(isHeifContainer(webp)).toBe(false)
40+
it('rejects a HEIF brand that is not behind an ftyp box', () => {
41+
const riff = Buffer.alloc(16)
42+
riff.write('RIFF', 0, 'ascii')
43+
riff.write('heic', 8, 'ascii')
44+
expect(isHeifContainer(riff)).toBe(false)
4545
})
4646

4747
it('rejects an unknown brand in a well-formed ftyp box', () => {
@@ -53,3 +53,11 @@ describe('isHeifContainer', () => {
5353
expect(isHeifContainer(ftypHeader('heic').subarray(0, 11))).toBe(false)
5454
})
5555
})
56+
57+
describe('transcodeHeicToJpeg', () => {
58+
it('returns null for bytes libheif cannot decode', async () => {
59+
// Also proves the dynamic `heic-convert` import resolves at runtime, which no
60+
// amount of type-checking establishes for a lazily loaded WebAssembly module.
61+
expect(await transcodeHeicToJpeg(ftypHeader('heic'))).toBeNull()
62+
})
63+
})

apps/sim/lib/uploads/server/heic.ts

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const logger = createLogger('HeicTranscode')
1010
* The list is deliberately broad, `avif` included. It answers "are these bytes
1111
* worth handing to a HEIF decoder", not "which codec is inside" — the brand cannot
1212
* answer the latter anyway, since `mif1` is generic and carries either HEVC or AV1.
13-
* Callers reach this only after a faster decoder has already failed.
1413
*/
1514
const HEIF_BRANDS = new Set([
1615
'heic',
@@ -25,11 +24,6 @@ const HEIF_BRANDS = new Set([
2524
'avis',
2625
])
2726

28-
/** JPEG quality for the transcode, on heic-convert's 0-1 scale. */
29-
const TRANSCODE_QUALITY = 0.92
30-
31-
export const HEIC_TRANSCODE_MEDIA_TYPE = 'image/jpeg'
32-
3327
/**
3428
* Whether these bytes are an ISO-BMFF container in the HEIF family.
3529
*
@@ -46,25 +40,25 @@ export function isHeifContainer(buffer: Buffer): boolean {
4640
/**
4741
* Transcode a HEVC-coded HEIF still to JPEG.
4842
*
49-
* Needed at two levels, neither of which has a workaround: no vision model accepts
50-
* HEIC (the Claude Messages API takes JPEG, PNG, GIF, and WebP only), and sharp's
51-
* prebuilt libvips ships libheif with AV1 support but not HEVC, so it decodes AVIF
52-
* and rejects an iPhone photo. `heic-convert` wraps a WebAssembly build of libheif,
53-
* which also keeps a historically CVE-prone parser inside the WASM sandbox rather
54-
* than in-process.
43+
* Two reasons, neither with a workaround: no vision model accepts HEIC (the Claude
44+
* Messages API takes JPEG, PNG, GIF, and WebP only), and sharp's prebuilt libvips
45+
* ships libheif with AV1 but not HEVC — it decodes AVIF and rejects an iPhone photo.
5546
*
56-
* Returns `null` when the bytes cannot be decoded — a corrupt or truncated upload
57-
* must degrade to "unreadable", never to a partial image the model would describe
58-
* with false confidence.
47+
* Returns `null` when the bytes cannot be decoded; never a partial image.
5948
*/
6049
export async function transcodeHeicToJpeg(buffer: Buffer): Promise<Buffer | null> {
6150
try {
6251
const convert = (await import('heic-convert')).default
63-
const jpeg = await convert({ buffer, format: 'JPEG', quality: TRANSCODE_QUALITY })
52+
const jpeg = await convert({ buffer, format: 'JPEG' })
53+
logger.info('Transcoded HEIC image', {
54+
inputBytes: buffer.length,
55+
outputBytes: jpeg.length,
56+
})
6457
return Buffer.from(jpeg)
6558
} catch (error) {
6659
logger.warn('Failed to transcode HEIC image', {
6760
bytes: buffer.length,
61+
brand: buffer.toString('ascii', 8, 12),
6862
error: getErrorMessage(error),
6963
})
7064
return null

0 commit comments

Comments
 (0)