From 3122aea7906e54cda35a868a5987eae8db784d54 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 3 Mar 2026 20:15:37 -0800 Subject: [PATCH 1/2] Don't depend on sessionResource in a few places Removes some antipaterns and unused variables. --- .../api/browser/mainThreadChatAgents2.ts | 3 +- .../browser/actions/chatContinueInAction.ts | 5 +- .../chat/browser/attachments/chatVariables.ts | 124 ++++++------ .../contrib/chat/browser/widget/chatWidget.ts | 7 +- .../browser/widget/input/chatInputPart.ts | 6 +- .../input/editor/chatInputEditorContrib.ts | 3 +- .../widget/input/editor/chatPasteProviders.ts | 6 +- .../common/requestParser/chatRequestParser.ts | 11 +- .../browser/attachments/chatVariables.test.ts | 191 ++++++++++++++++++ 9 files changed, 281 insertions(+), 75 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/test/browser/attachments/chatVariables.test.ts diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts index 4fdf590d553729..bf0ba0049b04cd 100644 --- a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts +++ b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts @@ -32,6 +32,7 @@ import { isValidPromptType } from '../../contrib/chat/common/promptSyntax/prompt import { IChatModel } from '../../contrib/chat/common/model/chatModel.js'; import { ChatRequestAgentPart } from '../../contrib/chat/common/requestParser/chatParserTypes.js'; import { ChatRequestParser } from '../../contrib/chat/common/requestParser/chatRequestParser.js'; +import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../../contrib/chat/browser/attachments/chatVariables.js'; import { IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatNotebookEdit, IChatProgress, IChatService, IChatTask, IChatTaskSerialized, IChatWarningMessage } from '../../contrib/chat/common/chatService/chatService.js'; import { IChatSessionsService } from '../../contrib/chat/common/chatSessionsService.js'; import { ChatAgentLocation, ChatModeKind } from '../../contrib/chat/common/constants.js'; @@ -459,7 +460,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA return; } - const parsedRequest = this._instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionResource, model.getValue()).parts; + const parsedRequest = this._instantiationService.createInstance(ChatRequestParser).parseChatRequestWithReferences(getDynamicVariablesForWidget(widget), getSelectedToolAndToolSetsForWidget(widget), model.getValue()).parts; const agentPart = parsedRequest.find((part): part is ChatRequestAgentPart => part instanceof ChatRequestAgentPart); const thisAgentId = this._agents.get(handle)?.id; if (agentPart?.agent.id !== thisAgentId) { diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatContinueInAction.ts b/src/vs/workbench/contrib/chat/browser/actions/chatContinueInAction.ts index 40827f04060d7b..3ab18f156f540e 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatContinueInAction.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatContinueInAction.ts @@ -34,6 +34,7 @@ import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { chatEditingWidgetFileStateContextKey, ModifiedFileEntryState } from '../../common/editing/chatEditingService.js'; import { ChatModel } from '../../common/model/chatModel.js'; import { ChatRequestParser } from '../../common/requestParser/chatRequestParser.js'; +import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../attachments/chatVariables.js'; import { ChatSendResult, IChatService } from '../../common/chatService/chatService.js'; import { IChatSessionsExtensionPoint, IChatSessionsService } from '../../common/chatSessionsService.js'; import { ChatAgentLocation } from '../../common/constants.js'; @@ -403,7 +404,7 @@ export class CreateRemoteAgentJobAction { userPrompt = 'implement this.'; } - const attachedContext = widget.input.getAttachedAndImplicitContext(sessionResource); + const attachedContext = widget.input.getAttachedAndImplicitContext(); widget.input.acceptInput(true); // For inline editor mode, add selection or cursor information @@ -479,7 +480,7 @@ export class CreateRemoteAgentJobAction { const requestParser = instantiationService.createInstance(ChatRequestParser); // Add the request to the model first - const parsedRequest = requestParser.parseChatRequest(sessionResource, userPrompt, ChatAgentLocation.Chat); + const parsedRequest = requestParser.parseChatRequestWithReferences(getDynamicVariablesForWidget(widget), getSelectedToolAndToolSetsForWidget(widget), userPrompt, ChatAgentLocation.Chat); const addedRequest = chatModel.addRequest( parsedRequest, { variables: attachedContext.asArray() }, diff --git a/src/vs/workbench/contrib/chat/browser/attachments/chatVariables.ts b/src/vs/workbench/contrib/chat/browser/attachments/chatVariables.ts index 47e1491204420d..ffdb625d5c9f1d 100644 --- a/src/vs/workbench/contrib/chat/browser/attachments/chatVariables.ts +++ b/src/vs/workbench/contrib/chat/browser/attachments/chatVariables.ts @@ -5,11 +5,72 @@ import { IChatVariablesService, IDynamicVariable } from '../../common/attachments/chatVariables.js'; import { IToolAndToolSetEnablementMap } from '../../common/tools/languageModelToolsService.js'; -import { IChatWidgetService } from '../chat.js'; +import { IChatWidget, IChatWidgetService } from '../chat.js'; import { ChatDynamicVariableModel } from './chatDynamicVariables.js'; import { Range } from '../../../../../editor/common/core/range.js'; import { URI } from '../../../../../base/common/uri.js'; +export function getDynamicVariablesForWidget(widget: IChatWidget): ReadonlyArray { + if (!widget.viewModel || !widget.supportsFileReferences) { + return []; + } + + const model = widget.getContrib(ChatDynamicVariableModel.ID); + if (!model) { + return []; + } + + // track for editing state + if (widget.viewModel.editing && model.variables.length > 0) { + return model.variables; + } + + if (widget.input.attachmentModel.attachments.length > 0 && widget.viewModel.editing) { + const references: IDynamicVariable[] = []; + const editorModel = widget.inputEditor.getModel(); + const modelTextLength = editorModel?.getValueLength() ?? 0; + for (const attachment of widget.input.attachmentModel.attachments) { + // If the attachment has a range, it is a dynamic variable + if (attachment.range) { + if (attachment.range.start >= attachment.range.endExclusive) { + continue; + } + + if (attachment.range.start < 0 || attachment.range.endExclusive > modelTextLength) { + continue; + } + + if (!editorModel) { + continue; + } + + const startPos = editorModel.getPositionAt(attachment.range.start); + const endPos = editorModel.getPositionAt(attachment.range.endExclusive); + + const referenceObj: IDynamicVariable = { + id: attachment.id, + fullName: attachment.name, + modelDescription: attachment.modelDescription, + range: new Range(startPos.lineNumber, startPos.column, endPos.lineNumber, endPos.column), + icon: attachment.icon, + isFile: attachment.kind === 'file', + isDirectory: attachment.kind === 'directory', + data: attachment.value + }; + references.push(referenceObj); + } + } + + return references.length > 0 ? references : model.variables; + } + + return model.variables; +} + +export function getSelectedToolAndToolSetsForWidget(widget: IChatWidget): IToolAndToolSetEnablementMap { + return widget.input.selectedToolsModel.entriesMap.get(); +} + export class ChatVariablesService implements IChatVariablesService { declare _serviceBrand: undefined; @@ -18,65 +79,11 @@ export class ChatVariablesService implements IChatVariablesService { ) { } getDynamicVariables(sessionResource: URI): ReadonlyArray { - // This is slightly wrong... the parser pulls dynamic references from the input widget, but there is no guarantee that message came from the input here. - // Need to ... - // - Parser takes list of dynamic references (annoying) - // - Or the parser is known to implicitly act on the input widget, and we need to call it before calling the chat service (maybe incompatible with the future, but easy) const widget = this.chatWidgetService.getWidgetBySessionResource(sessionResource); - if (!widget || !widget.viewModel || !widget.supportsFileReferences) { - return []; - } - - const model = widget.getContrib(ChatDynamicVariableModel.ID); - if (!model) { + if (!widget) { return []; } - - // track for editing state - if (widget.viewModel.editing && model.variables.length > 0) { - return model.variables; - } - - if (widget.input.attachmentModel.attachments.length > 0 && widget.viewModel.editing) { - const references: IDynamicVariable[] = []; - const editorModel = widget.inputEditor.getModel(); - const modelTextLength = editorModel?.getValueLength() ?? 0; - for (const attachment of widget.input.attachmentModel.attachments) { - // If the attachment has a range, it is a dynamic variable - if (attachment.range) { - if (attachment.range.start >= attachment.range.endExclusive) { - continue; - } - - if (attachment.range.start < 0 || attachment.range.endExclusive > modelTextLength) { - continue; - } - - if (!editorModel) { - continue; - } - - const startPos = editorModel.getPositionAt(attachment.range.start); - const endPos = editorModel.getPositionAt(attachment.range.endExclusive); - - const referenceObj: IDynamicVariable = { - id: attachment.id, - fullName: attachment.name, - modelDescription: attachment.modelDescription, - range: new Range(startPos.lineNumber, startPos.column, endPos.lineNumber, endPos.column), - icon: attachment.icon, - isFile: attachment.kind === 'file', - isDirectory: attachment.kind === 'directory', - data: attachment.value - }; - references.push(referenceObj); - } - } - - return references.length > 0 ? references : model.variables; - } - - return model.variables; + return getDynamicVariablesForWidget(widget); } getSelectedToolAndToolSets(sessionResource: URI): IToolAndToolSetEnablementMap { @@ -84,7 +91,6 @@ export class ChatVariablesService implements IChatVariablesService { if (!widget) { return new Map(); } - return widget.input.selectedToolsModel.entriesMap.get(); - + return getSelectedToolAndToolSetsForWidget(widget); } } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index 7c172e77ee98f2..09a203bd5b06c2 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -56,6 +56,7 @@ import { IChatModel, IChatModelInputState, IChatResponseModel } from '../../comm import { ChatMode, getModeNameForTelemetry, IChatModeService } from '../../common/chatModes.js'; import { chatAgentLeader, ChatRequestAgentPart, ChatRequestDynamicVariablePart, ChatRequestSlashPromptPart, ChatRequestToolPart, ChatRequestToolSetPart, chatSubcommandLeader, formatChatQuestion, IParsedChatRequest } from '../../common/requestParser/chatParserTypes.js'; import { ChatRequestParser } from '../../common/requestParser/chatRequestParser.js'; +import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../attachments/chatVariables.js'; import { ChatRequestQueueKind, ChatSendResult, IChatLocationData, IChatSendRequestOptions, IChatService } from '../../common/chatService/chatService.js'; import { IChatSessionsService } from '../../common/chatSessionsService.js'; import { IChatSlashCommandService } from '../../common/participants/chatSlashCommands.js'; @@ -333,7 +334,7 @@ export class ChatWidget extends Disposable implements IChatWidget { } this.parsedChatRequest = this.instantiationService.createInstance(ChatRequestParser) - .parseChatRequest(this.viewModel.sessionResource, this.getInput(), this.location, { + .parseChatRequestWithReferences(getDynamicVariablesForWidget(this), getSelectedToolAndToolSetsForWidget(this), this.getInput(), this.location, { selectedAgent: this._lastSelectedAgent, mode: this.input.currentModeKind, attachmentCapabilities: this.attachmentCapabilities, @@ -854,7 +855,7 @@ export class ChatWidget extends Disposable implements IChatWidget { } const previous = this.parsedChatRequest; - this.parsedChatRequest = this.instantiationService.createInstance(ChatRequestParser).parseChatRequest(this.viewModel.sessionResource, this.getInput(), this.location, { selectedAgent: this._lastSelectedAgent, mode: this.input.currentModeKind, attachmentCapabilities: this.attachmentCapabilities }); + this.parsedChatRequest = this.instantiationService.createInstance(ChatRequestParser).parseChatRequestWithReferences(getDynamicVariablesForWidget(this), getSelectedToolAndToolSetsForWidget(this), this.getInput(), this.location, { selectedAgent: this._lastSelectedAgent, mode: this.input.currentModeKind, attachmentCapabilities: this.attachmentCapabilities }); if (!previous || !IParsedChatRequest.equals(previous, this.parsedChatRequest)) { this._onDidChangeParsedInput.fire(); } @@ -2218,7 +2219,7 @@ export class ChatWidget extends Disposable implements IChatWidget { const editorValue = this.getInput(); const requestInputs: IChatRequestInputOptions = { input: !query ? editorValue : query.query, - attachedContext: options?.enableImplicitContext === false ? this.input.getAttachedContext(this.viewModel.sessionResource) : this.input.getAttachedAndImplicitContext(this.viewModel.sessionResource), + attachedContext: options?.enableImplicitContext === false ? this.input.getAttachedContext() : this.input.getAttachedAndImplicitContext(), }; const isUserQuery = !query; diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts index 1a4b90632fb524..87a0817bba6e54 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts @@ -248,15 +248,15 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge readonly selectedToolsModel: ChatSelectedTools; - public getAttachedContext(sessionResource: URI) { + public getAttachedContext() { const contextArr = new ChatRequestVariableSet(); contextArr.add(...this.attachmentModel.attachments, ...this.chatContextService.getWorkspaceContextItems()); return contextArr; } - public getAttachedAndImplicitContext(sessionResource: URI): ChatRequestVariableSet { + public getAttachedAndImplicitContext(): ChatRequestVariableSet { - const contextArr = this.getAttachedContext(sessionResource); + const contextArr = this.getAttachedContext(); if (this.implicitContext) { const implicitChatVariables = this.implicitContext.enabledBaseEntries(this.configurationService.getValue('chat.implicitContext.suggestedContext')); diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts index ce838a3e0a146e..180f17414abb24 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts @@ -20,6 +20,7 @@ import { IChatAgentCommand, IChatAgentData, IChatAgentService } from '../../../. import { chatSlashCommandBackground, chatSlashCommandForeground } from '../../../../common/widget/chatColors.js'; import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicVariablePart, ChatRequestSlashCommandPart, ChatRequestSlashPromptPart, ChatRequestTextPart, ChatRequestToolPart, ChatRequestToolSetPart, IParsedChatRequestPart, chatAgentLeader, chatSubcommandLeader } from '../../../../common/requestParser/chatParserTypes.js'; import { ChatRequestParser } from '../../../../common/requestParser/chatRequestParser.js'; +import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../../../attachments/chatVariables.js'; import { IPromptsService } from '../../../../common/promptSyntax/service/promptsService.js'; import { IChatWidget } from '../../../chat.js'; import { ChatWidget } from '../../chatWidget.js'; @@ -411,7 +412,7 @@ 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) { const attachmentCapabilities = previousSelectedAgent?.capabilities ?? this.widget.attachmentCapabilities; - const previousParsedValue = parser.parseChatRequest(this.widget.viewModel.sessionResource, previousInputValue, widget.location, { selectedAgent: previousSelectedAgent, mode: this.widget.input.currentModeKind, attachmentCapabilities }); + const previousParsedValue = parser.parseChatRequestWithReferences(getDynamicVariablesForWidget(this.widget), getSelectedToolAndToolSetsForWidget(this.widget), previousInputValue, widget.location, { selectedAgent: previousSelectedAgent, mode: this.widget.input.currentModeKind, attachmentCapabilities }); // 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 ChatRequestSlashPromptPart || p instanceof ChatRequestToolPart); diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.ts b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.ts index 65089b2020086c..eafcf7aa1c6012 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.ts @@ -24,8 +24,9 @@ import { IInstantiationService } from '../../../../../../../platform/instantiati import { ILogService } from '../../../../../../../platform/log/common/log.js'; import { IExtensionService, isProposedApiEnabled } from '../../../../../../services/extensions/common/extensions.js'; import { IChatRequestPasteVariableEntry, IChatRequestVariableEntry } from '../../../../common/attachments/chatVariableEntries.js'; -import { IChatVariablesService, IDynamicVariable } from '../../../../common/attachments/chatVariables.js'; +import { IDynamicVariable } from '../../../../common/attachments/chatVariables.js'; import { IChatWidgetService } from '../../../chat.js'; +import { getDynamicVariablesForWidget } from '../../../attachments/chatVariables.js'; import { ChatDynamicVariableModel } from '../../../attachments/chatDynamicVariables.js'; import { cleanupOldImages, createFileForMedia, resizeImage } from '../../../chatImageUtils.js'; @@ -201,7 +202,6 @@ class CopyAttachmentsProvider implements DocumentPasteEditProvider { constructor( @IChatWidgetService private readonly chatWidgetService: IChatWidgetService, - @IChatVariablesService private readonly chatVariableService: IChatVariablesService ) { } async prepareDocumentPaste(model: ITextModel, _ranges: readonly IRange[], _dataTransfer: IReadonlyVSDataTransfer, _token: CancellationToken): Promise { @@ -212,7 +212,7 @@ class CopyAttachmentsProvider implements DocumentPasteEditProvider { } const attachments = widget.attachmentModel.attachments; - const dynamicVariables = this.chatVariableService.getDynamicVariables(widget.viewModel.sessionResource); + const dynamicVariables = getDynamicVariablesForWidget(widget); if (attachments.length === 0 && dynamicVariables.length === 0) { return undefined; diff --git a/src/vs/workbench/contrib/chat/common/requestParser/chatRequestParser.ts b/src/vs/workbench/contrib/chat/common/requestParser/chatRequestParser.ts index 9ec0508a489f23..bf1214267a518e 100644 --- a/src/vs/workbench/contrib/chat/common/requestParser/chatRequestParser.ts +++ b/src/vs/workbench/contrib/chat/common/requestParser/chatRequestParser.ts @@ -12,7 +12,7 @@ import { ChatAgentLocation, ChatModeKind } from '../constants.js'; import { IChatAgentAttachmentCapabilities, IChatAgentData, IChatAgentService } from '../participants/chatAgents.js'; import { IChatSlashCommandService } from '../participants/chatSlashCommands.js'; import { IPromptsService } from '../promptSyntax/service/promptsService.js'; -import { IToolData, IToolSet, isToolSet } from '../tools/languageModelToolsService.js'; +import { IToolAndToolSetEnablementMap, IToolData, IToolSet, isToolSet } from '../tools/languageModelToolsService.js'; import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestDynamicVariablePart, ChatRequestSlashCommandPart, ChatRequestSlashPromptPart, ChatRequestTextPart, ChatRequestToolPart, ChatRequestToolSetPart, IParsedChatRequest, IParsedChatRequestPart, chatAgentLeader, chatSubcommandLeader, chatVariableLeader } from './chatParserTypes.js'; const agentReg = /^@([\w_\-\.]+)(?=(\s|$|\b))/i; // An @-agent @@ -37,11 +37,16 @@ export class ChatRequestParser { ) { } parseChatRequest(sessionResource: URI, message: string, location: ChatAgentLocation = ChatAgentLocation.Chat, context?: IChatParserContext): IParsedChatRequest { - const parts: IParsedChatRequestPart[] = []; const references = this.variableService.getDynamicVariables(sessionResource); // must access this list before any async calls + const selectedToolAndToolSets = this.variableService.getSelectedToolAndToolSets(sessionResource); + return this.parseChatRequestWithReferences(references, selectedToolAndToolSets, message, location, context); + } + + parseChatRequestWithReferences(references: ReadonlyArray, selectedToolAndToolSets: IToolAndToolSetEnablementMap, message: string, location: ChatAgentLocation = ChatAgentLocation.Chat, context?: IChatParserContext): IParsedChatRequest { + const parts: IParsedChatRequestPart[] = []; const toolsByName = new Map(); const toolSetsByName = new Map(); - for (const [entry, enabled] of this.variableService.getSelectedToolAndToolSets(sessionResource)) { + for (const [entry, enabled] of selectedToolAndToolSets) { if (enabled) { if (isToolSet(entry)) { toolSetsByName.set(entry.referenceName, entry); diff --git a/src/vs/workbench/contrib/chat/test/browser/attachments/chatVariables.test.ts b/src/vs/workbench/contrib/chat/test/browser/attachments/chatVariables.test.ts new file mode 100644 index 00000000000000..59e05f0411bb7a --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/attachments/chatVariables.test.ts @@ -0,0 +1,191 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { Range } from '../../../../../../editor/common/core/range.js'; +import { IDynamicVariable } from '../../../common/attachments/chatVariables.js'; +import { IChatWidget } from '../../../browser/chat.js'; +import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../../../browser/attachments/chatVariables.js'; +import { ChatDynamicVariableModel } from '../../../browser/attachments/chatDynamicVariables.js'; +import { IChatRequestVariableEntry } from '../../../common/attachments/chatVariableEntries.js'; +import { IToolData, IToolSet, ToolDataSource } from '../../../common/tools/languageModelToolsService.js'; +import { observableValue } from '../../../../../../base/common/observable.js'; + +function createMockVariable(overrides?: Partial): IDynamicVariable { + return { + id: 'var-1', + fullName: 'test-var', + range: new Range(1, 1, 1, 10), + data: 'test-data', + ...overrides, + }; +} + +function createMockAttachment(overrides?: Partial): IChatRequestVariableEntry { + return { + id: 'attach-1', + name: 'test-attachment', + kind: 'file', + value: 'test-value', + ...overrides, + } as IChatRequestVariableEntry; +} + +function createMockWidget(options: { + hasViewModel?: boolean; + supportsFileReferences?: boolean; + contribVariables?: IDynamicVariable[]; + editing?: boolean; + attachments?: IChatRequestVariableEntry[]; + editorTextLength?: number; +}): IChatWidget { + const { + hasViewModel = true, + supportsFileReferences = true, + contribVariables = [], + editing = false, + attachments = [], + editorTextLength = 100, + } = options; + + const contribModel = { + id: ChatDynamicVariableModel.ID, + variables: contribVariables, + }; + + return { + viewModel: hasViewModel ? { editing: editing ? {} : undefined } : undefined, + supportsFileReferences, + getContrib: (id: string) => id === ChatDynamicVariableModel.ID ? contribModel : undefined, + input: { + attachmentModel: { attachments }, + }, + inputEditor: { + getModel: () => ({ + getValueLength: () => editorTextLength, + getPositionAt: (offset: number) => ({ lineNumber: 1, column: offset + 1 }), + }), + }, + } as unknown as IChatWidget; +} + +suite('getDynamicVariablesForWidget', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('returns empty when no viewModel', () => { + const widget = createMockWidget({ hasViewModel: false }); + assert.deepStrictEqual(getDynamicVariablesForWidget(widget), []); + }); + + test('returns empty when file references not supported', () => { + const widget = createMockWidget({ supportsFileReferences: false }); + assert.deepStrictEqual(getDynamicVariablesForWidget(widget), []); + }); + + test('returns contrib model variables when not editing', () => { + const variables = [createMockVariable()]; + const widget = createMockWidget({ contribVariables: variables }); + assert.deepStrictEqual(getDynamicVariablesForWidget(widget), variables); + }); + + test('returns contrib model variables when editing with existing variables', () => { + const variables = [createMockVariable()]; + const widget = createMockWidget({ editing: true, contribVariables: variables }); + assert.deepStrictEqual(getDynamicVariablesForWidget(widget), variables); + }); + + test('converts attachments to dynamic variables when editing with attachments and no contrib variables', () => { + const attachments = [ + createMockAttachment({ + id: 'a1', + name: 'file.ts', + kind: 'file', + value: 'file-value', + range: { start: 0, endExclusive: 8 }, + }), + ]; + const widget = createMockWidget({ editing: true, attachments, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].id, 'a1'); + assert.strictEqual(result[0].fullName, 'file.ts'); + assert.strictEqual(result[0].isFile, true); + assert.strictEqual(result[0].isDirectory, false); + assert.strictEqual(result[0].data, 'file-value'); + }); + + test('skips attachments without range when editing', () => { + const attachments = [createMockAttachment({ range: undefined })]; + const widget = createMockWidget({ editing: true, attachments, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + + // No ranged attachments, falls back to contrib model variables (empty) + assert.deepStrictEqual(result, []); + }); + + test('skips attachments with empty range', () => { + const attachments = [createMockAttachment({ range: { start: 5, endExclusive: 5 } })]; + const widget = createMockWidget({ editing: true, attachments, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + assert.deepStrictEqual(result, []); + }); + + test('skips attachments with out-of-bounds range', () => { + const attachments = [createMockAttachment({ range: { start: 0, endExclusive: 200 } })]; + const widget = createMockWidget({ editing: true, attachments, editorTextLength: 100, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + assert.deepStrictEqual(result, []); + }); + + test('skips attachments with negative start', () => { + const attachments = [createMockAttachment({ range: { start: -1, endExclusive: 5 } })]; + const widget = createMockWidget({ editing: true, attachments, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + assert.deepStrictEqual(result, []); + }); + + test('sets isDirectory for directory attachments', () => { + const attachments = [ + createMockAttachment({ + kind: 'directory', + range: { start: 0, endExclusive: 5 }, + }), + ]; + const widget = createMockWidget({ editing: true, attachments, contribVariables: [] }); + const result = getDynamicVariablesForWidget(widget); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].isFile, false); + assert.strictEqual(result[0].isDirectory, true); + }); +}); + +suite('getSelectedToolAndToolSetsForWidget', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('returns the entriesMap from the selected tools model', () => { + const toolData: IToolData = { + id: 'tool-1', + toolReferenceName: 'myTool', + displayName: 'My Tool', + modelDescription: 'A test tool', + canBeReferencedInPrompt: true, + source: ToolDataSource.Internal, + }; + const expectedMap = new Map([[toolData, true]]); + const entriesMap = observableValue('test', expectedMap); + + const widget = { + input: { + selectedToolsModel: { entriesMap }, + }, + } as unknown as IChatWidget; + + const result = getSelectedToolAndToolSetsForWidget(widget); + assert.strictEqual(result, expectedMap); + }); +}); From 636a3b5ed91fc3f051941a94254108963279a445 Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt <2644648+TylerLeonhardt@users.noreply.github.com> Date: Tue, 3 Mar 2026 20:26:20 -0800 Subject: [PATCH 2/2] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../chat/browser/widget/input/editor/chatInputEditorContrib.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts index 180f17414abb24..0bf73505e4e6f1 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputEditorContrib.ts @@ -412,7 +412,7 @@ 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) { const attachmentCapabilities = previousSelectedAgent?.capabilities ?? this.widget.attachmentCapabilities; - const previousParsedValue = parser.parseChatRequestWithReferences(getDynamicVariablesForWidget(this.widget), getSelectedToolAndToolSetsForWidget(this.widget), previousInputValue, widget.location, { selectedAgent: previousSelectedAgent, mode: this.widget.input.currentModeKind, attachmentCapabilities }); + const previousParsedValue = parser.parseChatRequestWithReferences(getDynamicVariablesForWidget(this.widget), getSelectedToolAndToolSetsForWidget(this.widget), previousInputValue, this.widget.location, { selectedAgent: previousSelectedAgent, mode: this.widget.input.currentModeKind, attachmentCapabilities }); // 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 ChatRequestSlashPromptPart || p instanceof ChatRequestToolPart);