From d60cae097b3b341fc26de78dcfa2aaeca50bec28 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Fri, 24 Jul 2026 21:41:21 +0200 Subject: [PATCH 1/2] Restore hydrated image attachment previews in the Agents window Hydrated (reload/resume/reconnect) image attachments arrive as uri-only variable entries whose `value` is a URI, with the real file reference in `references[0]` and no inline bytes. Since #327300, `coerceImageBuffer` correctly returns undefined for such values instead of a bogus buffer, so `ImageAttachmentWidget` no longer renders garbage - but it still falls back to a generic file-media icon because nothing loads the real bytes. Make `ImageAttachmentWidget` lazily read the bytes via `IFileService` when inline bytes are absent but a resource is available (and the image is not fully omitted / over the image limit), then re-render the preview with the actual image. The icon fallback is kept if the read fails (e.g. the file no longer exists). Works for `file:` and remote `vscode-agent-host:` resources. Adds regression tests for the widget lazy byte-load (a hydrated uri-only image triggers a resource read; an inline-bytes image does not). Fixes #327329 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e04660de-4e7b-4432-99dd-24e0ffc35d1a --- .../attachments/chatAttachmentWidgets.ts | 36 ++++++++++++- .../chatAttachmentsContentPart.test.ts | 51 ++++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts index 0773c51da91acb..7925d52402bc74 100644 --- a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts +++ b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts @@ -538,7 +538,19 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget { const currentLanguageModelName = this.currentLanguageModel ? this.languageModelsService.lookupLanguageModel(this.currentLanguageModel.identifier)?.name ?? this.currentLanguageModel.identifier : 'Current model'; const fullName = resource ? this.labelService.getUriLabel(resource) : (attachment.fullName || attachment.name); - this._register(createImageElements(resource, attachment.name, fullName, this.element, imageData ?? (attachment.value as Uint8Array), attachment.id, this.hoverService, ariaLabel, currentLanguageModelName, clickHandler, this.currentLanguageModel, omittedState)); + + const imageElements = this._register(new MutableDisposable()); + const renderImageElements = (buffer: Uint8Array) => { + imageElements.value = createImageElements(resource, attachment.name, fullName, this.element, buffer, attachment.id, this.hoverService, ariaLabel, currentLanguageModelName, clickHandler, this.currentLanguageModel, omittedState); + }; + renderImageElements(imageData ?? new Uint8Array()); + + // Hydrated image attachments (e.g. after a window reload or session resume) arrive as a + // resource reference without inline bytes. Load the bytes from disk so the preview renders + // the actual image instead of falling back to a generic file icon. + if (!imageData && resource && omittedState !== OmittedState.Full && omittedState !== OmittedState.ImageLimitExceeded) { + void this.loadImageBytes(resource, renderImageElements); + } this.attachSaveButton(resource, imageData, attachment.name, options.supportsDeletion); this.element.ariaLabel = this.appendDeletionHint(ariaLabel); @@ -558,6 +570,20 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget { } } + private async loadImageBytes(resource: URI, render: (buffer: Uint8Array) => void): Promise { + let content: VSBuffer; + try { + content = (await this.fileService.readFile(resource)).value; + } catch { + // The file may no longer exist; keep the icon fallback that is already rendered. + return; + } + if (this._store.isDisposed) { + return; + } + render(content.buffer); + } + private async openInCarousel(id: string, name: string, data: Uint8Array | undefined, referenceUri: URI | undefined, preferCurrentInput: boolean | undefined): Promise { const resource = referenceUri ?? URI.from({ scheme: 'data', path: `${id}/${encodeURIComponent(name)}` }); await this.chatImageCarouselService.openCarouselAtResource(resource, data, { preferCurrentInput }); @@ -723,6 +749,14 @@ function createImageElements(resource: URI | undefined, name: string, fullName: hoverElement.appendChild(dom.$('div', undefined, localize('chat.autoImageAttachmentHover', "Image support depends on the model selected by Auto."))); } } + + // Remove the pill and label from the DOM when disposed so the widget can safely re-render + // (e.g. after lazily loading the image bytes for a hydrated attachment). + disposable.add(toDisposable(() => { + currentPill.remove(); + textLabel.remove(); + })); + return disposable; } diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts index fdaa407bed504d..2e3836d97ffd22 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts @@ -9,7 +9,8 @@ import { mainWindow } from '../../../../../../../base/browser/window.js'; import { URI } from '../../../../../../../base/common/uri.js'; import { ExtensionIdentifier } from '../../../../../../../platform/extensions/common/extensions.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js'; -import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; +import { IFileService } from '../../../../../../../platform/files/common/files.js'; +import { TestFileService, workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; import { getEffectiveImageOmittedState } from '../../../../browser/attachments/chatAttachmentWidgets.js'; import { ChatAttachmentsContentPart } from '../../../../browser/widget/chatContentParts/chatAttachmentsContentPart.js'; import { AgentHostCompletionReferenceKind, IChatRequestVariableEntry, OmittedState, toAgentHostCompletionVariableEntry } from '../../../../common/attachments/chatVariableEntries.js'; @@ -347,5 +348,53 @@ suite('ChatAttachmentsContentPart', () => { assert.strictEqual(getEffectiveImageOmittedState(OmittedState.Full, autoModel, true), OmittedState.NotOmitted); }); + + suite('hydrated image attachments', () => { + async function renderImageAndCollectReads(image: IChatRequestVariableEntry): Promise { + const fileService = instantiationService.get(IFileService) as TestFileService; + const part = store.add(instantiationService.createInstance( + ChatAttachmentsContentPart, + { variables: [image] } + )); + + mainWindow.document.body.appendChild(part.domNode!); + disposables.add(toDisposable(() => part.domNode?.remove())); + + // Let the widget's lazy byte load (a microtask) settle before inspecting reads. + await new Promise(resolve => setTimeout(resolve, 0)); + + return fileService.readOperations.map(read => read.resource.toString()); + } + + test('should load bytes from the resource for a hydrated (uri-only) image', async () => { + const resource = URI.file('/test/pasted-image.png'); + const reads = await renderImageAndCollectReads({ + kind: 'image', + id: 'hydrated-image', + name: 'pasted-image.png', + value: resource, + mimeType: 'image/png', + isURL: true, + references: [{ kind: 'reference', reference: resource }] + }); + + assert.deepStrictEqual(reads, [resource.toString()]); + }); + + test('should not read the resource for an image with inline bytes', async () => { + const resource = URI.file('/test/inline-image.png'); + const reads = await renderImageAndCollectReads({ + kind: 'image', + id: 'inline-image', + name: 'inline-image.png', + value: new Uint8Array([0x89, 0x50, 0x4E, 0x47]), + mimeType: 'image/png', + isURL: false, + references: [{ kind: 'reference', reference: resource }] + }); + + assert.deepStrictEqual(reads, []); + }); + }); }); }); From a6a45dec2fd24340c3300b551830f8c3cb4221fe Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Sun, 26 Jul 2026 15:37:09 +0200 Subject: [PATCH 2/2] Restore delete ARIA hint after hydrated image re-render Address PR #327381 review feedback: - Re-apply the deletion-hint aria-label inside renderImageElements so it survives the lazy byte load for a hydrated image attachment (it was clobbered when createImageElements reset the label on re-render). - Collapse two over-long inline comments to concise one-liners. - Add a regression test asserting the delete hint persists after the hydrated bytes load. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e04660de-4e7b-4432-99dd-24e0ffc35d1a --- .../attachments/chatAttachmentWidgets.ts | 10 +++--- .../chatAttachmentsContentPart.test.ts | 32 ++++++++++++++++++- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts index 7925d52402bc74..e97d9c5b1a8b84 100644 --- a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts +++ b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts @@ -542,17 +542,16 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget { const imageElements = this._register(new MutableDisposable()); const renderImageElements = (buffer: Uint8Array) => { imageElements.value = createImageElements(resource, attachment.name, fullName, this.element, buffer, attachment.id, this.hoverService, ariaLabel, currentLanguageModelName, clickHandler, this.currentLanguageModel, omittedState); + // createImageElements resets the label; restore the deletion hint after each render. + this.element.ariaLabel = this.appendDeletionHint(ariaLabel); }; renderImageElements(imageData ?? new Uint8Array()); - // Hydrated image attachments (e.g. after a window reload or session resume) arrive as a - // resource reference without inline bytes. Load the bytes from disk so the preview renders - // the actual image instead of falling back to a generic file icon. + // Hydrated attachments need disk bytes so the preview does not fall back to a generic file icon. if (!imageData && resource && omittedState !== OmittedState.Full && omittedState !== OmittedState.ImageLimitExceeded) { void this.loadImageBytes(resource, renderImageElements); } this.attachSaveButton(resource, imageData, attachment.name, options.supportsDeletion); - this.element.ariaLabel = this.appendDeletionHint(ariaLabel); // Wire up click + keyboard (Enter/Space) open handlers const canOpenCarousel = !!imageData && configurationService.getValue(ChatConfiguration.ImageCarouselEnabled); @@ -750,8 +749,7 @@ function createImageElements(resource: URI | undefined, name: string, fullName: } } - // Remove the pill and label from the DOM when disposed so the widget can safely re-render - // (e.g. after lazily loading the image bytes for a hydrated attachment). + // Remove old DOM so the widget can safely re-render after hydrated bytes load. disposable.add(toDisposable(() => { currentPill.remove(); textLabel.remove(); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts index 2e3836d97ffd22..15b98f1d43ab63 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatAttachmentsContentPart.test.ts @@ -11,7 +11,8 @@ import { ExtensionIdentifier } from '../../../../../../../platform/extensions/co import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js'; import { IFileService } from '../../../../../../../platform/files/common/files.js'; import { TestFileService, workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; -import { getEffectiveImageOmittedState } from '../../../../browser/attachments/chatAttachmentWidgets.js'; +import { DEFAULT_LABELS_CONTAINER, ResourceLabels } from '../../../../../../browser/labels.js'; +import { getEffectiveImageOmittedState, ImageAttachmentWidget } from '../../../../browser/attachments/chatAttachmentWidgets.js'; import { ChatAttachmentsContentPart } from '../../../../browser/widget/chatContentParts/chatAttachmentsContentPart.js'; import { AgentHostCompletionReferenceKind, IChatRequestVariableEntry, OmittedState, toAgentHostCompletionVariableEntry } from '../../../../common/attachments/chatVariableEntries.js'; import { ILanguageModelChatMetadata, ILanguageModelsService } from '../../../../common/languageModels.js'; @@ -395,6 +396,35 @@ suite('ChatAttachmentsContentPart', () => { assert.deepStrictEqual(reads, []); }); + + test('should keep delete hint after loading hydrated image bytes', async () => { + const resource = URI.file('/test/pasted-image.png'); + const container = mainWindow.document.createElement('div'); + mainWindow.document.body.appendChild(container); + disposables.add(toDisposable(() => container.remove())); + const contextResourceLabels = disposables.add(instantiationService.createInstance(ResourceLabels, DEFAULT_LABELS_CONTAINER)); + const widget = disposables.add(instantiationService.createInstance( + ImageAttachmentWidget, + resource, + { + kind: 'image', + id: 'hydrated-image-with-delete', + name: 'pasted-image.png', + value: resource, + mimeType: 'image/png', + isURL: true, + references: [{ kind: 'reference', reference: resource }] + }, + undefined, + { shouldFocusClearButton: false, supportsDeletion: true }, + container, + contextResourceLabels + )); + + await new Promise(resolve => setTimeout(resolve, 0)); + + assert.strictEqual(widget.element.ariaLabel, 'Attached image, pasted-image.png (Delete)'); + }); }); }); });