From dbd391ebba2827eb12707abf499c1912d2aac31e Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Thu, 1 Feb 2024 12:33:38 -0300 Subject: [PATCH] Don't allow picking the same variable multiple times (#203975) * Cache agent subCommands, make some async code sync * Fix lastSlashCommands * Cache parsed chat query in one place so we don't have to parse the same thing over and over * Don't allow picking the same variable multiple times Fix microsoft/vscode-copilot-release#733 * Fix tests --- .../api/browser/mainThreadChatAgents2.ts | 11 ++- src/vs/workbench/contrib/chat/browser/chat.ts | 2 + .../contrib/chat/browser/chatWidget.ts | 16 +++- .../browser/contrib/chatInputEditorContrib.ts | 89 ++++++++----------- .../contrib/chat/common/chatAgents.ts | 12 +-- .../contrib/chat/common/chatRequestParser.ts | 12 ++- .../contrib/chat/common/chatServiceImpl.ts | 4 +- ..._agent_and_subcommand_after_newline.0.snap | 8 +- ..._subcommand_with_leading_whitespace.0.snap | 8 +- ...uestParser_agent_with_question_mark.0.snap | 8 +- ...er_agent_with_subcommand_after_text.0.snap | 8 +- ...hatRequestParser_agents__subCommand.0.snap | 8 +- ..._agents_and_variables_and_multiline.0.snap | 8 +- ..._and_variables_and_multiline__part2.0.snap | 8 +- .../__snapshots__/Chat_can_serialize.1.snap | 2 +- .../test/common/chatRequestParser.test.ts | 54 +++++------ .../chat/test/common/chatService.test.ts | 2 - 17 files changed, 154 insertions(+), 106 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts index 7be5e2e6ea0..8ed8650fffc 100644 --- a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts +++ b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts @@ -18,7 +18,7 @@ import { ExtHostChatAgentsShape2, ExtHostContext, IChatProgressDto, IExtensionCh import { IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat'; import { ChatInputPart } from 'vs/workbench/contrib/chat/browser/chatInputPart'; import { AddDynamicVariableAction, IAddDynamicVariableContext } from 'vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables'; -import { IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; +import { IChatAgentCommand, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; import { ChatRequestAgentPart } from 'vs/workbench/contrib/chat/common/chatParserTypes'; import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser'; import { IChatFollowup, IChatProgress, IChatService } from 'vs/workbench/contrib/chat/common/chatService'; @@ -75,6 +75,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA } $registerAgent(handle: number, name: string, metadata: IExtensionChatAgentMetadata): void { + let lastSlashCommands: IChatAgentCommand[] | undefined; const d = this._chatAgentService.registerAgent({ id: name, metadata: revive(metadata), @@ -93,11 +94,15 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA return this._proxy.$provideFollowups(handle, sessionId, token); }, + get lastSlashCommands() { + return lastSlashCommands; + }, provideSlashCommands: async (token) => { if (!this._agents.get(handle)?.hasSlashCommands) { return []; // save an IPC call } - return this._proxy.$provideSlashCommands(handle, token); + lastSlashCommands = await this._proxy.$provideSlashCommands(handle, token); + return lastSlashCommands; } }); this._agents.set(handle, { @@ -141,7 +146,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA return; } - const parsedRequest = (await this._instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue())).parts; + const parsedRequest = this._instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue()).parts; const agentPart = parsedRequest.find((part): part is ChatRequestAgentPart => part instanceof ChatRequestAgentPart); const thisAgentName = this._agents.get(handle)?.name; if (agentPart?.agent.id !== thisAgentName) { diff --git a/src/vs/workbench/contrib/chat/browser/chat.ts b/src/vs/workbench/contrib/chat/browser/chat.ts index 17d1875bfc4..ff95527a982 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.ts @@ -9,6 +9,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { Selection } from 'vs/editor/common/core/selection'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IChatWidgetContrib } from 'vs/workbench/contrib/chat/browser/chatWidget'; +import { IParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes'; import { IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, IChatWelcomeMessageViewModel } from 'vs/workbench/contrib/chat/common/chatViewModel'; export const IChatWidgetService = createDecorator('chatWidgetService'); @@ -105,6 +106,7 @@ export interface IChatWidget { readonly inputEditor: ICodeEditor; readonly providerId: string; readonly supportsFileReferences: boolean; + readonly parsedInput: IParsedChatRequest; getContrib(id: string): T | undefined; reveal(item: ChatTreeItem): void; diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 7487035e490..e667ddf78b2 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -33,6 +33,8 @@ import { ChatModelInitState, IChatModel } from 'vs/workbench/contrib/chat/common import { IChatReplyFollowup, IChatService } from 'vs/workbench/contrib/chat/common/chatService'; import { ChatViewModel, IChatResponseViewModel, isRequestVM, isResponseVM, isWelcomeVM } from 'vs/workbench/contrib/chat/common/chatViewModel'; import { IThemeService } from 'vs/platform/theme/common/themeService'; +import { IParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes'; +import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser'; const $ = dom.$; @@ -127,6 +129,15 @@ export class ChatWidget extends Disposable implements IChatWidget { return this._viewModel; } + private parsedChatRequest: IParsedChatRequest | undefined; + get parsedInput() { + if (this.parsedChatRequest === undefined) { + this.parsedChatRequest = this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(this.viewModel!.sessionId, this.getInput()); + } + + return this.parsedChatRequest; + } + constructor( readonly viewContext: IChatWidgetViewContext, private readonly viewOptions: IChatWidgetViewOptions, @@ -436,6 +447,9 @@ export class ChatWidget extends Disposable implements IChatWidget { }); })); this._register(this.inputPart.onDidChangeHeight(() => this.bodyDimension && this.layout(this.bodyDimension.height, this.bodyDimension.width))); + this._register(this.inputEditor.onDidChangeModelContent(() => { + this.parsedChatRequest = undefined; + })); } private onDidStyleChange(): void { @@ -553,8 +567,6 @@ export class ChatWidget extends Disposable implements IChatWidget { const lastResponse = responses?.[responses.length - 1]; this._chatAccessibilityService.acceptResponse(lastResponse, requestId); }); - } else { - this._chatAccessibilityService.acceptResponse(undefined, requestId); } } } diff --git a/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts b/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts index 503d7c98258..84956526adb 100644 --- a/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts +++ b/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts @@ -15,7 +15,6 @@ import { CompletionContext, CompletionItem, CompletionItemKind, CompletionList } import { ITextModel } from 'vs/editor/common/model'; import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; import { localize } from 'vs/nls'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { Registry } from 'vs/platform/registry/common/platform'; import { inputPlaceholderForeground } from 'vs/platform/theme/common/colorRegistry'; @@ -33,7 +32,6 @@ import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestP import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; import { IChatSlashCommandService } from 'vs/workbench/contrib/chat/common/chatSlashCommands'; import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables'; -import { isResponseVM } from 'vs/workbench/contrib/chat/common/chatViewModel'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; const decorationDescription = 'chat'; @@ -55,7 +53,6 @@ class InputEditorDecorations extends Disposable { constructor( private readonly widget: IChatWidget, - @IInstantiationService private readonly instantiationService: IInstantiationService, @ICodeEditorService private readonly codeEditorService: ICodeEditorService, @IThemeService private readonly themeService: IThemeService, @IChatService private readonly chatService: IChatService, @@ -154,7 +151,7 @@ class InputEditorDecorations extends Disposable { return; } - const parsedRequest = (await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(viewModel.sessionId, inputValue)).parts; + const parsedRequest = this.widget.parsedInput.parts; let placeholderDecoration: IDecorationOptions[] | undefined; const agentPart = parsedRequest.find((p): p is ChatRequestAgentPart => p instanceof ChatRequestAgentPart); @@ -294,7 +291,6 @@ class SlashCommandCompletions extends Disposable { constructor( @ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService, @IChatWidgetService private readonly chatWidgetService: IChatWidgetService, - @IInstantiationService private readonly instantiationService: IInstantiationService, @IChatSlashCommandService private readonly chatSlashCommandService: IChatSlashCommandService ) { super(); @@ -313,7 +309,7 @@ class SlashCommandCompletions extends Disposable { return null; } - const parsedRequest = (await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue())).parts; + const parsedRequest = widget.parsedInput.parts; const usedAgent = parsedRequest.find(p => p instanceof ChatRequestAgentPart); if (usedAgent) { // No (classic) global slash commands when an agent is used @@ -351,7 +347,6 @@ class AgentCompletions extends Disposable { @ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService, @IChatWidgetService private readonly chatWidgetService: IChatWidgetService, @IChatAgentService private readonly chatAgentService: IChatAgentService, - @IInstantiationService private readonly instantiationService: IInstantiationService, ) { super(); @@ -364,7 +359,7 @@ class AgentCompletions extends Disposable { return null; } - const parsedRequest = (await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue())).parts; + const parsedRequest = widget.parsedInput.parts; const usedAgent = parsedRequest.find(p => p instanceof ChatRequestAgentPart); if (usedAgent && !Range.containsPosition(usedAgent.editorRange, position)) { // Only one agent allowed @@ -407,7 +402,7 @@ class AgentCompletions extends Disposable { return null; } - const parsedRequest = (await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue())).parts; + const parsedRequest = widget.parsedInput.parts; const usedAgentIdx = parsedRequest.findIndex((p): p is ChatRequestAgentPart => p instanceof ChatRequestAgentPart); if (usedAgentIdx < 0) { return; @@ -428,7 +423,7 @@ class AgentCompletions extends Disposable { } const usedAgent = parsedRequest[usedAgentIdx] as ChatRequestAgentPart; - const commands = await usedAgent.agent.provideSlashCommands(token); + const commands = await usedAgent.agent.provideSlashCommands(token); // Refresh the cache here return { suggestions: commands.map((c, i) => { @@ -576,7 +571,6 @@ class VariableCompletions extends Disposable { @ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService, @IChatWidgetService private readonly chatWidgetService: IChatWidgetService, @IChatVariablesService private readonly chatVariablesService: IChatVariablesService, - @IConfigurationService private readonly configurationService: IConfigurationService, ) { super(); @@ -595,33 +589,24 @@ class VariableCompletions extends Disposable { return null; } - const history = widget.viewModel!.getItems() - .filter(isResponseVM); - - // TODO@roblourens work out a real API for this- maybe it can be part of the two-step flow that @file will probably use - const historyVariablesEnabled = this.configurationService.getValue('chat.experimental.historyVariables'); - const historyItems = historyVariablesEnabled ? history.map((h, i): CompletionItem => ({ - label: `${chatVariableLeader}response:${i + 1}`, - detail: h.response.asString(), - insertText: `${chatVariableLeader}response:${String(i + 1).padStart(String(history.length).length, '0')} `, - kind: CompletionItemKind.Text, - range, - })) : []; - - const variableItems = Array.from(this.chatVariablesService.getVariables()).map(v => { - const withLeader = `${chatVariableLeader}${v.name}`; - return { - label: withLeader, - range, - insertText: withLeader + ' ', - detail: v.description, - kind: CompletionItemKind.Text, // The icons are disabled here anyway - sortText: 'z' - }; - }); + const usedVariables = widget.parsedInput.parts.filter((p): p is ChatRequestVariablePart => p instanceof ChatRequestVariablePart); + const variableItems = Array.from(this.chatVariablesService.getVariables()) + // This doesn't look at dynamic variables like `file`, where multiple makes sense. + .filter(v => !usedVariables.some(usedVar => usedVar.variableName === v.name)) + .map(v => { + const withLeader = `${chatVariableLeader}${v.name}`; + return { + label: withLeader, + range, + insertText: withLeader + ' ', + detail: v.description, + kind: CompletionItemKind.Text, // The icons are disabled here anyway + sortText: 'z' + }; + }); return { - suggestions: [...variableItems, ...historyItems] + suggestions: variableItems }; } })); @@ -655,22 +640,22 @@ class ChatTokenDeleter extends Disposable { // If this was a simple delete, try to find out whether it was inside a token if (!change.text && this.widget.viewModel) { - parser.parseChatRequest(this.widget.viewModel.sessionId, previousInputValue).then(previousParsedValue => { - // For dynamic variables, this has to happen in ChatDynamicVariableModel with the other bookkeeping - const deletableTokens = previousParsedValue.parts.filter(p => p instanceof ChatRequestAgentPart || p instanceof ChatRequestAgentSubcommandPart || p instanceof ChatRequestSlashCommandPart || p instanceof ChatRequestVariablePart); - deletableTokens.forEach(token => { - const deletedRangeOfToken = Range.intersectRanges(token.editorRange, change.range); - // Part of this token was deleted, and the deletion range doesn't go off the front of the token, for simpler math - if ((deletedRangeOfToken && !deletedRangeOfToken.isEmpty()) && Range.compareRangesUsingStarts(token.editorRange, change.range) < 0) { - // Assume single line tokens - const length = deletedRangeOfToken.endColumn - deletedRangeOfToken.startColumn; - const rangeToDelete = new Range(token.editorRange.startLineNumber, token.editorRange.startColumn, token.editorRange.endLineNumber, token.editorRange.endColumn - length); - this.widget.inputEditor.executeEdits(this.id, [{ - range: rangeToDelete, - text: '', - }]); - } - }); + const previousParsedValue = parser.parseChatRequest(this.widget.viewModel.sessionId, previousInputValue); + + // For dynamic variables, this has to happen in ChatDynamicVariableModel with the other bookkeeping + const deletableTokens = previousParsedValue.parts.filter(p => p instanceof ChatRequestAgentPart || p instanceof ChatRequestAgentSubcommandPart || p instanceof ChatRequestSlashCommandPart || p instanceof ChatRequestVariablePart); + deletableTokens.forEach(token => { + const deletedRangeOfToken = Range.intersectRanges(token.editorRange, change.range); + // Part of this token was deleted, and the deletion range doesn't go off the front of the token, for simpler math + if ((deletedRangeOfToken && !deletedRangeOfToken.isEmpty()) && Range.compareRangesUsingStarts(token.editorRange, change.range) < 0) { + // Assume single line tokens + const length = deletedRangeOfToken.endColumn - deletedRangeOfToken.startColumn; + const rangeToDelete = new Range(token.editorRange.startLineNumber, token.editorRange.startColumn, token.editorRange.endLineNumber, token.editorRange.endColumn - length); + this.widget.inputEditor.executeEdits(this.id, [{ + range: rangeToDelete, + text: '', + }]); + } }); } diff --git a/src/vs/workbench/contrib/chat/common/chatAgents.ts b/src/vs/workbench/contrib/chat/common/chatAgents.ts index bab428dd38c..1bdaf8ad424 100644 --- a/src/vs/workbench/contrib/chat/common/chatAgents.ts +++ b/src/vs/workbench/contrib/chat/common/chatAgents.ts @@ -17,20 +17,21 @@ import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chat //#region agent service, commands etc -export interface IChatAgentData { - id: string; - metadata: IChatAgentMetadata; -} - export interface IChatAgentHistoryEntry { request: IChatAgentRequest; response: ReadonlyArray; result: IChatAgentResult; } +export interface IChatAgentData { + id: string; + metadata: IChatAgentMetadata; +} + export interface IChatAgent extends IChatAgentData { invoke(request: IChatAgentRequest, progress: (part: IChatProgress) => void, history: IChatAgentHistoryEntry[], token: CancellationToken): Promise; provideFollowups?(sessionId: string, token: CancellationToken): Promise; + lastSlashCommands?: IChatAgentCommand[]; provideSlashCommands(token: CancellationToken): Promise; } @@ -146,6 +147,7 @@ export class ChatAgentService extends Disposable implements IChatAgentService { throw new Error(`No agent with id ${id} registered`); } data.agent.metadata = { ...data.agent.metadata, ...updateMetadata }; + data.agent.provideSlashCommands(CancellationToken.None); // Update the cached slash commands this._onDidChangeAgents.fire(); } diff --git a/src/vs/workbench/contrib/chat/common/chatRequestParser.ts b/src/vs/workbench/contrib/chat/common/chatRequestParser.ts index cdc91180453..007b8c8a191 100644 --- a/src/vs/workbench/contrib/chat/common/chatRequestParser.ts +++ b/src/vs/workbench/contrib/chat/common/chatRequestParser.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { CancellationToken } from 'vs/base/common/cancellation'; import { OffsetRange } from 'vs/editor/common/core/offsetRange'; import { IPosition, Position } from 'vs/editor/common/core/position'; import { Range } from 'vs/editor/common/core/range'; @@ -23,7 +22,7 @@ export class ChatRequestParser { @IChatSlashCommandService private readonly slashCommandService: IChatSlashCommandService ) { } - async parseChatRequest(sessionId: string, message: string): Promise { + parseChatRequest(sessionId: string, message: string): IParsedChatRequest { const parts: IParsedChatRequestPart[] = []; const references = this.variableService.getDynamicVariables(sessionId); // must access this list before any async calls @@ -39,8 +38,7 @@ export class ChatRequestParser { } else if (char === chatAgentLeader) { newPart = this.tryToParseAgent(message.slice(i), message, i, new Position(lineNumber, column), parts); } else if (char === chatSubcommandLeader) { - // TODO try to make this sync - newPart = await this.tryToParseSlashCommand(sessionId, message.slice(i), message, i, new Position(lineNumber, column), parts); + newPart = this.tryToParseSlashCommand(message.slice(i), message, i, new Position(lineNumber, column), parts); } if (!newPart) { @@ -140,7 +138,7 @@ export class ChatRequestParser { return; } - private async tryToParseSlashCommand(sessionId: string, remainingMessage: string, fullMessage: string, offset: number, position: IPosition, parts: ReadonlyArray): Promise { + private tryToParseSlashCommand(remainingMessage: string, fullMessage: string, offset: number, position: IPosition, parts: ReadonlyArray): ChatRequestSlashCommandPart | ChatRequestAgentSubcommandPart | undefined { const nextSlashMatch = remainingMessage.match(slashReg); if (!nextSlashMatch) { return; @@ -169,8 +167,8 @@ export class ChatRequestParser { return; } - const subCommands = await usedAgent.agent.provideSlashCommands(CancellationToken.None); - const subCommand = subCommands.find(c => c.name === command); + const subCommands = usedAgent.agent.lastSlashCommands; + const subCommand = subCommands?.find(c => c.name === command); if (subCommand) { // Valid agent subcommand return new ChatRequestAgentSubcommandPart(slashRange, slashEditorRange, subCommand); diff --git a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts index 8eaa7771901..9574afe19d6 100644 --- a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts @@ -459,7 +459,7 @@ export class ChatService extends Disposable implements IChatService { } private async _sendRequestAsync(model: ChatModel, sessionId: string, provider: IChatProvider, message: string): Promise { - const parsedRequest = await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(sessionId, message); + const parsedRequest = this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(sessionId, message); let request: ChatRequestModel; const agentPart = 'kind' in parsedRequest ? undefined : parsedRequest.parts.find((r): r is ChatRequestAgentPart => r instanceof ChatRequestAgentPart); @@ -656,7 +656,7 @@ export class ChatService extends Disposable implements IChatService { await model.waitForInitialization(); const parsedRequest = typeof message === 'string' ? - await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(sessionId, message) : + this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(sessionId, message) : message; const request = model.addRequest(parsedRequest, variableData || { message: parsedRequest.text, variables: {} }); if (typeof response.message === 'string') { diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap index 7a73d008baa..cc7ecaf508d 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap @@ -28,7 +28,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap index ccd7eb870e0..d46c197f633 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap @@ -28,7 +28,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_question_mark.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_question_mark.0.snap index 65e2aa78ac0..4891a73e641 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_question_mark.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_question_mark.0.snap @@ -14,7 +14,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap index b1954f78a47..d7889981915 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap @@ -14,7 +14,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents__subCommand.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents__subCommand.0.snap index ca9a0569fcd..df42f889d05 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents__subCommand.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents__subCommand.0.snap @@ -14,7 +14,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline.0.snap index 750f1bc39f6..d3f091a95e8 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline.0.snap @@ -14,7 +14,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline__part2.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline__part2.0.snap index 310f36005b3..c4b86b46fff 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline__part2.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline__part2.0.snap @@ -14,7 +14,13 @@ agent: { id: "agent", metadata: { description: "" }, - provideSlashCommands: [Function provideSlashCommands] + provideSlashCommands: [Function provideSlashCommands], + lastSlashCommands: [ + { + name: "subCommand", + description: "" + } + ] }, kind: "agent" }, diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap index a332c9b3af7..0c270a4d7f3 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap @@ -50,7 +50,7 @@ }, response: [ ], responseErrorDetails: undefined, - followups: [ ], + followups: undefined, isCanceled: false, vote: undefined, agent: { diff --git a/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts b/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts index dfdb6cf1fa5..5585c30afa8 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts @@ -9,7 +9,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/uti import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { ILogService, NullLogService } from 'vs/platform/log/common/log'; import { IStorageService } from 'vs/platform/storage/common/storage'; -import { ChatAgentService, IChatAgent, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; +import { ChatAgentService, IChatAgent, IChatAgentCommand, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser'; import { IChatSlashCommandService } from 'vs/workbench/contrib/chat/common/chatSlashCommands'; import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables'; @@ -37,14 +37,14 @@ suite('ChatRequestParser', () => { test('plain text', async () => { parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', 'test'); + const result = parser.parseChatRequest('1', 'test'); await assertSnapshot(result); }); test('plain text with newlines', async () => { parser = instantiationService.createInstance(ChatRequestParser); const text = 'line 1\nline 2\r\nline 3'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -55,7 +55,7 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = '/fix this'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -66,7 +66,7 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = '/explain this'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -77,7 +77,7 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = '/fix /fix'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -86,7 +86,7 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = 'What does #selection mean?'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -95,7 +95,7 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = 'What is #selection?'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); @@ -104,91 +104,95 @@ suite('ChatRequestParser', () => { parser = instantiationService.createInstance(ChatRequestParser); const text = 'What does #selection mean?'; - const result = await parser.parseChatRequest('1', text); + const result = parser.parseChatRequest('1', text); await assertSnapshot(result); }); + const getAgentWithSlashcommands = (slashCommands: IChatAgentCommand[]) => { + return >{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => [], lastSlashCommands: slashCommands }; + }; + test('agent with subcommand after text', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', '@agent Please do /subCommand thanks'); + const result = parser.parseChatRequest('1', '@agent Please do /subCommand thanks'); await assertSnapshot(result); }); test('agents, subCommand', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', '@agent /subCommand Please do thanks'); + const result = parser.parseChatRequest('1', '@agent /subCommand Please do thanks'); await assertSnapshot(result); }); test('agent with question mark', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', '@agent? Are you there'); + const result = parser.parseChatRequest('1', '@agent? Are you there'); await assertSnapshot(result); }); test('agent and subcommand with leading whitespace', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', ' \r\n\t @agent \r\n\t /subCommand Thanks'); + const result = parser.parseChatRequest('1', ' \r\n\t @agent \r\n\t /subCommand Thanks'); await assertSnapshot(result); }); test('agent and subcommand after newline', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', ' \n@agent\n/subCommand Thanks'); + const result = parser.parseChatRequest('1', ' \n@agent\n/subCommand Thanks'); await assertSnapshot(result); }); test('agent not first', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', 'Hello Mr. @agent'); + const result = parser.parseChatRequest('1', 'Hello Mr. @agent'); await assertSnapshot(result); }); test('agents and variables and multiline', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); varService.hasVariable.returns(true); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', '@agent /subCommand \nPlease do with #selection\nand #debugConsole'); + const result = parser.parseChatRequest('1', '@agent /subCommand \nPlease do with #selection\nand #debugConsole'); await assertSnapshot(result); }); test('agents and variables and multiline, part2', async () => { const agentsService = mockObject()({}); - agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); + agentsService.getAgent.returns(getAgentWithSlashcommands([{ name: 'subCommand', description: '' }])); instantiationService.stub(IChatAgentService, agentsService as any); varService.hasVariable.returns(true); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', '@agent Please \ndo /subCommand with #selection\nand #debugConsole'); + const result = parser.parseChatRequest('1', '@agent Please \ndo /subCommand with #selection\nand #debugConsole'); await assertSnapshot(result); }); }); diff --git a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts index 227df61de91..c885bf9444d 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts @@ -227,8 +227,6 @@ suite('Chat', () => { const response = await testService.sendRequest(model.sessionId, `@${chatAgentWithUsedContextId} test request`); assert(response); - await response.responseCompletePromise; - assert.strictEqual(model.getRequests().length, 1); await assertSnapshot(model.toExport());