Restore hydrated image attachment previews in the Agents window#327381
Open
ulugbekna wants to merge 1 commit into
Open
Restore hydrated image attachment previews in the Agents window#327381ulugbekna wants to merge 1 commit into
ulugbekna wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes hydrated image previews by rejecting URI values as image bytes and lazily reading referenced image files.
Changes:
- Prevent URI objects from being coerced into corrupt buffers.
- Load missing image bytes through
IFileService. - Add regression tests for URI-only and inline-byte attachments.
Show a summary per file
| File | Description |
|---|---|
chatImageExtraction.ts |
Excludes URIs from byte coercion. |
chatAttachmentWidgets.ts |
Lazily loads and rerenders image previews. |
chatImageExtraction.test.ts |
Tests URI-valued image extraction. |
chatAttachmentsContentPart.test.ts |
Tests resource-read behavior. |
Review details
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts:754
- This inline comment exceeds the repository's one-line limit. Condense it to the lifecycle constraint that is not evident from the cleanup code.
// 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).
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Medium
Comment on lines
+543
to
+545
| const renderImageElements = (buffer: Uint8Array) => { | ||
| imageElements.value = createImageElements(resource, attachment.name, fullName, this.element, buffer, attachment.id, this.hoverService, ariaLabel, currentLanguageModelName, clickHandler, this.currentLanguageModel, omittedState); | ||
| }; |
Comment on lines
+192
to
+194
| // A URI references image bytes rather than being the bytes themselves. Its own | ||
| // enumerable properties (scheme, path, ...) would otherwise be misread as a | ||
| // serialized byte record by the object branch below, yielding a bogus buffer. |
Comment on lines
+548
to
+550
| // 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. |
ulugbekna
force-pushed
the
ulugbekna/investigate-missing-images
branch
from
July 24, 2026 20:36
905a622 to
81cf65d
Compare
ulugbekna
changed the title
aeschli
approved these changes
Jul 24, 2026
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
ulugbekna
force-pushed
the
ulugbekna/investigate-missing-images
branch
from
July 24, 2026 20:38
81cf65d to
d60cae0
Compare
ulugbekna
added a commit
that referenced
this pull request
Jul 25, 2026
A uri-only image in a chat draft is not model-facing, so the draft path keeps it as a lightweight MessageAttachmentKind.Resource reference and must not read file bytes on every debounced draft sync (hydrateImageBytes defaults to false). Locks in that behavior, which was previously untested. Follow-up to #327381 for #327329. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c77566c8-d94f-4245-b965-75ea069af547
ulugbekna
added a commit
that referenced
this pull request
Jul 25, 2026
Uri-only image attachments (e.g. hydrated edit-and-resend images) reached the model as a Resource path reference instead of pixels, so the Agents window agent could not actually see them. In _toImageAttachment's no-inline-bytes branch, read the referenced file via IFileService and emit an EmbeddedResource (base64) for model-facing conversions (turn + pending), falling back to the existing Resource path reference on any read failure. Draft input-state sync stays a lightweight path reference. The attachment-conversion caller chain (_convertVariableToAttachment, _variableEntriesToAttachments, _appendActiveEditorAttachments, _convertVariablesToAttachments, _syncPendingMessages, _inputStateToDraft) is now async. Per-session pending-message syncs are serialized and coalesced through a ThrottlerByKey so the added hydration I/O cannot interleave concurrent runs and dispatch a stale same-id message. Helps Copilot and Codex; Claude still drops EmbeddedResource today (pre-existing, out of scope). Follow-up to #327381 for #327329. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c77566c8-d94f-4245-b965-75ea069af547
ulugbekna
added a commit
that referenced
this pull request
Jul 25, 2026
A uri-only image in a chat draft is not model-facing, so the draft path keeps it as a lightweight MessageAttachmentKind.Resource reference and must not read file bytes on every debounced draft sync (hydrateImageBytes defaults to false). Locks in that behavior, which was previously untested. Follow-up to #327381 for #327329. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c77566c8-d94f-4245-b965-75ea069af547
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #327329
Builds on #327300.
Problem
In the Agents window, image attachments on user messages lose their thumbnail preview after a window reload / session resume, falling back to a generic file‑media icon — even though the file still exists on disk.
Background
Hydrated (reload/resume/reconnect) image attachments arrive as uri‑only variable entries whose
valueis aURI(the real file reference is inreferences[0], with no inline bytes). #327300 fixed the sharedcoerceImageBufferhelper so it returnsundefinedfor such values instead of mis‑coercing the URI's enumerable keys into a bogus all‑zerosUint8Array.That stopped the corrupt buffer (and, on the edit‑and‑resend path, corrupt bytes being sent to the model), but the visible preview is still not restored:
ImageAttachmentWidgetreceivesundefinedbytes and never loads the real image, so it renders the generic icon.Fix
ImageAttachmentWidgetnow lazily reads the bytes via the injectedIFileServicewhen inline bytes are absent but aresourceis available (and the image isn't fully omitted / over the image limit), then re‑renders the preview with the actual image. The icon fallback is kept if the read fails (e.g. the file no longer exists). Works forfile:and remotevscode-agent-host:(filesystem provider registered) resources.Tests
chatAttachmentsContentPart.test.ts— a hydrated uri‑only image triggers a resource read and re‑render; an inline‑bytes image does not.Validation
npm run typecheck-client✔main(which now includes Fix hydrated image attachment previews #327300); this branch's earliercoerceImageBufferURI guard and its duplicate extraction test were dropped as redundant.