From 6490e99d80eca60013b26041da9736b5fd43b5f5 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 17 Oct 2023 01:46:56 -0700 Subject: [PATCH] Don't show suggestions for agent subcommand in an invalid spot after other text (#195752) And clean up the parser a little --- .../browser/contrib/chatInputEditorContrib.ts | 12 ++- .../contrib/chat/common/chatRequestParser.ts | 79 +++++++++--------- ..._agent_and_subcommand_after_newline.0.snap | 82 +++++++++++++++++++ ..._subcommand_with_leading_whitespace.0.snap | 82 +++++++++++++++++++ .../ChatRequestParser_agent_not_first.0.snap | 62 +------------- ...r_agent_with_subcommand_after_text.0.snap} | 32 +------- ..._and_variables_and_multiline__part2.0.snap | 32 +------- .../test/common/chatRequestParser.test.ts | 24 +++++- 8 files changed, 243 insertions(+), 162 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap create mode 100644 src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap rename src/vs/workbench/contrib/chat/test/common/__snapshots__/{ChatRequestParser_agents.0.snap => ChatRequestParser_agent_with_subcommand_after_text.0.snap} (55%) diff --git a/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts b/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts index cb000e55f38..e6d57bf6efb 100644 --- a/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts +++ b/src/vs/workbench/contrib/chat/browser/contrib/chatInputEditorContrib.ts @@ -374,8 +374,8 @@ class AgentCompletions extends Disposable { } const parsedRequest = (await this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue())).parts; - const usedAgent = parsedRequest.find((p): p is ChatRequestAgentPart => p instanceof ChatRequestAgentPart); - if (!usedAgent) { + const usedAgentIdx = parsedRequest.findIndex((p): p is ChatRequestAgentPart => p instanceof ChatRequestAgentPart); + if (usedAgentIdx < 0) { return; } @@ -385,6 +385,14 @@ class AgentCompletions extends Disposable { return; } + for (const partAfterAgent of parsedRequest.slice(usedAgentIdx + 1)) { + if (!(partAfterAgent instanceof ChatRequestTextPart) || !partAfterAgent.text.match(/^\s+(\/\w*)?$/)) { + // No text allowed between agent and subcommand + return; + } + } + + const usedAgent = parsedRequest[usedAgentIdx] as ChatRequestAgentPart; const commands = await usedAgent.agent.provideSlashCommands(token); return { diff --git a/src/vs/workbench/contrib/chat/common/chatRequestParser.ts b/src/vs/workbench/contrib/chat/common/chatRequestParser.ts index 6b322bdea63..9491e40cf77 100644 --- a/src/vs/workbench/contrib/chat/common/chatRequestParser.ts +++ b/src/vs/workbench/contrib/chat/common/chatRequestParser.ts @@ -7,7 +7,7 @@ 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'; -import { IChatAgent, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; +import { IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicReferencePart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart, IParsedChatRequest, IParsedChatRequestPart, chatVariableLeader } from 'vs/workbench/contrib/chat/common/chatParserTypes'; import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables'; @@ -33,14 +33,14 @@ export class ChatRequestParser { const previousChar = message.charAt(i - 1); const char = message.charAt(i); let newPart: IParsedChatRequestPart | undefined; - if (previousChar === ' ' || i === 0) { + if (previousChar.match(/\s/) || i === 0) { if (char === chatVariableLeader) { newPart = this.tryToParseVariable(message.slice(i), i, new Position(lineNumber, column), parts); } else if (char === '@') { - newPart = this.tryToParseAgent(message.slice(i), i, new Position(lineNumber, column), parts); + newPart = this.tryToParseAgent(message.slice(i), message, i, new Position(lineNumber, column), parts); } else if (char === '/') { // TODO try to make this sync - newPart = await this.tryToParseSlashCommand(sessionId, message.slice(i), i, new Position(lineNumber, column), parts); + newPart = await this.tryToParseSlashCommand(sessionId, message.slice(i), message, i, new Position(lineNumber, column), parts); } else if (char === '$') { newPart = await this.tryToParseDynamicVariable(sessionId, message.slice(i), i, new Position(lineNumber, column), parts); } @@ -79,36 +79,13 @@ export class ChatRequestParser { message.slice(lastPartEnd, message.length))); } - - // fix up parts: - // * only one agent at the beginning of the message - // * only one agent command after the agent or at the beginning of the message - let agentIndex = -1; - for (let i = 0; i < parts.length; i++) { - const part = parts[i]; - if (part instanceof ChatRequestAgentPart) { - if (i === 0) { - agentIndex = 0; - } else { - // agent not first -> make text part - parts[i] = new ChatRequestTextPart(part.range, part.editorRange, part.text); - } - } - if (part instanceof ChatRequestAgentSubcommandPart) { - if (!(i === 0 || agentIndex === 0 && i === 2 && /^\s+$/.test(parts[1].text))) { - // agent command not after agent nor first -> make text part - parts[i] = new ChatRequestTextPart(part.range, part.editorRange, part.text); - } - } - } - return { parts, text: message, }; } - private tryToParseAgent(message: string, offset: number, position: IPosition, parts: ReadonlyArray): ChatRequestAgentPart | ChatRequestVariablePart | undefined { + private tryToParseAgent(message: string, fullMessage: string, offset: number, position: IPosition, parts: ReadonlyArray): ChatRequestAgentPart | ChatRequestVariablePart | undefined { const nextVariableMatch = message.match(agentReg); if (!nextVariableMatch) { return; @@ -118,17 +95,29 @@ export class ChatRequestParser { const varRange = new OffsetRange(offset, offset + full.length); const varEditorRange = new Range(position.lineNumber, position.column, position.lineNumber, position.column + full.length); - let agent: IChatAgent | undefined; - if ((agent = this.agentService.getAgent(name))) { - if (parts.some(p => p instanceof ChatRequestAgentPart)) { - // Only one agent allowed - return; - } else { - return new ChatRequestAgentPart(varRange, varEditorRange, agent); - } + const agent = this.agentService.getAgent(name); + if (!agent) { + return; } - return; + if (parts.some(p => p instanceof ChatRequestAgentPart)) { + // Only one agent allowed + return; + } + + // The agent must come first + if (parts.some(p => (p instanceof ChatRequestTextPart && p.text.trim() !== '') || !(p instanceof ChatRequestAgentPart))) { + return; + } + + const previousPart = parts.at(-1); + const previousPartEnd = previousPart?.range.endExclusive ?? 0; + const textSincePreviousPart = fullMessage.slice(previousPartEnd, offset); + if (textSincePreviousPart.trim() !== '') { + return; + } + + return new ChatRequestAgentPart(varRange, varEditorRange, agent); } private tryToParseVariable(message: string, offset: number, position: IPosition, parts: ReadonlyArray): ChatRequestAgentPart | ChatRequestVariablePart | undefined { @@ -149,8 +138,8 @@ export class ChatRequestParser { return; } - private async tryToParseSlashCommand(sessionId: string, message: string, offset: number, position: IPosition, parts: ReadonlyArray): Promise { - const nextSlashMatch = message.match(slashReg); + private async tryToParseSlashCommand(sessionId: string, remainingMessage: string, fullMessage: string, offset: number, position: IPosition, parts: ReadonlyArray): Promise { + const nextSlashMatch = remainingMessage.match(slashReg); if (!nextSlashMatch) { return; } @@ -166,6 +155,18 @@ export class ChatRequestParser { const usedAgent = parts.find((p): p is ChatRequestAgentPart => p instanceof ChatRequestAgentPart); if (usedAgent) { + // The slash command must come immediately after the agent + if (parts.some(p => (p instanceof ChatRequestTextPart && p.text.trim() !== '') || !(p instanceof ChatRequestAgentPart) && !(p instanceof ChatRequestTextPart))) { + return; + } + + const previousPart = parts.at(-1); + const previousPartEnd = previousPart?.range.endExclusive ?? 0; + const textSincePreviousPart = fullMessage.slice(previousPartEnd, offset); + if (textSincePreviousPart.trim() !== '') { + return; + } + const subCommands = await usedAgent.agent.provideSlashCommands(CancellationToken.None); const subCommand = subCommands.find(c => c.name === command); if (subCommand) { 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 new file mode 100644 index 00000000000..7a73d008baa --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_after_newline.0.snap @@ -0,0 +1,82 @@ +{ + parts: [ + { + range: { + start: 0, + endExclusive: 5 + }, + editorRange: { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 2, + endColumn: 1 + }, + text: " \n", + kind: "text" + }, + { + range: { + start: 5, + endExclusive: 11 + }, + editorRange: { + startLineNumber: 2, + startColumn: 1, + endLineNumber: 2, + endColumn: 7 + }, + agent: { + id: "agent", + metadata: { description: "" }, + provideSlashCommands: [Function provideSlashCommands] + }, + kind: "agent" + }, + { + range: { + start: 11, + endExclusive: 12 + }, + editorRange: { + startLineNumber: 2, + startColumn: 7, + endLineNumber: 3, + endColumn: 1 + }, + text: "\n", + kind: "text" + }, + { + range: { + start: 12, + endExclusive: 23 + }, + editorRange: { + startLineNumber: 3, + startColumn: 1, + endLineNumber: 3, + endColumn: 12 + }, + command: { + name: "subCommand", + description: "" + }, + kind: "subcommand" + }, + { + range: { + start: 23, + endExclusive: 30 + }, + editorRange: { + startLineNumber: 3, + startColumn: 12, + endLineNumber: 3, + endColumn: 19 + }, + text: " Thanks", + kind: "text" + } + ], + text: " \n@agent\n/subCommand Thanks" +} \ No newline at end of file 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 new file mode 100644 index 00000000000..ccd7eb870e0 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_and_subcommand_with_leading_whitespace.0.snap @@ -0,0 +1,82 @@ +{ + parts: [ + { + range: { + start: 0, + endExclusive: 10 + }, + editorRange: { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 2, + endColumn: 5 + }, + text: " \r\n\t ", + kind: "text" + }, + { + range: { + start: 10, + endExclusive: 16 + }, + editorRange: { + startLineNumber: 2, + startColumn: 5, + endLineNumber: 2, + endColumn: 11 + }, + agent: { + id: "agent", + metadata: { description: "" }, + provideSlashCommands: [Function provideSlashCommands] + }, + kind: "agent" + }, + { + range: { + start: 16, + endExclusive: 23 + }, + editorRange: { + startLineNumber: 2, + startColumn: 11, + endLineNumber: 3, + endColumn: 5 + }, + text: " \r\n\t ", + kind: "text" + }, + { + range: { + start: 23, + endExclusive: 34 + }, + editorRange: { + startLineNumber: 3, + startColumn: 5, + endLineNumber: 3, + endColumn: 16 + }, + command: { + name: "subCommand", + description: "" + }, + kind: "subcommand" + }, + { + range: { + start: 34, + endExclusive: 41 + }, + editorRange: { + startLineNumber: 3, + startColumn: 16, + endLineNumber: 3, + endColumn: 23 + }, + text: " Thanks", + kind: "text" + } + ], + text: " \r\n\t @agent \r\n\t /subCommand Thanks" +} \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_not_first.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_not_first.0.snap index 0ac17204ee0..29c85351e95 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_not_first.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_not_first.0.snap @@ -3,73 +3,17 @@ { range: { start: 0, - endExclusive: 10 + endExclusive: 16 }, editorRange: { startLineNumber: 1, startColumn: 1, endLineNumber: 1, - endColumn: 11 - }, - text: "Hello Mr. ", - kind: "text" - }, - { - range: { - start: 10, - endExclusive: 16 - }, - editorRange: { - startLineNumber: 1, - startColumn: 11, - endLineNumber: 1, endColumn: 17 }, - text: "@agent", - kind: "text" - }, - { - range: { - start: 16, - endExclusive: 17 - }, - editorRange: { - startLineNumber: 1, - startColumn: 17, - endLineNumber: 1, - endColumn: 18 - }, - text: " ", - kind: "text" - }, - { - range: { - start: 17, - endExclusive: 28 - }, - editorRange: { - startLineNumber: 1, - startColumn: 18, - endLineNumber: 1, - endColumn: 29 - }, - text: "/subCommand", - kind: "text" - }, - { - range: { - start: 28, - endExclusive: 35 - }, - editorRange: { - startLineNumber: 1, - startColumn: 29, - endLineNumber: 1, - endColumn: 36 - }, - text: " thanks", + text: "Hello Mr. @agent", kind: "text" } ], - text: "Hello Mr. @agent /subCommand thanks" + text: "Hello Mr. @agent" } \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap similarity index 55% rename from src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents.0.snap rename to src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap index 8a83800323f..b1954f78a47 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agent_with_subcommand_after_text.0.snap @@ -21,43 +21,15 @@ { range: { start: 6, - endExclusive: 17 + endExclusive: 35 }, editorRange: { startLineNumber: 1, startColumn: 7, endLineNumber: 1, - endColumn: 18 - }, - text: " Please do ", - kind: "text" - }, - { - range: { - start: 17, - endExclusive: 28 - }, - editorRange: { - startLineNumber: 1, - startColumn: 18, - endLineNumber: 1, - endColumn: 29 - }, - text: "/subCommand", - kind: "text" - }, - { - range: { - start: 28, - endExclusive: 35 - }, - editorRange: { - startLineNumber: 1, - startColumn: 29, - endLineNumber: 1, endColumn: 36 }, - text: " thanks", + text: " Please do /subCommand thanks", kind: "text" } ], 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 3708cf78541..310f36005b3 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 @@ -21,43 +21,15 @@ { range: { start: 6, - endExclusive: 18 + endExclusive: 35 }, editorRange: { startLineNumber: 1, startColumn: 7, endLineNumber: 2, - endColumn: 4 - }, - text: " Please \ndo ", - kind: "text" - }, - { - range: { - start: 18, - endExclusive: 29 - }, - editorRange: { - startLineNumber: 2, - startColumn: 4, - endLineNumber: 2, - endColumn: 15 - }, - text: "/subCommand", - kind: "text" - }, - { - range: { - start: 29, - endExclusive: 35 - }, - editorRange: { - startLineNumber: 2, - startColumn: 15, - endLineNumber: 2, endColumn: 21 }, - text: " with ", + text: " Please \ndo /subCommand with ", kind: "text" }, { 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 78317a10496..f3e56c5ff02 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts @@ -109,7 +109,7 @@ suite('ChatRequestParser', () => { await assertSnapshot(result); }); - test('agents', async () => { + test('agent with subcommand after text', async () => { const agentsService = mockObject()({}); agentsService.getAgent.returns(>{ id: 'agent', metadata: { description: '' }, provideSlashCommands: async () => { return [{ name: 'subCommand', description: '' }]; } }); instantiationService.stub(IChatAgentService, agentsService as any); @@ -139,13 +139,33 @@ suite('ChatRequestParser', () => { 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: '' }]; } }); + 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'); + 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: '' }]; } }); + instantiationService.stub(IChatAgentService, agentsService as any); + + parser = instantiationService.createInstance(ChatRequestParser); + const result = await 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: '' }]; } }); instantiationService.stub(IChatAgentService, agentsService as any); parser = instantiationService.createInstance(ChatRequestParser); - const result = await parser.parseChatRequest('1', 'Hello Mr. @agent /subCommand thanks'); + const result = await parser.parseChatRequest('1', 'Hello Mr. @agent'); await assertSnapshot(result); });