Skip to content

Commit d920fc1

Browse files
authored
tools: remove old experiments (#244)
Remove a trio of experiments that were successful and have been rolled out (Claude exclusive string replace, apply_patch, and apply_patch's indentation normalization).
1 parent 13f2e9a commit d920fc1

File tree

5 files changed

+22
-63
lines changed

5 files changed

+22
-63
lines changed

src/extension/intents/node/agentIntent.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { BudgetExceededError } from '@vscode/prompt-tsx/dist/base/materialized';
1010
import type * as vscode from 'vscode';
1111
import { ChatLocation, ChatResponse } from '../../../platform/chat/common/commonTypes';
1212
import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService';
13-
import { modelMightUseReplaceStringExclusively, modelSupportsApplyPatch, modelSupportsReplaceString } from '../../../platform/endpoint/common/chatModelCapabilities';
13+
import { modelCanUseReplaceStringExclusively, modelSupportsApplyPatch, modelSupportsReplaceString } from '../../../platform/endpoint/common/chatModelCapabilities';
1414
import { IEndpointProvider } from '../../../platform/endpoint/common/endpointProvider';
1515
import { IEnvService } from '../../../platform/env/common/envService';
1616
import { ILogService } from '../../../platform/log/common/logService';
@@ -57,16 +57,12 @@ const getTools = (instaService: IInstantiationService, request: vscode.ChatReque
5757
const endpointProvider = accessor.get<IEndpointProvider>(IEndpointProvider);
5858
const model = await endpointProvider.getChatEndpoint(request);
5959

60-
// Claude: replace_string AND insert_edits
61-
// 4.1/o4-mini: apply_patch AND insert_edits
6260
const allowTools: Record<string, boolean> = {};
63-
const applyPatchConfigEnabled = configurationService.getExperimentBasedConfig<boolean>(ConfigKey.Internal.EnableApplyPatchTool, experimentationService); // (can't use extension exp config in package.json "when" clause)
64-
const useApplyPatch = !!(modelSupportsApplyPatch(model) && applyPatchConfigEnabled && toolsService.getTool(ToolName.ApplyPatch));
6561
allowTools[ToolName.EditFile] = true;
6662
allowTools[ToolName.ReplaceString] = modelSupportsReplaceString(model) || !!(model.family.includes('gemini') && experimentationService.getTreatmentVariable<boolean>('vscode', 'copilotchat.geminiReplaceString'));
67-
allowTools[ToolName.ApplyPatch] = useApplyPatch;
63+
allowTools[ToolName.ApplyPatch] = modelSupportsApplyPatch(model) && !!toolsService.getTool(ToolName.ApplyPatch);
6864

69-
if (modelMightUseReplaceStringExclusively(model) && experimentationService.getTreatmentVariable<boolean>('vscode', 'copilotchat.claudeReplaceStringExclusively')) {
65+
if (modelCanUseReplaceStringExclusively(model)) {
7066
allowTools[ToolName.ReplaceString] = true;
7167
allowTools[ToolName.EditFile] = false;
7268
}

src/extension/tools/node/applyPatch/parser.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ export class Parser {
169169
patch: Patch = { actions: {} };
170170
fuzz = 0;
171171

172-
constructor(currentFiles: Record<string, AbstractDocumentWithLanguageId | TextDocument>, lines: Array<string>, private readonly fixIndentationDuringMatch: boolean) {
172+
constructor(currentFiles: Record<string, AbstractDocumentWithLanguageId | TextDocument>, lines: Array<string>) {
173173
this.current_files = currentFiles;
174174
this.lines = lines;
175175
for (const [path, doc] of Object.entries(currentFiles)) {
@@ -416,10 +416,7 @@ export class Parser {
416416
}
417417

418418
ch.insLines = ch.insLines.map(replace_explicit_tabs);
419-
420-
if (this.fixIndentationDuringMatch) {
421-
ch.insLines = ch.insLines.map(ins => isFalsyOrWhitespace(ins) ? ins : additionalIndentation + transformIndentation(ins, srcIndentStyle, targetIndentStyle));
422-
}
419+
ch.insLines = ch.insLines.map(ins => isFalsyOrWhitespace(ins) ? ins : additionalIndentation + transformIndentation(ins, srcIndentStyle, targetIndentStyle));
423420

424421
if (match.fuzz & Fuzz.NormalizedExplicitTab) {
425422
ch.delLines = ch.delLines.map(replace_explicit_tabs);
@@ -763,7 +760,6 @@ function peek_next_section(
763760
export function text_to_patch(
764761
text: string,
765762
orig: Record<string, AbstractDocumentWithLanguageId | TextDocument>,
766-
fixIndentationDuringMatch = true,
767763
): [Patch, number] {
768764
const lines = text.trim().split("\n");
769765
if (lines.length < 2) {
@@ -777,7 +773,7 @@ export function text_to_patch(
777773
if (lines[lines.length - 1] !== patchSuffix) {
778774
lines.push(patchSuffix);
779775
}
780-
const parser = new Parser(orig, lines, fixIndentationDuringMatch);
776+
const parser = new Parser(orig, lines);
781777
parser.index = 1;
782778
parser.parse();
783779
return [parser.patch, parser.fuzz];
@@ -921,13 +917,12 @@ export function apply_commit(
921917
export async function processPatch(
922918
text: string,
923919
openFn: (p: string) => Promise<AbstractDocumentWithLanguageId | TextDocument>,
924-
fixIndentationDuringMatch: boolean,
925920
): Promise<Commit> {
926921
if (!text.startsWith(PATCH_PREFIX)) {
927922
throw new InvalidPatchFormatError("Patch must start with *** Begin Patch\\n", 'patchMustStartWithBeginPatch');
928923
}
929924
const paths = identify_files_needed(text);
930925
const orig = await load_files(paths, openFn);
931-
const [patch, _fuzz] = text_to_patch(text, orig, fixIndentationDuringMatch);
926+
const [patch, _fuzz] = text_to_patch(text, orig);
932927
return patch_to_commit(patch, orig);
933928
}

src/extension/tools/node/applyPatchTool.tsx

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import { ActionType, Commit, DiffError, FileChange, InvalidContextError, Invalid
4747
import { EditFileResult, IEditedFile } from './editFileToolResult';
4848
import { sendEditNotebookTelemetry } from './editNotebookTool';
4949
import { assertFileOkForTool, resolveToolInputPath } from './toolUtils';
50-
import { guessIndentation, normalizeIndentation } from '../../prompt/node/indentationGuesser';
5150

5251
export const applyPatchWithNotebookSupportDescription: vscode.LanguageModelToolInformation = {
5352
name: ToolName.ApplyPatch,
@@ -126,39 +125,10 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
126125
return trailingEmptyLines;
127126
}
128127

129-
130-
/**
131-
* Normalize the indentation of the content to match the original document's style
132-
* @param document The original document
133-
* @param content The content to normalize
134-
* @returns The normalized content
135-
*/
136-
private normalizeIndentationStyle(document: vscode.TextDocument, content: string): string {
137-
// Detect the indentation style of the original document
138-
const indentInfo = guessIndentation(document, 4, true);
139-
140-
// Split into lines, normalize each line, and then join back
141-
const lines = content.split('\n');
142-
const normalizedLines = lines.map(line => {
143-
// Only process lines that have indentation
144-
if (line.match(/^\s+/)) {
145-
return normalizeIndentation(line, indentInfo.tabSize, indentInfo.insertSpaces);
146-
}
147-
return line;
148-
});
149-
150-
return normalizedLines.join('\n');
151-
}
152-
153-
154-
private async generateUpdateTextDocumentEdit(file: string, change: FileChange, workspaceEdit: WorkspaceEdit, applyIndentationFix: boolean) {
128+
private async generateUpdateTextDocumentEdit(file: string, change: FileChange, workspaceEdit: WorkspaceEdit) {
155129
const uri = resolveToolInputPath(file, this.promptPathRepresentationService);
156130
const textDocument = await this.workspaceService.openTextDocument(uri);
157-
let newContent = removeLeadingFilepathComment(change.newContent ?? '', textDocument.languageId, file);
158-
159-
if (!applyIndentationFix) { // the 'old way' of normalizing the edit once it's created
160-
newContent = this.normalizeIndentationStyle(textDocument, newContent);
161-
}
131+
const newContent = removeLeadingFilepathComment(change.newContent ?? '', textDocument.languageId, file);
162132

163133
const lines = newContent?.split('\n') ?? [];
164134
let path = uri;
@@ -242,13 +212,11 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
242212
throw new Error('Missing patch text or stream');
243213
}
244214

245-
const useIndentationFix = !!this.experimentationService.getTreatmentVariable<boolean>('vscode', 'copilotchat.applyPatchIndentationFix');
246-
247215
let commit: Commit | undefined;
248216
let healed: string | undefined;
249217
const docText: DocText = {};
250218
try {
251-
({ commit, healed } = await this.buildCommitWithHealing(options.input.input, docText, options.input.explanation, useIndentationFix, token));
219+
({ commit, healed } = await this.buildCommitWithHealing(options.input.input, docText, options.input.explanation, token));
252220
} catch (error) {
253221
if (error instanceof HealedError) {
254222
healed = error.healedPatch;
@@ -335,7 +303,7 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
335303
}
336304
}
337305
else {
338-
path = await this.generateUpdateTextDocumentEdit(file, changes, workspaceEdit, useIndentationFix);
306+
path = await this.generateUpdateTextDocumentEdit(file, changes, workspaceEdit);
339307
}
340308
resourceToOperation.set(path, ActionType.UPDATE);
341309
break;
@@ -502,9 +470,9 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
502470
return patchEnd === -1 ? fetchResult.value.slice(patchStart) : fetchResult.value.slice(patchStart, patchEnd + PATCH_SUFFIX.length);
503471
}
504472

505-
private async buildCommitWithHealing(patch: string, docText: DocText, explanation: string, useIndentationFix: boolean, token: CancellationToken): Promise<{ commit: Commit; healed?: string }> {
473+
private async buildCommitWithHealing(patch: string, docText: DocText, explanation: string, token: CancellationToken): Promise<{ commit: Commit; healed?: string }> {
506474
try {
507-
return await this.buildCommit(patch, docText, useIndentationFix);
475+
return await this.buildCommit(patch, docText);
508476
} catch (error) {
509477
if (!(error instanceof DiffError)) {
510478
throw error;
@@ -519,7 +487,7 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
519487
throw error;
520488
}
521489

522-
const { commit } = await this.buildCommit(healed, docText, useIndentationFix);
490+
const { commit } = await this.buildCommit(healed, docText);
523491
return { commit, healed };
524492
} catch (healedError) {
525493
success = false;
@@ -543,7 +511,7 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
543511
}
544512
}
545513

546-
private async buildCommit(patch: string, docText: DocText, useIndentationFix: boolean): Promise<{ commit: Commit }> {
514+
private async buildCommit(patch: string, docText: DocText): Promise<{ commit: Commit }> {
547515
const commit = await processPatch(patch, async (uri) => {
548516
const vscodeUri = resolveToolInputPath(uri, this.promptPathRepresentationService);
549517
if (this.notebookService.hasSupportedNotebooks(vscodeUri)) {
@@ -556,7 +524,7 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
556524
docText[vscodeUri.toString()] = { text: textDocument.getText() };
557525
return textDocument;
558526
}
559-
}, useIndentationFix);
527+
});
560528
return { commit };
561529
}
562530

src/extension/tools/node/test/applyPatch.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ plt.title('Correlation Heatmap')
119119
plt.show()
120120
</VSCode.Cell>`;
121121

122-
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)), true);
122+
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)));
123123
expect(patch).toBeDefined();
124124
});
125125

@@ -175,7 +175,7 @@ import sys
175175
#%% vscode.cell [id=d7161d69] [language=python]
176176
sys.executable`;
177177

178-
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)), true);
178+
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)));
179179
expect(patch).toBeDefined();
180180
});
181181

@@ -206,7 +206,7 @@ Hello
206206
#%% vscode.cell [id=05e875f9] [language=python]
207207
print(1)`;
208208

209-
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)), true);
209+
const patch = await processPatch(input, () => Promise.resolve(new StringTextDocumentWithLanguageId('xml', notebookContent)));
210210
expect(patch).toBeDefined();
211211
});
212212
it('Has same details as defined in package.json', async () => {

src/platform/endpoint/common/chatModelCapabilities.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ export function modelSupportsReplaceString(model: LanguageModelChat | IChatEndpo
3737
}
3838

3939
/**
40-
* Currently (@connor4312) running an experiment to see if we should use
41-
* replace string exclusively for these models.
40+
* The model is capable of using replace_string_in_file exclusively,
41+
* without needing insert_edit_into_file.
4242
*/
43-
export function modelMightUseReplaceStringExclusively(model: LanguageModelChat | IChatEndpoint): boolean {
43+
export function modelCanUseReplaceStringExclusively(model: LanguageModelChat | IChatEndpoint): boolean {
4444
return model.family.startsWith('claude') || model.family.startsWith('Anthropic');
4545
}

0 commit comments

Comments
 (0)