fix(desktop): keep full-res previewUrl separate from downscaled thumbnail
Addresses review feedback on #68744: downscaling inside
attachmentPreviewDataUrl made previewUrl the 2048px PNG, but current main
(d8cb73b4ab) feeds that field to ImageLightbox and useImageDownload, so
large attachments would open and download at reduced resolution.
- attachmentPreviewDataUrl returns the full-resolution data URL again
- ComposerAttachment gains thumbnailUrl?: string (store/composer.ts)
- attachImagePath stores previewUrl (full-res) + thumbnailUrl (downscaled)
- Attachment pill renders thumbnailUrl ?? previewUrl; lightbox/download
keep the full-res previewUrl
- optimisticAttachmentRef prefers thumbnailUrl for the in-flight bubble
display ref (same main-thread decode freeze at send time)
- Attachment-level regression test in use-composer-actions.test.ts:
full-res previewUrl preserved while thumbnailUrl is a separate
downscaled value (4000x3000 -> 2048x1536)
This commit is contained in:
@@ -127,7 +127,7 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme
|
||||
alt={attachment.label}
|
||||
className="size-full object-cover"
|
||||
draggable={false}
|
||||
src={attachment.previewUrl}
|
||||
src={attachment.thumbnailUrl ?? attachment.previewUrl}
|
||||
/>
|
||||
) : (
|
||||
<Icon className="size-3.5" />
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ComposerAttachment } from '@/store/composer'
|
||||
import { $composerAttachments, type ComposerAttachment } from '@/store/composer'
|
||||
import { $connection } from '@/store/session'
|
||||
|
||||
import {
|
||||
@@ -319,3 +319,86 @@ describe('useComposerActions native image drops', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('attachImagePath thumbnail separation', () => {
|
||||
// Full-resolution data URL the local bridge returns for a pasted screenshot.
|
||||
// Content is irrelevant — the mock bitmap below reports 4000×3000 so the
|
||||
// real downscale path runs (the data URL itself is never decoded in jsdom).
|
||||
const FULL_RES = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+GkZcAAAAASUVORK5CYII='
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
|
||||
$composerAttachments.set([])
|
||||
})
|
||||
|
||||
it('keeps the full-resolution previewUrl and stores a separate downscaled thumbnailUrl', async () => {
|
||||
const readFileDataUrl = vi.fn(async () => FULL_RES)
|
||||
|
||||
;(window as unknown as { hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } }).hermesDesktop = {
|
||||
readFileDataUrl
|
||||
}
|
||||
|
||||
// Exercise the real downscale path: 4000×3000 bitmap → 2048×1536 canvas.
|
||||
const drawImage = vi.fn()
|
||||
const close = vi.fn()
|
||||
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn(async () => ({
|
||||
blob: async () => new Blob([new Uint8Array([0])], { type: 'image/png' })
|
||||
}))
|
||||
)
|
||||
vi.stubGlobal('createImageBitmap', vi.fn(async () => ({ width: 4000, height: 3000, close })))
|
||||
|
||||
class MockOffscreenCanvas {
|
||||
getContext = () => ({ drawImage })
|
||||
convertToBlob = vi.fn(async () => new Blob(['x'], { type: 'image/png' }))
|
||||
constructor(_width: number, _height: number) {}
|
||||
}
|
||||
|
||||
vi.stubGlobal('OffscreenCanvas', MockOffscreenCanvas)
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() })
|
||||
)
|
||||
|
||||
await act(async () => {
|
||||
await result.current.attachImagePath('/tmp/shot.png')
|
||||
})
|
||||
|
||||
const attachment = $composerAttachments.get()[0]
|
||||
|
||||
expect(attachment).toBeDefined()
|
||||
// Full-resolution bytes stay on `previewUrl` — the same field main feeds
|
||||
// to ImageLightbox and useImageDownload, so open/download keep full res.
|
||||
expect(attachment?.previewUrl).toBe(FULL_RES)
|
||||
// The pill thumbnail is a SEPARATE downscaled value, never the multi-MB
|
||||
// original (which would re-introduce the main-thread decode freeze).
|
||||
expect(attachment?.thumbnailUrl).toBeDefined()
|
||||
expect(attachment?.thumbnailUrl).not.toBe(FULL_RES)
|
||||
expect(drawImage).toHaveBeenCalledWith(expect.anything(), 0, 0, 2048, 1536)
|
||||
expect(close).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('leaves the thumbnail undefined when the preview is not an image', async () => {
|
||||
const readFileDataUrl = vi.fn(async () => 'data:text/plain;base64,aGVsbG8=')
|
||||
|
||||
;(window as unknown as { hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } }).hermesDesktop = {
|
||||
readFileDataUrl
|
||||
}
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() })
|
||||
)
|
||||
|
||||
await act(async () => {
|
||||
await result.current.attachImagePath('/tmp/shot.txt')
|
||||
})
|
||||
|
||||
const attachment = $composerAttachments.get()[0]
|
||||
|
||||
expect(attachment?.previewUrl).toBe('data:text/plain;base64,aGVsbG8=')
|
||||
expect(attachment?.thumbnailUrl).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -68,13 +68,6 @@ export async function attachmentPreviewDataUrl(filePath: string): Promise<string
|
||||
dataUrl = await readDesktopFileDataUrl(filePath)
|
||||
}
|
||||
|
||||
// Downscale large images for preview to prevent main-thread decode freezes.
|
||||
// macOS ImageIO/vImage blocks the Chromium renderer on Retina screenshots
|
||||
// (6000×4000+, >5 MB). The full-resolution bytes are still on disk for the model.
|
||||
if (dataUrl.startsWith('data:image/')) {
|
||||
return downscaleDataUrlForPreview(dataUrl)
|
||||
}
|
||||
|
||||
return dataUrl
|
||||
}
|
||||
|
||||
@@ -483,7 +476,17 @@ export function useComposerActions({
|
||||
const previewUrl = await attachmentPreviewDataUrl(filePath)
|
||||
|
||||
if (previewUrl) {
|
||||
scope.add({ ...baseAttachment, previewUrl })
|
||||
// Downscale only the pill thumbnail. `previewUrl` must keep the
|
||||
// full-resolution bytes: current main feeds it to ImageLightbox and
|
||||
// useImageDownload (attachments.tsx), and the attached-image
|
||||
// pipeline uploads the on-disk original to the model. The helper
|
||||
// never rejects — on failure it returns a 1×1 placeholder so the
|
||||
// pill never renders the multi-MB original.
|
||||
const thumbnailUrl = previewUrl.startsWith('data:image/')
|
||||
? await downscaleDataUrlForPreview(previewUrl)
|
||||
: undefined
|
||||
|
||||
scope.add({ ...baseAttachment, previewUrl, thumbnailUrl })
|
||||
}
|
||||
|
||||
return true
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
} from './chat-runtime'
|
||||
|
||||
const DATA_URL = 'data:image/png;base64,iVBORw0KGgoAAAANS'
|
||||
const THUMB_URL = 'data:image/png;base64,dGh1bWI='
|
||||
|
||||
function attachment(overrides: Partial<ComposerAttachment> & Pick<ComposerAttachment, 'kind'>): ComposerAttachment {
|
||||
return { id: 'a', label: 'file.png', ...overrides }
|
||||
@@ -27,6 +28,16 @@ describe('optimisticAttachmentRef', () => {
|
||||
expect(ref).toBe(DATA_URL)
|
||||
})
|
||||
|
||||
it('prefers the downscaled thumbnail for the display ref when present', () => {
|
||||
const ref = optimisticAttachmentRef(
|
||||
attachment({ kind: 'image', detail: '/tmp/shot.png', previewUrl: DATA_URL, thumbnailUrl: THUMB_URL })
|
||||
)
|
||||
|
||||
// The bubble is a display-only thumbnail; full-res previewUrl stays for
|
||||
// lightbox/download and the model gets bytes via the upload pipeline.
|
||||
expect(ref).toBe(THUMB_URL)
|
||||
})
|
||||
|
||||
it('falls back to an @image: path ref when no preview is available', () => {
|
||||
expect(optimisticAttachmentRef(attachment({ kind: 'image', detail: '/tmp/shot.png' }))).toBe('@image:/tmp/shot.png')
|
||||
})
|
||||
|
||||
@@ -259,7 +259,10 @@ export function optimisticAttachmentRef(attachment: ComposerAttachment): string
|
||||
}
|
||||
|
||||
if (attachment.kind === 'image' && attachment.previewUrl?.startsWith('data:')) {
|
||||
return attachment.previewUrl
|
||||
// The pill and the in-flight bubble render the downscaled thumbnail; the
|
||||
// full-resolution `previewUrl` stays for lightbox/download, and the model
|
||||
// receives bytes via the attached-image upload pipeline, not this ref.
|
||||
return attachment.thumbnailUrl ?? attachment.previewUrl
|
||||
}
|
||||
|
||||
return attachmentDisplayText(attachment)
|
||||
|
||||
@@ -10,6 +10,9 @@ export interface ComposerAttachment {
|
||||
detail?: string
|
||||
refText?: string
|
||||
previewUrl?: string
|
||||
/** Downscaled data URL for the attachment card's <img> pill only. Keeps the
|
||||
* full-resolution `previewUrl` for lightbox/download/model bytes. */
|
||||
thumbnailUrl?: string
|
||||
path?: string
|
||||
attachedSessionId?: string
|
||||
/** Set while the file/image bytes are being staged into the session
|
||||
|
||||
Reference in New Issue
Block a user