Get rid of history2, variables2 (#205124)

* Get rid of history2

* Remove prompt2 and variables2

* Clean up variables2/prompt2

* Fix tests

* Fix ranges

* Fix test

extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts

@@ -64,8 +64,8 @@ suite('chat', () => {
 		const deferred = getDeferredForRequest();
 		interactive.sendInteractiveRequestToProvider('provider', { message: '@agent hi #myVar' });
 		const request = await deferred.p;
-		assert.strictEqual(request.prompt, 'hi [#myVar](values:myVar)');
-		assert.strictEqual(request.variables['myVar'][0].value, 'myValue');
+		assert.strictEqual(request.prompt, 'hi #myVar');
+		assert.strictEqual(request.variables[0].values[0].value, 'myValue');
 	});
 
 	test('result metadata is returned to the followup provider', async () => {

src/vs/workbench/api/common/extHostChatAgents2.ts

@@ -200,7 +200,7 @@ export class ExtHostChatAgents2 implements ExtHostChatAgentsShape2 {
 			const convertedHistory = await this.prepareHistoryTurns(request, context);
 			const task = agent.invoke(
 				typeConvert.ChatAgentRequest.to(request),
-				{ history: convertedHistory, history2: convertedHistory },
+				{ history: convertedHistory },
 				stream.apiObject,
 				token
 			);
@@ -239,7 +239,7 @@ export class ExtHostChatAgents2 implements ExtHostChatAgentsShape2 {
 				{ ...ehResult, metadata: undefined };
 
 			// REQUEST turn
-			res.push(new extHostTypes.ChatAgentRequestTurn(h.request.message, h.request.command, h.request.variables2.variables.map(typeConvert.ChatAgentResolvedVariable.to), { extensionId: '', agentId: h.request.agentId }));
+			res.push(new extHostTypes.ChatAgentRequestTurn(h.request.message, h.request.command, h.request.variables.variables.map(typeConvert.ChatAgentResolvedVariable.to), { extensionId: '', agentId: h.request.agentId }));
 
 			// RESPONSE turn
 			const parts = coalesce(h.response.map(r => typeConvert.ChatResponsePart.from(r, this.commands.converter)));

src/vs/workbench/api/common/extHostTypeConverters.ts

@@ -2631,11 +2631,8 @@ export namespace ChatAgentRequest {
 	export function to(request: IChatAgentRequest): vscode.ChatAgentRequest {
 		return {
 			prompt: request.message,
-			prompt2: request.variables2.message,
-			variables: ChatVariable.objectTo(request.variables),
 			command: request.command,
-			agentId: request.agentId,
-			variables2: request.variables2.variables.map(ChatAgentResolvedVariable.to)
+			variables: request.variables.variables.map(ChatAgentResolvedVariable.to)
 		};
 	}
 }

src/vs/workbench/contrib/chat/browser/chatVariables.ts

@@ -3,10 +3,12 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { coalesce } from 'vs/base/common/arrays';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { onUnexpectedExternalError } from 'vs/base/common/errors';
 import { Iterable } from 'vs/base/common/iterator';
 import { IDisposable, toDisposable } from 'vs/base/common/lifecycle';
+import { IOffsetRange } from 'vs/editor/common/core/offsetRange';
 import { IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
 import { ChatDynamicVariableModel } from 'vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables';
 import { IChatModel, IChatRequestVariableData } from 'vs/workbench/contrib/chat/common/chatModel';
@@ -29,51 +31,37 @@ export class ChatVariablesService implements IChatVariablesService {
 	}
 
 	async resolveVariables(prompt: IParsedChatRequest, model: IChatModel, token: CancellationToken): Promise<IChatRequestVariableData> {
-		const resolvedVariables: Record<string, IChatRequestVariableValue[]> = {};
+		let resolvedVariables: { name: string; range: IOffsetRange; values: IChatRequestVariableValue[] }[] = [];
 		const jobs: Promise<any>[] = [];
 
-		const parsedPrompt: string[] = [];
 		prompt.parts
 			.forEach((part, i) => {
 				if (part instanceof ChatRequestVariablePart) {
 					const data = this._resolver.get(part.variableName.toLowerCase());
 					if (data) {
-						jobs.push(data.resolver(prompt.text, part.variableArg, model, token).then(value => {
-							if (value) {
-								resolvedVariables[part.variableName] = value;
-								parsedPrompt[i] = `[${part.text}](values:${part.variableName})`;
-							} else {
-								parsedPrompt[i] = part.promptText;
+						jobs.push(data.resolver(prompt.text, part.variableArg, model, token).then(values => {
+							if (values?.length) {
+								resolvedVariables[i] = { name: part.variableName, range: part.range, values };
 							}
 						}).catch(onUnexpectedExternalError));
 					}
 				} else if (part instanceof ChatRequestDynamicVariablePart) {
-					const referenceName = this.getUniqueReferenceName(part.referenceText, resolvedVariables);
-					resolvedVariables[referenceName] = part.data;
-					const safeText = part.text.replace(/[\[\]]/g, '_');
-					const safeTarget = referenceName.replace(/[\(\)]/g, '_');
-					parsedPrompt[i] = `[${safeText}](values:${safeTarget})`;
-				} else {
-					parsedPrompt[i] = part.promptText;
+					resolvedVariables[i] = { name: part.referenceText, range: part.range, values: part.data };
 				}
 			});
 
 		await Promise.allSettled(jobs);
 
+		resolvedVariables = coalesce(resolvedVariables);
+
+		// "reverse", high index first so that replacement is simple
+		resolvedVariables.sort((a, b) => b.range.start - a.range.start);
+
 		return {
 			variables: resolvedVariables,
-			message: parsedPrompt.join('').trim()
 		};
 	}
 
-	private getUniqueReferenceName(name: string, vars: Record<string, any>): string {
-		let i = 1;
-		while (vars[name]) {
-			name = `${name}_${i++}`;
-		}
-		return name;
-	}
-
 	hasVariable(name: string): boolean {
 		return this._resolver.has(name.toLowerCase());
 	}

src/vs/workbench/contrib/chat/common/chatAgents.ts

@@ -13,9 +13,8 @@ import { URI } from 'vs/base/common/uri';
 import { ProviderResult } from 'vs/editor/common/languages';
 import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { IChatProgressResponseContent, IChatRequestVariableData2 } from 'vs/workbench/contrib/chat/common/chatModel';
+import { IChatProgressResponseContent, IChatRequestVariableData } from 'vs/workbench/contrib/chat/common/chatModel';
 import { IChatFollowup, IChatProgress, IChatResponseErrorDetails } from 'vs/workbench/contrib/chat/common/chatService';
-import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';
 
 //#region agent service, commands etc
 
@@ -89,8 +88,7 @@ export interface IChatAgentRequest {
 	agentId: string;
 	command?: string;
 	message: string;
-	variables: Record<string, IChatRequestVariableValue[]>;
-	variables2: IChatRequestVariableData2;
+	variables: IChatRequestVariableData;
 }
 
 export interface IChatAgentResult {

src/vs/workbench/contrib/chat/common/chatModel.ts

@@ -20,16 +20,6 @@ import { IChat, IChatAgentMarkdownContentWithVulnerability, IChatCommandButton,
 import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';
 
 export interface IChatRequestVariableData {
-	/**
-	 * The user's message with variable references for extension API.
-	 */
-	message: string;
-
-	variables: Record<string, IChatRequestVariableValue[]>;
-}
-
-export interface IChatRequestVariableData2 {
-	message: string;
 	variables: { name: string; range: IOffsetRange; values: IChatRequestVariableValue[] }[];
 }
 
@@ -574,8 +564,10 @@ export class ChatModel extends Disposable implements IChatModel {
 						? this.getParsedRequestFromString(raw.message)
 						: reviveParsedChatRequest(raw.message);
 
-				// Only old messages don't have variableData
-				const variableData: IChatRequestVariableData = raw.variableData ?? { message: parsedRequest.text, variables: {} };
+				// Old messages don't have variableData, or have it in the wrong (non-array) shape
+				const variableData: IChatRequestVariableData = raw.variableData && Array.isArray(raw.variableData.variables)
+					? raw.variableData :
+					{ variables: [] };
 				const request = new ChatRequestModel(this, parsedRequest, variableData);
 				if (raw.response || raw.result || (raw as any).responseErrorDetails) {
 					const agent = (raw.agent && 'metadata' in raw.agent) ? // Check for the new format, ignore entries in the old format

src/vs/workbench/contrib/chat/common/chatParserTypes.ts

@@ -26,8 +26,11 @@ export interface IParsedChatRequestPart {
 	readonly promptText: string;
 }
 
-export function getPromptText(request: ReadonlyArray<IParsedChatRequestPart>): string {
-	return request.map(r => r.promptText).join('');
+export function getPromptText(request: IParsedChatRequest): { message: string; diff: number } {
+	const message = request.parts.map(r => r.promptText).join('').trimStart();
+	const diff = request.text.length - message.length;
+
+	return { message, diff };
 }
 
 export class ChatRequestTextPart implements IParsedChatRequestPart {

src/vs/workbench/contrib/chat/common/chatServiceImpl.ts

@@ -22,8 +22,8 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IChatAgentHistoryEntry, IChatAgentRequest, IChatAgentResult, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
 import { CONTEXT_PROVIDER_EXISTS } from 'vs/workbench/contrib/chat/common/chatContextKeys';
-import { ChatModel, ChatModelInitState, ChatRequestModel, ChatWelcomeMessageModel, IChatModel, IChatRequestVariableData, IChatRequestVariableData2, ISerializableChatData, ISerializableChatsData } from 'vs/workbench/contrib/chat/common/chatModel';
-import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicVariablePart, ChatRequestSlashCommandPart, ChatRequestVariablePart, IParsedChatRequest, getPromptText } from 'vs/workbench/contrib/chat/common/chatParserTypes';
+import { ChatModel, ChatModelInitState, ChatRequestModel, ChatWelcomeMessageModel, IChatModel, IChatRequestVariableData, ISerializableChatData, ISerializableChatsData } from 'vs/workbench/contrib/chat/common/chatModel';
+import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestSlashCommandPart, IParsedChatRequest, getPromptText } from 'vs/workbench/contrib/chat/common/chatParserTypes';
 import { ChatMessageRole, IChatMessage } from 'vs/workbench/contrib/chat/common/chatProvider';
 import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser';
 import { ChatAgentCopyKind, IChat, IChatCompleteResponse, IChatDetail, IChatDynamicRequest, IChatFollowup, IChatProgress, IChatProvider, IChatProviderInfo, IChatSendRequestData, IChatService, IChatTransferredSessionData, IChatUserActionEvent, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService';
@@ -537,38 +537,38 @@ export class ChatService extends Disposable implements IChatService {
 							continue;
 						}
 
+						const promptTextResult = getPromptText(request.message);
 						const historyRequest: IChatAgentRequest = {
 							sessionId,
 							requestId: request.id,
 							agentId: request.response.agent?.id ?? '',
-							message: request.variableData.message,
-							variables: request.variableData.variables,
+							message: promptTextResult.message,
 							command: request.response.slashCommand?.name,
-							variables2: asVariablesData2(request.message, request.variableData)
+							variables: updateRanges(request.variableData, promptTextResult.diff) // TODO bit of a hack
 						};
 						history.push({ request: historyRequest, response: request.response.response.value, result: request.response.result ?? {} });
 					}
 
-					const initVariableData: IChatRequestVariableData = { message: getPromptText(parsedRequest.parts), variables: {} };
+					const initVariableData: IChatRequestVariableData = { variables: [] };
 					request = model.addRequest(parsedRequest, initVariableData, agent, agentSlashCommandPart?.command);
 					const variableData = await this.chatVariablesService.resolveVariables(parsedRequest, model, token);
 					request.variableData = variableData;
 
+					const promptTextResult = getPromptText(request.message);
 					const requestProps: IChatAgentRequest = {
 						sessionId,
 						requestId: request.id,
 						agentId: agent.id,
-						message: variableData.message,
-						variables: variableData.variables,
+						message: promptTextResult.message,
 						command: agentSlashCommandPart?.command.name,
-						variables2: asVariablesData2(parsedRequest, variableData)
+						variables: updateRanges(variableData, promptTextResult.diff) // TODO bit of a hack
 					};
 
 					const agentResult = await this.chatAgentService.invokeAgent(agent.id, requestProps, progressCallback, history, token);
 					rawResult = agentResult;
 					agentOrCommandFollowups = this.chatAgentService.getFollowups(agent.id, requestProps, agentResult, followupsCancelToken);
 				} else if (commandPart && this.chatSlashCommandService.hasCommand(commandPart.slashCommand.command)) {
-					request = model.addRequest(parsedRequest, { message: parsedRequest.text, variables: {} });
+					request = model.addRequest(parsedRequest, { variables: [] });
 					// contributed slash commands
 					// TODO: spell this out in the UI
 					const history: IChatMessage[] = [];
@@ -670,7 +670,7 @@ export class ChatService extends Disposable implements IChatService {
 		const parsedRequest = typeof message === 'string' ?
 			this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(sessionId, message) :
 			message;
-		const request = model.addRequest(parsedRequest, variableData || { message: parsedRequest.text, variables: {} });
+		const request = model.addRequest(parsedRequest, variableData || { variables: [] });
 		if (typeof response.message === 'string') {
 			model.acceptResponseProgress(request, { content: response.message, kind: 'content' });
 		} else {
@@ -765,25 +765,14 @@ export class ChatService extends Disposable implements IChatService {
 	}
 }
 
-function asVariablesData2(parsedRequest: IParsedChatRequest, variableData: IChatRequestVariableData): IChatRequestVariableData2 {
-
-	const res: IChatRequestVariableData2 = {
-		message: getPromptText(parsedRequest.parts),
-		variables: []
+function updateRanges(variableData: IChatRequestVariableData, diff: number): IChatRequestVariableData {
+	return {
+		variables: variableData.variables.map(v => ({
+			...v,
+			range: {
+				start: v.range.start - diff,
+				endExclusive: v.range.endExclusive - diff
+			}
+		}))
 	};
-
-	for (const part of parsedRequest.parts) {
-		if (part instanceof ChatRequestVariablePart) {
-			const values = variableData.variables[part.variableName];
-			res.variables.push({ name: part.variableName, range: part.range, values });
-		} else if (part instanceof ChatRequestDynamicVariablePart) {
-			// Need variable without `#`
-			res.variables.push({ name: part.referenceText, range: part.range, values: part.data });
-		}
-	}
-
-	// "reverse", high index first so that replacement is simple
-	res.variables.sort((a, b) => b.range.start - a.range.start);
-
-	return res;
 }

src/vs/workbench/contrib/chat/test/browser/chatVariables.test.ts

@@ -41,48 +41,44 @@ suite('ChatVariables', function () {
 		const parser = instantiationService.createInstance(ChatRequestParser);
 
 		const resolveVariables = async (text: string) => {
-			const result = await parser.parseChatRequest('1', text);
+			const result = parser.parseChatRequest('1', text);
 			return await service.resolveVariables(result, null!, CancellationToken.None);
 		};
 
 		{
 			const data = await resolveVariables('Hello #foo and#far');
-			assert.strictEqual(Object.keys(data.variables).length, 1);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.message, 'Hello [#foo](values:foo) and#far');
+			assert.strictEqual(data.variables.length, 1);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo']);
 		}
 		{
 			const data = await resolveVariables('#foo Hello');
-			assert.strictEqual(Object.keys(data.variables).length, 1);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.message, '[#foo](values:foo) Hello');
+			assert.strictEqual(data.variables.length, 1);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo']);
 		}
 		{
 			const data = await resolveVariables('Hello #foo');
-			assert.strictEqual(Object.keys(data.variables).length, 1);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
+			assert.strictEqual(data.variables.length, 1);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo']);
 		}
 		{
 			const data = await resolveVariables('Hello #foo?');
-			assert.strictEqual(Object.keys(data.variables).length, 1);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.message, 'Hello [#foo](values:foo)?');
+			assert.strictEqual(data.variables.length, 1);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo']);
 		}
 		{
 			const data = await resolveVariables('Hello #foo and#far #foo');
-			assert.strictEqual(Object.keys(data.variables).length, 1);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
+			assert.strictEqual(data.variables.length, 2);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo', 'foo']);
 		}
 		{
 			const data = await resolveVariables('Hello #foo and #far #foo');
-			assert.strictEqual(Object.keys(data.variables).length, 2);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['far', 'foo']);
+			assert.strictEqual(data.variables.length, 3);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo', 'far', 'foo']);
 		}
 		{
 			const data = await resolveVariables('Hello #foo and #far #foo #unknown');
-			assert.strictEqual(Object.keys(data.variables).length, 2);
-			assert.deepEqual(Object.keys(data.variables).sort(), ['far', 'foo']);
-			assert.strictEqual(data.message, 'Hello [#foo](values:foo) and [#far](values:far) [#foo](values:foo) #unknown');
+			assert.strictEqual(data.variables.length, 3);
+			assert.deepEqual(data.variables.map(v => v.name), ['foo', 'far', 'foo']);
 		}
 
 		v1.dispose();

src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_deserialize.0.snap

@@ -42,10 +42,7 @@
           }
         ]
       },
-      variableData: {
-        message: "@ChatProviderWithUsedContext test request",
-        variables: {  }
-      },
+      variableData: { variables: [  ] },
       response: [  ],
       result: { metadata: { metadataKey: "value" } },
       followups: undefined,

src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap

@@ -42,10 +42,7 @@
         ],
         text: "@ChatProviderWithUsedContext test request"
       },
-      variableData: {
-        message: "@ChatProviderWithUsedContext test request",
-        variables: {  }
-      },
+      variableData: { variables: [  ] },
       response: [  ],
       result: { metadata: { metadataKey: "value" } },
       followups: undefined,

src/vs/workbench/contrib/chat/test/common/chatModel.test.ts

@@ -111,7 +111,7 @@ suite('ChatModel', () => {
 		model.startInitialize();
 		model.initialize({} as any, undefined);
 		const text = 'hello';
-		model.addRequest({ text, parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, text.length, 1, text.length), text)] }, { message: text, variables: {} });
+		model.addRequest({ text, parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, text.length, 1, text.length), text)] }, { variables: [] });
 		const requests = model.getRequests();
 		assert.strictEqual(requests.length, 1);
 

src/vs/workbench/contrib/chat/test/common/chatService.test.ts

@@ -131,11 +131,11 @@ suite('Chat', () => {
 
 		const session1 = testDisposables.add(testService.startSession('provider1', CancellationToken.None));
 		await session1.waitForInitialization();
-		session1.addRequest({ parts: [], text: 'request 1' }, { message: 'request 1', variables: {} });
+		session1.addRequest({ parts: [], text: 'request 1' }, { variables: [] });
 
 		const session2 = testDisposables.add(testService.startSession('provider2', CancellationToken.None));
 		await session2.waitForInitialization();
-		session2.addRequest({ parts: [], text: 'request 2' }, { message: 'request 2', variables: {} });
+		session2.addRequest({ parts: [], text: 'request 2' }, { variables: [] });
 
 		storageService.flush();
 		const testService2 = testDisposables.add(instantiationService.createInstance(ChatService));

src/vs/workbench/contrib/chat/test/common/mockChatVariables.ts

@@ -29,8 +29,7 @@ export class MockChatVariablesService implements IChatVariablesService {
 
 	async resolveVariables(prompt: IParsedChatRequest, model: IChatModel, token: CancellationToken): Promise<IChatRequestVariableData> {
 		return {
-			message: prompt.text,
-			variables: {}
+			variables: []
 		};
 	}
 }

src/vscode-dts/vscode.proposed.chatAgents2.d.ts

@@ -31,7 +31,7 @@ declare module 'vscode' {
 		/**
 		 * The prompt as entered by the user.
 		 *
-		 * Information about variables used in this request are is stored in {@link ChatAgentRequest.variables2}.
+		 * Information about variables used in this request are is stored in {@link ChatAgentRequest.variables}.
 		 *
 		 * *Note* that the {@link ChatAgent2.name name} of the agent and the {@link ChatAgentCommand.name command}
 		 * are not part of the prompt.
@@ -88,11 +88,6 @@ declare module 'vscode' {
 		 * All of the chat messages so far in the current chat session.
 		 */
 		readonly history: ReadonlyArray<ChatAgentRequestTurn | ChatAgentResponseTurn>;
-
-		/**
-		 * @deprecated, use histroy
-		 */
-		readonly history2: ReadonlyArray<ChatAgentRequestTurn | ChatAgentResponseTurn>;
 	}
 
 	/**
@@ -325,41 +320,25 @@ declare module 'vscode' {
 
 	export interface ChatAgentRequest {
 
-		/**
-		 * The prompt entered by the user. The {@link ChatAgent2.name name} of the agent or the {@link ChatAgentCommand.name command}
-		 * are not part of the prompt.
-		 *
-		 * @see {@link ChatAgentRequest.command}
-		 */
-		prompt: string;
-
 		/**
 		 * The prompt as entered by the user.
 		 *
-		 * Information about variables used in this request are is stored in {@link ChatAgentRequest.variables2}.
+		 * Information about variables used in this request are is stored in {@link ChatAgentRequest.variables}.
 		 *
 		 * *Note* that the {@link ChatAgent2.name name} of the agent and the {@link ChatAgentCommand.name command}
 		 * are not part of the prompt.
 		 */
-		prompt2: string;
-
-		/**
-		 * The ID of the chat agent to which this request was directed.
-		 */
-		agentId: string;
+		prompt: string;
 
 		/**
 		 * The name of the {@link ChatAgentCommand command} that was selected for this request.
 		 */
 		command?: string;
 
-		/** @deprecated */
-		variables: Record<string, ChatVariableValue[]>;
-
 		/**
-		 *
+		 * The list of variables that are referenced in the prompt.
 		 */
-		variables2: ChatAgentResolvedVariable[];
+		variables: ChatAgentResolvedVariable[];
 	}
 
 	export interface ChatAgentResponseStream {