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

Fix retrieving dynamic references for chat agents #196320

Merged
merged 4 commits into from Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -26,7 +26,7 @@ export class ChatDynamicReferenceModel extends Disposable implements IChatWidget

private readonly _references: IDynamicReference[] = [];
get references(): ReadonlyArray<IDynamicReference> {
return this._references;
return [...this._references];
}

get id() {
Expand Down Expand Up @@ -126,7 +126,7 @@ export class SelectAndInsertFileAction extends Action2 {

const fileName = basename(resource);
const editor = context.widget.inputEditor;
const text = `$file:${fileName}`;
const text = `#file:${fileName}`;
const range = context.range;
const success = editor.executeEdits('chatInsertFile', [{ range, text: text + ' ' }]);
if (!success) {
Expand Down
Expand Up @@ -458,7 +458,7 @@ class AgentCompletions extends Disposable {
}

class BuiltinDynamicCompletions extends Disposable {
private static readonly VariableNameDef = /\$\w*/g; // MUST be using `g`-flag
private static readonly VariableNameDef = new RegExp(`${chatVariableLeader}\\w*`, 'g'); // MUST be using `g`-flag

constructor(
@ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService,
Expand All @@ -470,7 +470,7 @@ class BuiltinDynamicCompletions extends Disposable {

this._register(this.languageFeaturesService.completionProvider.register({ scheme: ChatInputPart.INPUT_SCHEME, hasAccessToAllModels: true }, {
_debugDisplayName: 'chatDynamicCompletions',
triggerCharacters: ['$'],
triggerCharacters: [chatVariableLeader],
provideCompletionItems: async (model: ITextModel, position: Position, _context: CompletionContext, _token: CancellationToken) => {
const fileVariablesEnabled = this.configurationService.getValue('chat.experimental.fileVariables') ?? this.productService.quality !== 'stable';
if (!fileVariablesEnabled) {
Expand Down Expand Up @@ -501,8 +501,8 @@ class BuiltinDynamicCompletions extends Disposable {
return <CompletionList>{
suggestions: [
<CompletionItem>{
label: '$file',
insertText: '$file:',
label: `${chatVariableLeader}file`,
insertText: `${chatVariableLeader}file:`,
detail: localize('pickFileLabel', "Pick a file"),
range: { insert, replace },
kind: CompletionItemKind.Text,
Expand Down
9 changes: 6 additions & 3 deletions src/vs/workbench/contrib/chat/browser/media/chat.css
Expand Up @@ -54,16 +54,19 @@

@keyframes ellipsis {
0% {
content: "";
}
25% {
content: ".";
}
33% {
50% {
content: "..";
}
66% {
75% {
content: "...";
}
100% {
content: ".";
content: "";
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/vs/workbench/contrib/chat/common/chatParserTypes.ts
Expand Up @@ -66,7 +66,7 @@ export class ChatRequestAgentPart implements IParsedChatRequestPart {
constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly agent: IChatAgent) { }

get text(): string {
return `@${this.agent.id}`;
return `${chatAgentLeader}${this.agent.id}`;
}

get promptText(): string {
Expand All @@ -83,7 +83,7 @@ export class ChatRequestAgentSubcommandPart implements IParsedChatRequestPart {
constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly command: IChatAgentCommand) { }

get text(): string {
return `/${this.command.name}`;
return `${chatVariableLeader}${this.command.name}`;
}

get promptText(): string {
Expand All @@ -100,16 +100,16 @@ export class ChatRequestSlashCommandPart implements IParsedChatRequestPart {
constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly slashCommand: ISlashCommand) { }

get text(): string {
return `/${this.slashCommand.command}`;
return `${chatSubcommandLeader}${this.slashCommand.command}`;
}

get promptText(): string {
return `/${this.slashCommand.command}`;
return `${chatSubcommandLeader}${this.slashCommand.command}`;
}
}

/**
* An invocation of a dynamic reference like '$file:'
* An invocation of a dynamic reference like '#file:'
*/
export class ChatRequestDynamicReferencePart implements IParsedChatRequestPart {
static readonly Kind = 'dynamic';
Expand All @@ -121,7 +121,7 @@ export class ChatRequestDynamicReferencePart implements IParsedChatRequestPart {
}

get text(): string {
return `$${this.referenceText}`;
return `${chatVariableLeader}${this.referenceText}`;
}

get promptText(): string {
Expand Down
25 changes: 11 additions & 14 deletions src/vs/workbench/contrib/chat/common/chatRequestParser.ts
Expand Up @@ -8,14 +8,14 @@ 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 { 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 { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicReferencePart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart, IParsedChatRequest, IParsedChatRequestPart, chatAgentLeader, chatSubcommandLeader, 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';
import { IChatVariablesService, IDynamicReference } from 'vs/workbench/contrib/chat/common/chatVariables';

const agentReg = /^@([\w_\-]+)(?=(\s|$|\b))/i; // An @-agent
const variableReg = /^#([\w_\-]+)(:\d+)?(?=(\s|$|\b))/i; // A #-variable with an optional numeric : arg (@response:2)
const slashReg = /\/([\w_\-]+)(?=(\s|$|\b))/i; // A / command
const dollarSignVarReg = /\$([\w_\-]+):([\w_\-\.]+)(?=(\s|$|\b))/i; // A / command
const variableWithArgReg = /\#([\w_\-]+):([\w_\-\.]+)(?=(\s|$|\b))/i; // A variable with a string : arg (#file:foo.ts)

export class ChatRequestParser {
constructor(
Expand All @@ -26,6 +26,7 @@ export class ChatRequestParser {

async parseChatRequest(sessionId: string, message: string): Promise<IParsedChatRequest> {
const parts: IParsedChatRequestPart[] = [];
const references = this.variableService.getDynamicReferences(sessionId); // must access this list before any async calls

let lineNumber = 1;
let column = 1;
Expand All @@ -35,14 +36,13 @@ export class ChatRequestParser {
let newPart: IParsedChatRequestPart | undefined;
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.tryToParseVariable(message.slice(i), i, new Position(lineNumber, column), parts) ||
await this.tryToParseDynamicVariable(message.slice(i), i, new Position(lineNumber, column), references);
} else if (char === chatAgentLeader) {
newPart = this.tryToParseAgent(message.slice(i), message, i, new Position(lineNumber, column), parts);
} else if (char === '/') {
} 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);
} else if (char === '$') {
newPart = await this.tryToParseDynamicVariable(sessionId, message.slice(i), i, new Position(lineNumber, column), parts);
}
}

Expand Down Expand Up @@ -185,8 +185,8 @@ export class ChatRequestParser {
return;
}

private async tryToParseDynamicVariable(sessionId: string, message: string, offset: number, position: IPosition, parts: ReadonlyArray<IParsedChatRequestPart>): Promise<ChatRequestDynamicReferencePart | undefined> {
const nextVarMatch = message.match(dollarSignVarReg);
private async tryToParseDynamicVariable(message: string, offset: number, position: IPosition, references: ReadonlyArray<IDynamicReference>): Promise<ChatRequestDynamicReferencePart | undefined> {
const nextVarMatch = message.match(variableWithArgReg);
if (!nextVarMatch) {
return;
}
Expand All @@ -200,12 +200,9 @@ export class ChatRequestParser {
return;
}

const references = this.variableService.getDynamicReferences(sessionId);
const refAtThisPosition = references.find(r =>
r.range.startLineNumber === position.lineNumber &&
r.range.startColumn === position.column &&
r.range.endLineNumber === position.lineNumber &&
r.range.endColumn === position.column + full.length);
r.range.startColumn === position.column);
if (refAtThisPosition) {
return new ChatRequestDynamicReferencePart(range, editorRange, name, arg, refAtThisPosition.data);
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
Expand Up @@ -678,9 +678,9 @@ export class ChatService extends Disposable implements IChatService {
}
}

async sendRequestToProvider(sessionId: string, message: IChatDynamicRequest): Promise<void> {
async sendRequestToProvider(sessionId: string, message: IChatDynamicRequest): Promise<{ responseCompletePromise: Promise<void> } | undefined> {
this.trace('sendRequestToProvider', `sessionId: ${sessionId}`);
await this.sendRequest(sessionId, message.message);
return await this.sendRequest(sessionId, message.message);
}

getProviders(): string[] {
Expand Down
Expand Up @@ -13,6 +13,7 @@ import { ChatVariablesService } from 'vs/workbench/contrib/chat/browser/chatVari
import { ChatAgentService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser';
import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables';
import { MockChatWidgetService } from 'vs/workbench/contrib/chat/test/browser/mockChatWidget';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';

Expand All @@ -23,7 +24,7 @@ suite('ChatVariables', function () {
const testDisposables = ensureNoDisposablesAreLeakedInTestSuite();

setup(function () {
service = new ChatVariablesService(null!);
service = new ChatVariablesService(new MockChatWidgetService());
instantiationService = testDisposables.add(new TestInstantiationService());
instantiationService.stub(IStorageService, testDisposables.add(new TestStorageService()));
instantiationService.stub(ILogService, new NullLogService());
Expand Down
31 changes: 31 additions & 0 deletions src/vs/workbench/contrib/chat/test/browser/mockChatWidget.ts
@@ -0,0 +1,31 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { URI } from 'vs/base/common/uri';
import { IChatWidget, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';

export class MockChatWidgetService implements IChatWidgetService {
readonly _serviceBrand: undefined;

/**
* Returns the most recently focused widget if any.
*/
readonly lastFocusedWidget: IChatWidget | undefined;

/**
* Returns whether a view was successfully revealed.
*/
async revealViewForProvider(providerId: string): Promise<IChatWidget | undefined> {
return undefined;
}

getWidgetByInputUri(uri: URI): IChatWidget | undefined {
return undefined;
}

getWidgetBySessionId(sessionId: string): IChatWidget | undefined {
return undefined;
}
}
Expand Up @@ -28,6 +28,10 @@ suite('ChatRequestParser', () => {
instantiationService.stub(ILogService, new NullLogService());
instantiationService.stub(IExtensionService, new TestExtensionService());
instantiationService.stub(IChatAgentService, testDisposables.add(instantiationService.createInstance(ChatAgentService)));

const varService = mockObject<IChatVariablesService>()({});
varService.getDynamicReferences.returns([]);
instantiationService.stub(IChatVariablesService, varService as any);
});

test('plain text', async () => {
Expand Down
Expand Up @@ -230,7 +230,8 @@ suite('Chat', () => {
const model = testDisposables.add(testService.startSession('testProvider', CancellationToken.None));
assert.strictEqual(model.getRequests().length, 0);

await testService.sendRequestToProvider(model.sessionId, { message: 'test request' });
const response = await testService.sendRequestToProvider(model.sessionId, { message: 'test request' });
await response?.responseCompletePromise;
assert.strictEqual(model.getRequests().length, 1);
});

Expand Down
Expand Up @@ -24,7 +24,7 @@ export class MockChatVariablesService implements IChatVariablesService {
}

getDynamicReferences(sessionId: string): readonly IDynamicReference[] {
throw new Error('Method not implemented.');
return [];
}

async resolveVariables(prompt: IParsedChatRequest, model: IChatModel, token: CancellationToken): Promise<IChatVariableResolveResult> {
Expand Down