Skip to content

Commit

Permalink
warnings in chat attachments when attachments are not valid (#240600)
Browse files Browse the repository at this point in the history 
* some attachment warnings

* move await to before so we don't check so many times

* some cleanup

* fix merge conflicts
  • Loading branch information
@justschen
justschen authored Feb 13, 2025
1 parent 95cad8e commit 984a651
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 68 deletions.
21 changes: 16 additions & 5 deletions src/vs/workbench/contrib/chat/browser/actions/chatContextActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { URI } from '../../../../../base/common/uri.js';
import { ServicesAccessor } from '../../../../../editor/browser/editorExtensions.js';
import { IRange, Range } from '../../../../../editor/common/core/range.js';
import { Command } from '../../../../../editor/common/languages.js';
import { ITextModelService } from '../../../../../editor/common/services/resolverService.js';
import { AbstractGotoSymbolQuickAccessProvider, IGotoSymbolQuickPickItem } from '../../../../../editor/contrib/quickAccess/browser/gotoSymbolQuickAccess.js';
import { localize, localize2 } from '../../../../../nls.js';
import { Action2, IAction2Options, MenuId, registerAction2 } from '../../../../../platform/actions/common/actions.js';
Expand Down Expand Up @@ -455,7 +456,7 @@ export class AttachContextAction extends Action2 {
`:${item.range.startLineNumber}`);
}

private async _attachContext(widget: IChatWidget, quickInputService: IQuickInputService, commandService: ICommandService, clipboardService: IClipboardService, editorService: IEditorService, labelService: ILabelService, viewsService: IViewsService, chatEditingService: IChatEditingService | undefined, hostService: IHostService, fileService: IFileService, isInBackground?: boolean, ...picks: IChatContextQuickPickItem[]) {
private async _attachContext(widget: IChatWidget, quickInputService: IQuickInputService, commandService: ICommandService, clipboardService: IClipboardService, editorService: IEditorService, labelService: ILabelService, viewsService: IViewsService, chatEditingService: IChatEditingService | undefined, hostService: IHostService, fileService: IFileService, textModelService: ITextModelService, isInBackground?: boolean, ...picks: IChatContextQuickPickItem[]) {
const toAttach: IChatRequestVariableEntry[] = [];
for (const pick of picks) {
if (isISymbolQuickPickItem(pick) && pick.symbol) {
Expand Down Expand Up @@ -488,11 +489,20 @@ export class AttachContextAction extends Action2 {
if (chatEditingService) {
getEditingSession(chatEditingService, widget)?.addFileToWorkingSet(pick.resource);
} else {
let isOmitted = false;
try {
const createdModel = await textModelService.createModelReference(pick.resource);
createdModel.dispose();
} catch {
isOmitted = true;
}

toAttach.push({
id: this._getFileContextId({ resource: pick.resource }),
value: pick.resource,
name: pick.label,
isFile: true,
isOmitted
});
}
}
Expand Down Expand Up @@ -634,6 +644,7 @@ export class AttachContextAction extends Action2 {
const hostService = accessor.get(IHostService);
const extensionService = accessor.get(IExtensionService);
const fileService = accessor.get(IFileService);
const textModelService = accessor.get(ITextModelService);

const context: { widget?: IChatWidget; showFilesOnly?: boolean; placeholder?: string } | undefined = args[0];
const widget = context?.widget ?? widgetService.lastFocusedWidget;
Expand Down Expand Up @@ -782,19 +793,19 @@ export class AttachContextAction extends Action2 {
const second = extractTextFromIconLabel(b.label).toUpperCase();

return compare(first, second);
}), clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, '', context?.placeholder);
}), clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, textModelService, '', context?.placeholder);
}

private _show(quickInputService: IQuickInputService, commandService: ICommandService, widget: IChatWidget, quickChatService: IQuickChatService, quickPickItems: (IChatContextQuickPickItem | QuickPickItem)[] | undefined, clipboardService: IClipboardService, editorService: IEditorService, labelService: ILabelService, viewsService: IViewsService, chatEditingService: IChatEditingService | undefined, hostService: IHostService, fileService: IFileService, query: string = '', placeholder?: string) {
private _show(quickInputService: IQuickInputService, commandService: ICommandService, widget: IChatWidget, quickChatService: IQuickChatService, quickPickItems: (IChatContextQuickPickItem | QuickPickItem)[] | undefined, clipboardService: IClipboardService, editorService: IEditorService, labelService: ILabelService, viewsService: IViewsService, chatEditingService: IChatEditingService | undefined, hostService: IHostService, fileService: IFileService, textModelService: ITextModelService, query: string = '', placeholder?: string) {
const providerOptions: AnythingQuickAccessProviderRunOptions = {
handleAccept: (item: IChatContextQuickPickItem, isBackgroundAccept: boolean) => {
if ('prefix' in item) {
this._show(quickInputService, commandService, widget, quickChatService, quickPickItems, clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, item.prefix, placeholder);
this._show(quickInputService, commandService, widget, quickChatService, quickPickItems, clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, textModelService, item.prefix, placeholder);
} else {
if (!clipboardService) {
return;
}
this._attachContext(widget, quickInputService, commandService, clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, isBackgroundAccept, item);
this._attachContext(widget, quickInputService, commandService, clipboardService, editorService, labelService, viewsService, chatEditingService, hostService, fileService, textModelService, isBackgroundAccept, item);
if (isQuickChat(widget)) {
quickChatService.open();
}
Expand Down
67 changes: 40 additions & 27 deletions src/vs/workbench/contrib/chat/browser/chatContentParts/chatAttachmentsContentPart.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import * as dom from '../../../../../base/browser/dom.js';
import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js';
import { IManagedHoverTooltipMarkdownString } from '../../../../../base/browser/ui/hover/hover.js';
import { IHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegate.js';
import { createInstantHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegateFactory.js';
import { Promises } from '../../../../../base/common/async.js';
import { Codicon } from '../../../../../base/common/codicons.js';
Expand Down Expand Up @@ -64,7 +65,7 @@ export class ChatAttachmentsContentPart extends Disposable {
@IFileService private readonly fileService: IFileService,
@ICommandService private readonly commandService: ICommandService,
@IThemeService private readonly themeService: IThemeService,
@ILabelService private readonly labelService: ILabelService
@ILabelService private readonly labelService: ILabelService,
) {
super();

Expand Down Expand Up @@ -92,7 +93,7 @@ export class ChatAttachmentsContentPart extends Disposable {
const label = this._contextResourceLabels.create(widget, { supportIcons: true, hoverDelegate, hoverTargetOverride: widget });
this.attachedContextDisposables.add(label);

const correspondingContentReference = this.contentReferences.find((ref) => typeof ref.reference === 'object' && 'variableName' in ref.reference && ref.reference.variableName === attachment.name);
const correspondingContentReference = this.contentReferences.find((ref) => (typeof ref.reference === 'object' && 'variableName' in ref.reference && ref.reference.variableName === attachment.name) || (URI.isUri(ref.reference) && basename(ref.reference.path) === attachment.name));
const isAttachmentOmitted = correspondingContentReference?.options?.status?.kind === ChatResponseReferencePartStatusKind.Omitted;
const isAttachmentPartialOrOmitted = isAttachmentOmitted || correspondingContentReference?.options?.status?.kind === ChatResponseReferencePartStatusKind.Partial;

Expand All @@ -111,19 +112,23 @@ export class ChatAttachmentsContentPart extends Disposable {
ariaLabel = range ? localize('chat.fileAttachmentWithRange3', "Attached: {0}, line {1} to line {2}.", friendlyName, range.startLineNumber, range.endLineNumber) : localize('chat.fileAttachment3', "Attached: {0}.", friendlyName);
}

const fileOptions = {
hidePath: true,
title: correspondingContentReference?.options?.status?.description
};
label.setFile(resource, attachment.isFile ? {
...fileOptions,
fileKind: FileKind.FILE,
range,
} : {
...fileOptions,
fileKind: FileKind.FOLDER,
icon: !this.themeService.getFileIconTheme().hasFolderIcons ? FolderThemeIcon : undefined
});
if (attachment.isOmitted) {
this.customAttachment(widget, friendlyName, hoverDelegate, ariaLabel, isAttachmentOmitted);
} else {
const fileOptions = {
hidePath: true,
title: correspondingContentReference?.options?.status?.description
};
label.setFile(resource, attachment.isFile ? {
...fileOptions,
fileKind: FileKind.FILE,
range,
} : {
...fileOptions,
fileKind: FileKind.FOLDER,
icon: !this.themeService.getFileIconTheme().hasFolderIcons ? FolderThemeIcon : undefined
});
}

this.instantiationService.invokeFunction(accessor => {
if (resource) {
Expand All @@ -132,14 +137,8 @@ export class ChatAttachmentsContentPart extends Disposable {
});
} else if (attachment.isImage) {
ariaLabel = localize('chat.imageAttachment', "Attached image, {0}", attachment.name);
const hoverElement = dom.$('div.chat-attached-context-hover');
hoverElement.setAttribute('aria-label', ariaLabel);

// Custom label
const pillIcon = dom.$('div.chat-attached-context-pill', {}, dom.$(isAttachmentOmitted ? 'span.codicon.codicon-warning' : 'span.codicon.codicon-file-media'));
const textLabel = dom.$('span.chat-attached-context-custom-text', {}, attachment.name);
widget.appendChild(pillIcon);
widget.appendChild(textLabel);
const hoverElement = this.customAttachment(widget, attachment.name, hoverDelegate, ariaLabel, isAttachmentOmitted, attachment.isImage);

if (attachment.references) {
widget.style.cursor = 'pointer';
Expand All @@ -151,11 +150,7 @@ export class ChatAttachmentsContentPart extends Disposable {
this.attachedContextDisposables.add(dom.addDisposableListener(widget, 'click', clickHandler));
}

if (isAttachmentPartialOrOmitted) {
hoverElement.textContent = localize('chat.imageAttachmentHover', "Selected model does not support images.");
textLabel.style.textDecoration = 'line-through';
this.attachedContextDisposables.add(this.hoverService.setupManagedHover(hoverDelegate, widget, hoverElement, { trapFocus: true }));
} else {
if (!isAttachmentPartialOrOmitted) {
attachmentInitPromises.push(Promises.withAsyncBody(async (resolve) => {
let buffer: Uint8Array;
try {
Expand Down Expand Up @@ -263,6 +258,24 @@ export class ChatAttachmentsContentPart extends Disposable {
});
}

private customAttachment(widget: HTMLElement, friendlyName: string, hoverDelegate: IHoverDelegate, ariaLabel: string, isAttachmentOmitted: boolean, isImage?: boolean): HTMLElement {
const pillIcon = dom.$('div.chat-attached-context-pill', {}, dom.$(isAttachmentOmitted ? 'span.codicon.codicon-warning' : 'span.codicon.codicon-file-media'));
const textLabel = dom.$('span.chat-attached-context-custom-text', {}, friendlyName);
widget.appendChild(pillIcon);
widget.appendChild(textLabel);

const hoverElement = dom.$('div.chat-attached-context-hover');
hoverElement.setAttribute('aria-label', ariaLabel);

if (isAttachmentOmitted) {
widget.classList.add('warning');
hoverElement.textContent = localize('chat.fileAttachmentHover', "Selected model does not support this {0} type.", isImage ? 'image' : 'file');
this.attachedContextDisposables.add(this.hoverService.setupManagedHover(hoverDelegate, widget, hoverElement, { trapFocus: true }));
}

return hoverElement;
}

private openResource(resource: URI, isDirectory: true): void;
private openResource(resource: URI, isDirectory: false, range: IRange | undefined): void;
private openResource(resource: URI, isDirectory?: boolean, range?: IRange): void {
Expand Down
30 changes: 21 additions & 9 deletions src/vs/workbench/contrib/chat/browser/chatDragAndDrop.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { basename, joinPath } from '../../../../base/common/resources.js';
import { URI } from '../../../../base/common/uri.js';
import { IRange } from '../../../../editor/common/core/range.js';
import { SymbolKinds } from '../../../../editor/common/languages.js';
import { ITextModelService } from '../../../../editor/common/services/resolverService.js';
import { localize } from '../../../../nls.js';
import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { CodeDataTransfers, containsDragType, DocumentSymbolTransferData, extractEditorsDropData, extractSymbolDropData, IDraggedResourceEditorInput } from '../../../../platform/dnd/browser/dnd.js';
Expand Down Expand Up @@ -50,7 +51,8 @@ export class ChatDragAndDrop extends Themable {
@IExtensionService private readonly extensionService: IExtensionService,
@IFileService protected readonly fileService: IFileService,
@IEditorService protected readonly editorService: IEditorService,
@IDialogService protected readonly dialogService: IDialogService
@IDialogService protected readonly dialogService: IDialogService,
@ITextModelService protected readonly textModelService: ITextModelService
) {
super(themeService);

Expand Down Expand Up @@ -266,13 +268,13 @@ export class ChatDragAndDrop extends Themable {
return undefined;
}

return getResourceAttachContext(editor.resource, stat.isDirectory);
return await getResourceAttachContext(editor.resource, stat.isDirectory, this.textModelService);
}

private async resolveUntitledAttachContext(editor: IDraggedResourceEditorInput): Promise<IChatRequestVariableEntry | undefined> {
// If the resource is known, we can use it directly
if (editor.resource) {
return getResourceAttachContext(editor.resource, false);
return await getResourceAttachContext(editor.resource, false, this.textModelService);
}

// Otherwise, we need to check if the contents are already open in another editor
Expand All @@ -281,7 +283,7 @@ export class ChatDragAndDrop extends Themable {
const model = await canidate.resolve();
const contents = model.textEditorModel?.getValue();
if (contents === editor.contents) {
return getResourceAttachContext(canidate.resource, false);
return await getResourceAttachContext(canidate.resource, false, this.textModelService);
}
}

Expand Down Expand Up @@ -352,9 +354,10 @@ export class EditsDragAndDrop extends ChatDragAndDrop {
@IExtensionService extensionService: IExtensionService,
@IFileService fileService: IFileService,
@IEditorService editorService: IEditorService,
@IDialogService dialogService: IDialogService
@IDialogService dialogService: IDialogService,
@ITextModelService textModelService: ITextModelService
) {
super(attachmentModel, styles, themeService, extensionService, fileService, editorService, dialogService);
super(attachmentModel, styles, themeService, extensionService, fileService, editorService, dialogService, textModelService);
}

protected override handleDrop(context: IChatRequestVariableEntry[]): void {
Expand All @@ -376,8 +379,8 @@ export class EditsDragAndDrop extends ChatDragAndDrop {
}

const resolvedFiles = await resolveFilesInDirectory(directory, fileSystemProvider, true);
const resolvedFileContext = resolvedFiles.map(file => getResourceAttachContext(file, false)).filter(context => !!context);
nonDirectoryContext.push(...resolvedFileContext);
const resolvedFileContext = await Promise.all(resolvedFiles.map(file => getResourceAttachContext(file, false, this.textModelService)));
nonDirectoryContext.push(...resolvedFileContext.filter(context => !!context));
}

super.handleDrop(nonDirectoryContext);
Expand Down Expand Up @@ -417,13 +420,22 @@ async function resolveFilesInDirectory(resource: URI, fileSystemProvider: IFileS
return [...files, ...subFiles.flat()];
}

function getResourceAttachContext(resource: URI, isDirectory: boolean): IChatRequestVariableEntry | undefined {
async function getResourceAttachContext(resource: URI, isDirectory: boolean, textModelService: ITextModelService): Promise<IChatRequestVariableEntry | undefined> {
let isOmitted = false;
try {
const createdModel = await textModelService.createModelReference(resource);
createdModel.dispose();
} catch {
isOmitted = true;
}

return {
value: resource,
id: resource.toString(),
name: basename(resource),
isFile: !isDirectory,
isDirectory,
isOmitted
};
}

Expand Down
Loading

0 comments on commit 984a651

Please sign in to comment.