Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor, reduce IChatVariablesService to the bare minimum #240035

Merged
merged 12 commits into from
Feb 8, 2025
11 changes: 0 additions & 11 deletions src/vs/workbench/api/common/extHostChatAgents2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,6 @@ class ExtHostChatAgent {
private _welcomeMessageProvider?: vscode.ChatWelcomeMessageProvider | undefined;
private _titleProvider?: vscode.ChatTitleProvider | undefined;
private _requester: vscode.ChatRequesterInformation | undefined;
private _supportsSlowReferences: boolean | undefined;
private _pauseStateEmitter = new Emitter<vscode.ChatParticipantPauseStateEvent>();

constructor(
Expand Down Expand Up @@ -817,7 +816,6 @@ class ExtHostChatAgent {
helpTextPostfix: (!this._helpTextPostfix || typeof this._helpTextPostfix === 'string') ? this._helpTextPostfix : typeConvert.MarkdownString.from(this._helpTextPostfix),
supportIssueReporting: this._supportIssueReporting,
requester: this._requester,
supportsSlowVariables: this._supportsSlowReferences,
});
updateScheduled = false;
});
Expand Down Expand Up @@ -947,15 +945,6 @@ class ExtHostChatAgent {
get requester() {
return that._requester;
},
set supportsSlowReferences(v) {
checkProposedApiEnabled(that.extension, 'chatParticipantPrivate');
that._supportsSlowReferences = v;
updateMetadataSoon();
},
get supportsSlowReferences() {
checkProposedApiEnabled(that.extension, 'chatParticipantPrivate');
return that._supportsSlowReferences;
},
dispose() {
disposed = true;
that._followupProvider = undefined;
Expand Down
28 changes: 14 additions & 14 deletions src/vs/workbench/contrib/chat/browser/actions/chatActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ import { EXTENSIONS_CATEGORY, IExtensionsWorkbenchService } from '../../../exten
import { ChatAgentLocation, IChatAgentService } from '../../common/chatAgents.js';
import { ChatContextKeys } from '../../common/chatContextKeys.js';
import { extractAgentAndCommand } from '../../common/chatParserTypes.js';
import { IChatQuotasService, OPEN_CHAT_QUOTA_EXCEEDED_DIALOG, quotaToButtonMessage } from '../chatQuotasService.js';
import { IChatDetail, IChatService } from '../../common/chatService.js';
import { IChatVariablesService } from '../../common/chatVariables.js';
import { IChatRequestViewModel, IChatResponseViewModel, isRequestVM } from '../../common/chatViewModel.js';
import { IChatWidgetHistoryService } from '../../common/chatWidgetHistoryService.js';
import { CopilotUsageExtensionFeatureId } from '../../common/languageModelStats.js';
import { ILanguageModelToolsService } from '../../common/languageModelToolsService.js';
import { ChatViewId, IChatWidget, IChatWidgetService, showChatView } from '../chat.js';
import { IChatEditorOptions } from '../chatEditor.js';
import { ChatEditorInput } from '../chatEditorInput.js';
import { IChatQuotasService, OPEN_CHAT_QUOTA_EXCEEDED_DIALOG, quotaToButtonMessage } from '../chatQuotasService.js';
import { ChatViewPane } from '../chatViewPane.js';
import { convertBufferToScreenshotVariable } from '../contrib/screenshot.js';
import { clearChatEditor } from './chatClear.js';
Expand All @@ -63,9 +63,9 @@ export interface IChatViewOpenOptions {
*/
isPartialQuery?: boolean;
/**
* A list of simple variables that will be resolved and attached if they exist.
* A list of tools IDs with `canBeReferencedInPrompt` that will be resolved and attached if they exist.
*/
variableIds?: string[];
toolIds?: string[];
/**
* Any previous chat requests and responses that should be shown in the chat view.
*/
Expand Down Expand Up @@ -112,7 +112,7 @@ class OpenChatGlobalAction extends Action2 {
opts = typeof opts === 'string' ? { query: opts } : opts;

const chatService = accessor.get(IChatService);
const chatVariablesService = accessor.get(IChatVariablesService);
const toolsService = accessor.get(ILanguageModelToolsService);
const viewsService = accessor.get(IViewsService);
const hostService = accessor.get(IHostService);

Expand All @@ -138,17 +138,17 @@ class OpenChatGlobalAction extends Action2 {
chatWidget.acceptInput(opts.query);
}
}
if (opts?.variableIds && opts.variableIds.length > 0) {
const actualVariables = chatVariablesService.getVariables();
for (const actualVariable of actualVariables) {
if (opts.variableIds.includes(actualVariable.id)) {
if (opts?.toolIds && opts.toolIds.length > 0) {
for (const toolId of opts.toolIds) {
const tool = toolsService.getTool(toolId);
if (tool) {
chatWidget.attachmentModel.addContext({
range: undefined,
id: actualVariable.id ?? '',
id: tool.id,
name: tool.displayName,
fullName: tool.displayName,
value: undefined,
fullName: actualVariable.fullName,
name: actualVariable.name,
icon: actualVariable.icon
icon: ThemeIcon.isThemeIcon(tool.icon) ? tool.icon : undefined,
isTool: true
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import { ChatContextKeys } from '../../common/chatContextKeys.js';
import { IChatEditingService, WorkingSetEntryState } from '../../common/chatEditingService.js';
import { IChatRequestVariableEntry } from '../../common/chatModel.js';
import { ChatRequestAgentPart } from '../../common/chatParserTypes.js';
import { IChatVariableData, IChatVariablesService } from '../../common/chatVariables.js';
import { IChatVariablesService } from '../../common/chatVariables.js';
import { ILanguageModelToolsService } from '../../common/languageModelToolsService.js';
import { DOCUMENTATION_URL, PROMPT_FILE_EXTENSION } from '../../common/promptSyntax/constants.js';
import { IChatWidget, IChatWidgetService, IQuickChatService, showChatView, showEditsView } from '../chat.js';
Expand All @@ -71,7 +71,7 @@ export function registerChatContextActions() {
* We fill the quickpick with these types, and enable some quick access providers
*/
type IAttachmentQuickPickItem = ICommandVariableQuickPickItem | IQuickAccessQuickPickItem | IToolQuickPickItem |
IImageQuickPickItem | IVariableQuickPickItem | IOpenEditorsQuickPickItem | ISearchResultsQuickPickItem |
IImageQuickPickItem | IOpenEditorsQuickPickItem | ISearchResultsQuickPickItem |
IScreenShotQuickPickItem | IRelatedFilesQuickPickItem | IPromptInstructionsQuickPickItem;

/**
Expand Down Expand Up @@ -154,7 +154,6 @@ interface ICommandVariableQuickPickItem extends IQuickPickItem {
command: Command;
name?: string;
value: unknown;
isDynamic: true;

icon?: ThemeIcon;
}
Expand All @@ -166,11 +165,6 @@ interface IToolQuickPickItem extends IQuickPickItem {
icon?: ThemeIcon;
}

interface IVariableQuickPickItem extends IQuickPickItem {
kind: 'variable';
variable: IChatVariableData;
}

interface IQuickAccessQuickPickItem extends IQuickPickItem {
kind: 'quickaccess';
id: string;
Expand Down Expand Up @@ -474,7 +468,6 @@ export class AttachContextAction extends Action2 {
symbolKind: pick.symbol.kind,
fullName: pick.label,
name: pick.symbol.name,
isDynamic: true
});
} else if (isIQuickPickItemWithResource(pick) && pick.resource) {
if (/\.(png|jpg|jpeg|bmp|gif|tiff)$/i.test(pick.resource.path)) {
Expand All @@ -488,7 +481,6 @@ export class AttachContextAction extends Action2 {
name: pick.label,
fullName: pick.label,
value: resizedImage,
isDynamic: true,
isImage: true
});
}
Expand All @@ -502,7 +494,6 @@ export class AttachContextAction extends Action2 {
value: pick.resource,
name: pick.label,
isFile: true,
isDynamic: true,
});
}
}
Expand All @@ -513,7 +504,6 @@ export class AttachContextAction extends Action2 {
value: { uri: pick.uri, range: pick.range.decoration },
fullName: pick.label,
name: pick.symbolName!,
isDynamic: true
});
} else if (isIOpenEditorsQuickPickItem(pick)) {
for (const editor of editorService.editors.filter(e => e instanceof FileEditorInput || e instanceof DiffEditorInput || e instanceof UntitledTextEditorInput || e instanceof NotebookEditorInput)) {
Expand All @@ -527,7 +517,6 @@ export class AttachContextAction extends Action2 {
value: uri,
name: labelService.getUriBasenameLabel(uri),
isFile: true,
isDynamic: true
});
}
}
Expand All @@ -543,7 +532,6 @@ export class AttachContextAction extends Action2 {
value: result.resource,
name: labelService.getUriBasenameLabel(result.resource),
isFile: true,
isDynamic: true
});
}
}
Expand Down Expand Up @@ -601,7 +589,6 @@ export class AttachContextAction extends Action2 {
}
toAttach.push({
...attachmentPick,
isDynamic: attachmentPick.isDynamic,
value: attachmentPick.value,
name: `${typeof attachmentPick.value === 'string' && attachmentPick.value.startsWith('#') ? attachmentPick.value.slice(1) : ''}${selection}`,
// Apply the original icon with the new name
Expand All @@ -623,19 +610,8 @@ export class AttachContextAction extends Action2 {
name: localize('pastedImage', 'Pasted Image'),
fullName: localize('pastedImage', 'Pasted Image'),
value: fileBuffer,
isDynamic: true,
isImage: true
});
} else if (attachmentPick.kind === 'variable') {
// All other dynamic variables and static variables
toAttach.push({
range: undefined,
id: pick.id ?? '',
value: undefined,
fullName: pick.label,
name: attachmentPick.variable.name,
icon: attachmentPick.variable.icon
});
}
}
}
Expand All @@ -651,7 +627,6 @@ export class AttachContextAction extends Action2 {
override async run(accessor: ServicesAccessor, ...args: any[]): Promise<void> {
const quickInputService = accessor.get(IQuickInputService);
const chatAgentService = accessor.get(IChatAgentService);
const chatVariablesService = accessor.get(IChatVariablesService);
const commandService = accessor.get(ICommandService);
const widgetService = accessor.get(IChatWidgetService);
const languageModelToolsService = accessor.get(ILanguageModelToolsService);
Expand All @@ -673,22 +648,8 @@ export class AttachContextAction extends Action2 {
}
const chatEditingService = widget.location === ChatAgentLocation.EditingSession ? accessor.get(IChatEditingService) : undefined;

const usedAgent = widget.parsedInput.parts.find(p => p instanceof ChatRequestAgentPart);
const slowSupported = usedAgent ? usedAgent.agent.metadata.supportsSlowVariables : true;
const quickPickItems: IAttachmentQuickPickItem[] = [];
if (!context || !context.showFilesOnly) {
for (const variable of chatVariablesService.getVariables()) {
if (variable.fullName && (!variable.isSlow || slowSupported)) {
quickPickItems.push({
kind: 'variable',
variable,
label: variable.fullName,
id: variable.id,
iconClass: variable.icon ? ThemeIcon.asClassName(variable.icon) : undefined,
});
}
}

if (extensionService.extensions.some(ext => isProposedApiEnabled(ext, 'chatReferenceBinaryData'))) {
const imageData = await clipboardService.readImage();
if (isImage(imageData)) {
Expand Down Expand Up @@ -725,7 +686,6 @@ export class AttachContextAction extends Action2 {
icon: variable.icon,
iconClass: variable.icon ? ThemeIcon.asClassName(variable.icon) : undefined,
value: variable.value,
isDynamic: true,
name: variable.name
});
} else {
Expand Down Expand Up @@ -765,7 +725,6 @@ export class AttachContextAction extends Action2 {
quickPickItems.push({
kind: 'command',
id: 'chatContext.notebook.kernelVariable',
isDynamic: true,
icon: ThemeIcon.fromId(Codicon.serverEnvironment.id),
iconClass: ThemeIcon.asClassName(Codicon.serverEnvironment),
value: 'kernelVariable',
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/contrib/chat/browser/chat.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ class ChatSlashStaticSlashCommandsContribution extends Disposable {
}

const variables = [
...chatVariablesService.getVariables(),
{ name: 'file', description: nls.localize('file', "Choose a file in the workspace") }
];
const variableText = variables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export class ChatAttachmentModel extends Disposable {
id: uri.toString() + (range?.toString() ?? ''),
name: basename(uri),
isFile: true,
isDynamic: true,
isMarkedReadonly,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ export const toChatVariable = (
isSelection: false,
enabled: true,
isFile: true,
isDynamic: true,
isMarkedReadonly: isPromptSnippet,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,6 @@ class CollapsibleListRenderer implements IListRenderer<IChatCollapsibleListItem,
private labels: ResourceLabels,
private menuId: MenuId | undefined,
@IThemeService private readonly themeService: IThemeService,
@IChatVariablesService private readonly chatVariablesService: IChatVariablesService,
@IProductService private readonly productService: IProductService,
@IInstantiationService private readonly instantiationService: IInstantiationService,
@IContextKeyService private readonly contextKeyService: IContextKeyService,
Expand Down Expand Up @@ -364,12 +363,8 @@ class CollapsibleListRenderer implements IListRenderer<IChatCollapsibleListItem,
const label = `Kernel variable`;
templateData.label.setLabel(label, asVariableName, { title: data.options?.status?.description });
} else {
const variable = this.chatVariablesService.getVariable(reference.variableName);
// This is a hack to get chat attachment ThemeIcons to render for resource labels
const asThemeIcon = variable?.icon ? `$(${variable.icon.id}) ` : '';
const asVariableName = `#${reference.variableName}`; // Fallback, shouldn't really happen
const label = `${asThemeIcon}${variable?.fullName ?? asVariableName}`;
templateData.label.setLabel(label, asVariableName, { title: data.options?.status?.description ?? variable?.description });
// Nothing else is expected to fall into here
templateData.label.setLabel('Unknown variable type');
}
} else if (typeof reference === 'string') {
templateData.label.setLabel(reference, undefined, { iconPath: URI.isUri(icon) ? icon : undefined, title: data.options?.status?.description ?? data.title });
Expand Down
3 changes: 0 additions & 3 deletions src/vs/workbench/contrib/chat/browser/chatDragAndDrop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ export class ChatDragAndDrop extends Themable {
symbolKind: symbol.kind,
fullName: `$(${SymbolKinds.toIcon(symbol.kind).id}) ${symbol.name}`,
name: symbol.name,
isDynamic: true
};
});
}
Expand Down Expand Up @@ -422,7 +421,6 @@ function getResourceAttachContext(resource: URI, isDirectory: boolean): IChatReq
name: basename(resource),
isFile: !isDirectory,
isDirectory,
isDynamic: true
};
}

Expand All @@ -441,7 +439,6 @@ async function getImageAttachContext(editor: EditorInput | IDraggedResourceEdito
fullName: editor.resource.path,
value: resizedImage,
icon: Codicon.fileMedia,
isDynamic: true,
isImage: true,
isFile: false,
references: [{ reference: editor.resource, kind: 'reference' }]
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/contrib/chat/browser/chatInputPart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
name: uri.fsPath,
value: uri,
isFile: false,
isDynamic: true,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js';
import { contentRefUrl } from '../common/annotations.js';
import { getFullyQualifiedId, IChatAgentCommand, IChatAgentData, IChatAgentNameService, IChatAgentService } from '../common/chatAgents.js';
import { chatSlashCommandBackground, chatSlashCommandForeground } from '../common/chatColors.js';
import { chatAgentLeader, ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicVariablePart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestToolPart, ChatRequestVariablePart, chatSubcommandLeader, IParsedChatRequest, IParsedChatRequestPart } from '../common/chatParserTypes.js';
import { chatAgentLeader, ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicVariablePart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestToolPart, chatSubcommandLeader, IParsedChatRequest, IParsedChatRequestPart } from '../common/chatParserTypes.js';
import { IChatMarkdownContent, IChatService } from '../common/chatService.js';
import { IChatVariablesService } from '../common/chatVariables.js';
import { ILanguageModelToolsService } from '../common/languageModelToolsService.js';
import { IChatWidgetService } from './chat.js';
import { ChatAgentHover, getChatAgentHoverOptions } from './chatAgentHover.js';
Expand Down Expand Up @@ -86,7 +85,6 @@ export class ChatMarkdownDecorationsRenderer {
@IChatService private readonly chatService: IChatService,
@IChatWidgetService private readonly chatWidgetService: IChatWidgetService,
@ICommandService private readonly commandService: ICommandService,
@IChatVariablesService private readonly chatVariablesService: IChatVariablesService,
@ILabelService private readonly labelService: ILabelService,
@ILanguageModelToolsService private readonly toolsService: ILanguageModelToolsService,
@IChatMarkdownAnchorService private readonly chatMarkdownAnchorService: IChatMarkdownAnchorService,
Expand Down Expand Up @@ -114,9 +112,8 @@ export class ChatMarkdownDecorationsRenderer {
const title = uri ? this.labelService.getUriLabel(uri, { relative: true }) :
part instanceof ChatRequestSlashCommandPart ? part.slashCommand.detail :
part instanceof ChatRequestAgentSubcommandPart ? part.command.description :
part instanceof ChatRequestVariablePart ? (this.chatVariablesService.getVariable(part.variableName)?.description) :
part instanceof ChatRequestToolPart ? (this.toolsService.getTool(part.toolId)?.userDescription) :
'';
part instanceof ChatRequestToolPart ? (this.toolsService.getTool(part.toolId)?.userDescription) :
'';

const args: IDecorationWidgetArgs = { title };
const text = part.text;
Expand Down
2 changes: 0 additions & 2 deletions src/vs/workbench/contrib/chat/browser/chatPasteProviders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ async function getImageAttachContext(data: Uint8Array, mimeType: string, token:
name: displayName,
isImage: true,
icon: Codicon.fileMedia,
isDynamic: true,
mimeType
};
}
Expand Down Expand Up @@ -250,7 +249,6 @@ function getCopiedContext(code: string, file: URI, language: string, range: IRan
id: `${fileName}${start}${end}${range.startColumn}${range.endColumn}`,
name: `${fileName} ${pastedLines}`,
icon: Codicon.code,
isDynamic: true,
pastedLines,
language,
fileName: file.toString(),
Expand Down
Loading
Loading