Skip to content

Conversation

@DonJayamanne
Copy link
Collaborator

For microsoft/vscode#270852
For microsoft/vscode-internalbacklog#5982

@DonJayamanne DonJayamanne self-assigned this Nov 2, 2025
@DonJayamanne DonJayamanne marked this pull request as ready for review November 2, 2025 22:22
Copilot AI review requested due to automatic review settings November 2, 2025 22:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the Copilot CLI tool invocation formatting to improve message clarity and remove unused tracking code. The changes focus on enhancing user-facing messages for different editor commands and cleaning up technical debt.

Key Changes:

  • Removed the toolNames Map tracking that was redundant since tool names are available directly from events
  • Enhanced the StrReplaceEditor tool invocation formatter to provide more specific messages for different commands (view, edit, insert, create, undo)
  • Improved consistency by changing "Read" to "Viewed" for file viewing operations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
copilotcliToolInvocationFormatter.ts Added comprehensive command-specific formatting for StrReplaceEditor tool; removed redundant toolNames tracking and isCopilotCliEditToolCall filtering in createCopilotCLIToolInvocation
copilotcliToolInvocationFormatter.spec.ts Updated tests to reflect removal of toolNames parameter and changed assertion from "Read" to "Viewed"
copilotcliSession.ts Added isCopilotCliEditToolCall import; introduced typo in log message; removed _toolNames usage but left field declaration
copilotcliSessionService.ts Refactored session manager initialization to use Lazy<Promise> for better lazy loading pattern
Comments suppressed due to low confidence (2)

src/extension/agents/copilotcli/node/copilotcliSession.ts:173

  • The _toolNames field is declared but no longer used after removing its usage on lines 184 and 213. This field should be removed as it's now dead code.
	private _toolNames = new Map<string, string>();

src/extension/agents/copilotcli/node/copilotcliSession.ts:213

  • This line attempts to use _toolNames.get() which is no longer populated (the .set() call was removed). This will always return undefined, making toolName always default to ''. Use event.data.toolName directly instead, similar to the fix on line 203.
				const toolName = this._toolNames.get(event.data.toolCallId) || '<unknown>';

@DonJayamanne DonJayamanne added this pull request to the merge queue Nov 2, 2025
Merged via the queue into main with commit 8464add Nov 2, 2025
16 checks passed
@DonJayamanne DonJayamanne deleted the don/numerous-crow branch November 2, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants