tools: remove explanation field from MultiReplaceStringTool#3129
tools: remove explanation field from MultiReplaceStringTool#3129connor4312 merged 2 commits intomainfrom
Conversation
Removes the optional 'explanation' field from the multiReplaceString tool input parameters. This simplifies the tool interface and aligns with the latest tool design patterns. - Removes 'explanation' from package.json tool definition - Removes 'explanation' from IMultiReplaceStringToolParams interface - Updates multiReplaceStringTool to use IAbstractReplaceStringInput directly Fixes microsoft/vscode#289907 (Commit message generated by Copilot)
There was a problem hiding this comment.
Pull request overview
This PR attempts to remove the optional 'explanation' field from the multiReplaceString tool to simplify its interface and align with the latest tool design patterns, addressing issue #289907.
Changes:
- Removes 'explanation' property from individual replacement items in the package.json tool schema
- Changes the replacements array type from IReplaceStringToolParams[] to IAbstractReplaceStringInput[] in the TypeScript interface
- Removes the unused IReplaceStringToolParams import from multiReplaceStringTool.tsx
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Removes explanation property from replacement items schema (lines 826-843) |
| src/extension/tools/node/multiReplaceStringTool.tsx | Updates interface to use IAbstractReplaceStringInput and removes unused import |
Comments suppressed due to low confidence (1)
package.json:843
- The top-level explanation field is still present in the tool's inputSchema. The field is defined at lines 816-819 and is still listed as required at line 849. According to the PR description, the explanation field should be removed entirely from the multiReplaceString tool. You need to:
- Remove the explanation property definition (lines 816-819)
- Remove "explanation" from the required array (line 849)
The diff shown only removes explanation from the replacements items array, but the top-level explanation for the entire multi-replace operation still remains in the schema.
"items": {
"type": "object",
"properties": {
"filePath": {
"type": "string",
"description": "An absolute path to the file to edit."
},
"oldString": {
"type": "string",
"description": "The exact literal text to replace, preferably unescaped. Include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. If this string is not the exact literal text or does not match exactly, this replacement will fail."
},
"newString": {
"type": "string",
"description": "The exact literal text to replace `oldString` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic."
}
},
"required": [
"filePath",
"oldString",
"newString"
]
| export interface IMultiReplaceStringToolParams { | ||
| explanation: string; | ||
| replacements: IReplaceStringToolParams[]; | ||
| replacements: IAbstractReplaceStringInput[]; | ||
| } |
There was a problem hiding this comment.
The explanation field is still present in the IMultiReplaceStringToolParams interface. According to the PR description, this field should be removed. This needs to be changed from:
export interface IMultiReplaceStringToolParams {
explanation: string;
replacements: IAbstractReplaceStringInput[];
}to:
export interface IMultiReplaceStringToolParams {
replacements: IAbstractReplaceStringInput[];
}However, this will cause a compilation error because MultiReplaceStringTool extends AbstractReplaceStringTool which has a generic constraint requiring an explanation field (see abstractReplaceStringTool.tsx line 63: AbstractReplaceStringTool<T extends { explanation: string }>). To fix this properly, you'll also need to either:
- Remove or make optional the generic constraint in AbstractReplaceStringTool
- Or update AbstractReplaceStringTool to not require the explanation field
Additionally, the explanation field is used at line 428 in abstractReplaceStringTool.tsx when calling healReplaceStringParams, so that reference would also need to be updated or made optional.
See below for a potential fix:
replacements: IAbstractReplaceStringInput[];
}
export class MultiReplaceStringTool extends AbstractReplaceStringTool<IMultiReplaceStringToolParams & { explanation: string }> {
Removes the optional 'explanation' field from the multiReplaceString tool
input parameters. This simplifies the tool interface and aligns with the
latest tool design patterns.
Fixes microsoft/vscode#289907
(Commit message generated by Copilot)