From 9711aeb18124e538ad50dbc19ec2bbce48096937 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:46:40 +0200 Subject: [PATCH] Group session artifact images in carousel (#331946) * Group session artifact images in carousel Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Honor imageCarousel.chat.enabled for artifact images Co-authored-by: benibenj <44439583+benibenj@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- .../contrib/chat/browser/sessionArtifacts.ts | 77 ++++++++++++++++++- .../test/browser/sessionArtifacts.test.ts | 64 ++++++++++++++- 2 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts b/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts index 4e9d089a8f5..d56a7b4309f 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts @@ -6,20 +6,28 @@ import { Codicon } from '../../../../base/common/codicons.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; +import { getMediaMime } from '../../../../base/common/mime.js'; import { derived, IObservable, IReader } from '../../../../base/common/observable.js'; import { basename, getComparisonKey } from '../../../../base/common/resources.js'; import { ThemeIcon } from '../../../../base/common/themables.js'; import { URI } from '../../../../base/common/uri.js'; +import { generateUuid } from '../../../../base/common/uuid.js'; import { localize } from '../../../../nls.js'; import { toAction } from '../../../../base/common/actions.js'; import { IClipboardService } from '../../../../platform/clipboard/common/clipboardService.js'; +import { ICommandService } from '../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { observableConfigValue } from '../../../../platform/observable/common/platformObservableUtils.js'; import { IOpenerService } from '../../../../platform/opener/common/opener.js'; import type { IChatPillEntry, IChatPillSection } from '../../../../workbench/browser/chatPills.js'; import { openChatTurnFile, previewKind } from '../../../../workbench/contrib/chat/browser/widget/chatTurnPills.js'; +import { ChatConfiguration } from '../../../../workbench/contrib/chat/common/constants.js'; +import type { IImageCarouselCollection } from '../../../../workbench/contrib/imageCarousel/browser/imageCarouselTypes.js'; import { SessionArtifactKind, SessionFileOperation, type ISessionArtifact, type ISessionFile } from '../../../services/sessions/common/session.js'; import type { IActiveSession } from '../../../services/sessions/common/sessionsManagement.js'; +const OPEN_IMAGE_CAROUSEL_COMMAND_ID = 'workbench.action.chat.openImageInCarousel'; + const artifactIcons: ReadonlyMap = new Map([ [SessionArtifactKind.PullRequest, Codicon.gitPullRequest], [SessionArtifactKind.Issue, Codicon.issues], @@ -42,9 +50,15 @@ const sectionOrder: readonly { readonly kind: SessionArtifactKind; readonly titl export interface ISessionArtifactActions { openExternal(link: URI): void; openResource(uri: URI): void; + openImages(images: readonly ISessionArtifactImage[], startIndex: number): void; copy(text: string): void; } +export interface ISessionArtifactImage { + readonly uri: URI; + readonly mimeType: string; +} + function artifactValueKey(artifact: ISessionArtifact): string { if (artifact.uri) { return getComparisonKey(artifact.uri); @@ -62,6 +76,11 @@ function artifactLocation(uri: URI, label: string): Pick(); + const images: ISessionArtifactImage[] = []; const seen = new Set(); for (const artifact of artifacts) { + const imageMimeType = artifact.uri ? getImageMimeType(artifact.uri) : undefined; + if (artifact.kind === SessionArtifactKind.File && artifact.uri && imageMimeType) { + if (!seen.has(artifactValueKey(artifact))) { + seen.add(artifactValueKey(artifact)); + images.push({ uri: artifact.uri, mimeType: imageMimeType }); + } + continue; + } const entry = toEntry(artifact, actions); if (!entry || seen.has(artifactValueKey(artifact))) { continue; @@ -125,10 +153,15 @@ export function buildSessionArtifactSections(artifacts: readonly ISessionArtifac } for (const file of externalFiles) { - if (file.operation === SessionFileOperation.Deleted || !previewKind(file.uri) || seen.has(getComparisonKey(file.uri))) { + const imageMimeType = getImageMimeType(file.uri); + if (file.operation === SessionFileOperation.Deleted || (!previewKind(file.uri) && !imageMimeType) || seen.has(getComparisonKey(file.uri))) { continue; } seen.add(getComparisonKey(file.uri)); + if (imageMimeType) { + images.push({ uri: file.uri, mimeType: imageMimeType }); + continue; + } const entries = entriesByKind.get(SessionArtifactKind.File) ?? []; const label = basename(file.uri); entries.push({ id: file.uri.toString(), label, resource: file.uri, ...artifactLocation(file.uri, label), open: () => actions.openResource(file.uri) }); @@ -137,6 +170,26 @@ export function buildSessionArtifactSections(artifacts: readonly ISessionArtifac const sections: IChatPillSection[] = []; for (const { kind, title } of sectionOrder) { + if (kind === SessionArtifactKind.File && images.length) { + sections.push({ + title: localize('sessionArtifacts.images', "Images"), + entries: images.map(({ uri }, index) => { + const label = basename(uri); + return { + id: uri.toString(), + label, + resource: uri, + ...artifactLocation(uri, label), + ...(imageCarouselEnabled + ? { + ariaLabel: localize('sessionArtifacts.openImage', "Open {0} in Images Preview", label), + open: () => actions.openImages(images, index), + } + : { open: () => actions.openResource(uri) }), + }; + }), + }); + } const entries = entriesByKind.get(kind); if (entries?.length) { sections.push({ title, entries }); @@ -153,11 +206,14 @@ export class SessionArtifacts extends Disposable { constructor( session: IObservable, @IClipboardService private readonly _clipboardService: IClipboardService, + @ICommandService private readonly _commandService: ICommandService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IOpenerService private readonly _openerService: IOpenerService, ) { super(); + const imageCarouselEnabled = observableConfigValue(ChatConfiguration.ImageCarouselEnabled, true, this._configurationService); + this.sections = derived(this, reader => { const current = session.read(reader); if (!current) { @@ -167,6 +223,7 @@ export class SessionArtifacts extends Disposable { current.artifacts?.read(reader) ?? [], this._readExternalFiles(current, reader), this._actions(), + imageCarouselEnabled.read(reader), ); }); } @@ -185,6 +242,22 @@ export class SessionArtifacts extends Disposable { } void this._openerService.open(uri, { fromUserGesture: true }); }, + openImages: (images, startIndex) => { + const collection: IImageCarouselCollection = { + id: generateUuid(), + title: localize('sessionArtifacts.imageCarouselTitle', "Artifact Images"), + sections: [{ + title: localize('sessionArtifacts.images', "Images"), + images: images.map(image => ({ + id: image.uri.toString(), + name: basename(image.uri), + mimeType: image.mimeType, + uri: image.uri, + })), + }], + }; + void this._commandService.executeCommand(OPEN_IMAGE_CAROUSEL_COMMAND_ID, { collection, startIndex }); + }, copy: text => { void this._clipboardService.writeText(text); }, }; } diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts index cdda336a07b..0472982acac 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts @@ -7,7 +7,7 @@ import assert from 'assert'; import { isMarkdownString } from '../../../../../base/common/htmlContent.js'; import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { buildSessionArtifactSections, type ISessionArtifactActions } from '../../browser/sessionArtifacts.js'; +import { buildSessionArtifactSections, type ISessionArtifactActions, type ISessionArtifactImage } from '../../browser/sessionArtifacts.js'; import { type ISessionArtifact, SessionArtifactKind, SessionFileOperation } from '../../../../services/sessions/common/session.js'; suite('Session Artifacts', () => { @@ -16,6 +16,7 @@ suite('Session Artifacts', () => { const actions: ISessionArtifactActions = { openExternal() { }, openResource() { }, + openImages() { }, copy() { }, }; @@ -30,7 +31,7 @@ suite('Session Artifacts', () => { { id: 'resource', kind: SessionArtifactKind.Resource, label: 'Resource', uri: resourceUri }, ]; - const entries = buildSessionArtifactSections(artifacts, [{ uri: externalFileUri, operation: SessionFileOperation.Created }], actions).flatMap(section => section.entries); + const entries = buildSessionArtifactSections(artifacts, [{ uri: externalFileUri, operation: SessionFileOperation.Created }], actions, true).flatMap(section => section.entries); assert.deepStrictEqual(entries.map(entry => { const content = entry.hover?.content; return { @@ -47,4 +48,63 @@ suite('Session Artifacts', () => { { label: 'Resource', ariaLabel: 'Open Resource', ariaDescription: resourceUri.toString(true), hover: resourceUri.toString(true), tooltip: resourceUri.toString(true) }, ]); }); + + test('groups artifact images separately and opens all images in the carousel', () => { + const screenshotUri = URI.file('/artifacts/screenshot.png'); + const diagramUri = URI.file('/external/diagram.jpg'); + const reportUri = URI.file('/artifacts/report.md'); + const opened: { images: readonly ISessionArtifactImage[]; startIndex: number }[] = []; + const imageActions: ISessionArtifactActions = { + ...actions, + openImages: (images, startIndex) => opened.push({ images, startIndex }), + }; + const artifacts: readonly ISessionArtifact[] = [ + { id: 'screenshot', kind: SessionArtifactKind.File, label: 'Screenshot', uri: screenshotUri }, + { id: 'report', kind: SessionArtifactKind.File, label: 'Report', uri: reportUri }, + ]; + + const sections = buildSessionArtifactSections(artifacts, [ + { uri: diagramUri, operation: SessionFileOperation.Created }, + ], imageActions, true); + const imageSection = sections.find(section => section.title === 'Images'); + assert.ok(imageSection); + imageSection.entries[1].open(); + + assert.deepStrictEqual({ + sections: sections.map(section => ({ title: section.title, labels: section.entries.map(entry => entry.label) })), + opened: opened.map(entry => ({ images: entry.images.map(image => image.uri.path), startIndex: entry.startIndex })), + }, { + sections: [ + { title: 'Images', labels: ['screenshot.png', 'diagram.jpg'] }, + { title: 'Files', labels: ['report.md'] }, + ], + opened: [{ images: ['/artifacts/screenshot.png', '/external/diagram.jpg'], startIndex: 1 }], + }); + }); + + test('opens the image resource when the image carousel is disabled', () => { + const screenshotUri = URI.file('/artifacts/screenshot.png'); + const opened: string[] = []; + const imageActions: ISessionArtifactActions = { + ...actions, + openImages: () => opened.push('carousel'), + openResource: uri => opened.push(uri.path), + }; + const artifacts: readonly ISessionArtifact[] = [ + { id: 'screenshot', kind: SessionArtifactKind.File, label: 'Screenshot', uri: screenshotUri }, + ]; + + const sections = buildSessionArtifactSections(artifacts, [], imageActions, false); + const imageSection = sections.find(section => section.title === 'Images'); + assert.ok(imageSection); + imageSection.entries[0].open(); + + assert.deepStrictEqual({ + ariaLabel: imageSection.entries[0].ariaLabel, + opened, + }, { + ariaLabel: 'Open screenshot.png', + opened: ['/artifacts/screenshot.png'], + }); + }); });