fix(coding-agent): simplify edit tool input closes #2639

This commit is contained in:
Mario Zechner
2026-03-28 22:10:34 +01:00
parent 576e5e1a2f
commit e773527b3a
4 changed files with 52 additions and 145 deletions

View File

@@ -70,7 +70,7 @@ export interface FuzzyMatchResult {
contentForReplacement: string; contentForReplacement: string;
} }
export interface ReplaceEdit { export interface Edit {
oldText: string; oldText: string;
newText: string; newText: string;
} }
@@ -192,7 +192,7 @@ function getNoChangeError(path: string, totalEdits: number): Error {
*/ */
export function applyEditsToNormalizedContent( export function applyEditsToNormalizedContent(
normalizedContent: string, normalizedContent: string,
edits: ReplaceEdit[], edits: Edit[],
path: string, path: string,
): AppliedEditsResult { ): AppliedEditsResult {
const normalizedEdits = edits.map((edit) => ({ const normalizedEdits = edits.map((edit) => ({
@@ -403,7 +403,7 @@ export interface EditDiffError {
*/ */
export async function computeEditsDiff( export async function computeEditsDiff(
path: string, path: string,
edits: ReplaceEdit[], edits: Edit[],
cwd: string, cwd: string,
): Promise<EditDiffResult | EditDiffError> { ): Promise<EditDiffResult | EditDiffError> {
const absolutePath = resolveToCwd(path, cwd); const absolutePath = resolveToCwd(path, cwd);

View File

@@ -9,11 +9,11 @@ import {
applyEditsToNormalizedContent, applyEditsToNormalizedContent,
computeEditsDiff, computeEditsDiff,
detectLineEnding, detectLineEnding,
type Edit,
type EditDiffError, type EditDiffError,
type EditDiffResult, type EditDiffResult,
generateDiffString, generateDiffString,
normalizeToLF, normalizeToLF,
type ReplaceEdit,
restoreLineEndings, restoreLineEndings,
stripBom, stripBom,
} from "./edit-diff.js"; } from "./edit-diff.js";
@@ -41,46 +41,16 @@ const replaceEditSchema = Type.Object(
const editSchema = Type.Object( const editSchema = Type.Object(
{ {
path: Type.String({ description: "Path to the file to edit (relative or absolute)" }), path: Type.String({ description: "Path to the file to edit (relative or absolute)" }),
oldText: Type.Optional( edits: Type.Array(replaceEditSchema, {
Type.String({ description:
description: "One or more targeted replacements. Each edit is matched against the original file, not incrementally. Do not include overlapping or nested edits. If two changes touch the same block or nearby lines, merge them into one edit instead.",
"Exact text to replace for one contiguous change. Include only enough surrounding context to make the match unique. Do not use this to cover multiple distant changes in one large block.", }),
}),
),
newText: Type.Optional(
Type.String({
description: "Replacement text for oldText in single-replacement mode.",
}),
),
edits: Type.Optional(
Type.Array(replaceEditSchema, {
description:
"Use this when changing multiple separate, disjoint regions in the same file. Each edit is matched against the original file, not incrementally. Do not include overlapping or nested edits. If two changes touch the same block or nearby lines, merge them into one edit instead.",
}),
),
}, },
{ additionalProperties: false }, { additionalProperties: false },
); );
export type EditToolInput = Static<typeof editSchema>; export type EditToolInput = Static<typeof editSchema>;
interface ReplaceModeInput {
path: string;
oldText: string;
newText: string;
}
interface MultiReplaceModeInput {
path: string;
edits: ReplaceEdit[];
}
interface NormalizedEditInput {
path: string;
edits: ReplaceEdit[];
mode: "single" | "multi";
}
export interface EditToolDetails { export interface EditToolDetails {
/** Unified diff of the changes made */ /** Unified diff of the changes made */
diff: string; diff: string;
@@ -112,10 +82,11 @@ export interface EditToolOptions {
operations?: EditOperations; operations?: EditOperations;
} }
function getMultiReplaceModeInput(input: EditToolInput): MultiReplaceModeInput | null { function validateEditInput(input: EditToolInput): { path: string; edits: Edit[] } {
if (input.edits === undefined) return null; if (!Array.isArray(input.edits)) {
if (input.oldText !== undefined || input.newText !== undefined) { throw new Error(
throw new Error("Edit tool input is invalid. Use either edits or single replacement mode, not both."); "Edit tool input is invalid. edits must be an array of replacements in the form { oldText: string, newText: string }.",
);
} }
if (input.edits.length === 0) { if (input.edits.length === 0) {
throw new Error("Edit tool input is invalid. edits must contain at least one replacement."); throw new Error("Edit tool input is invalid. edits must contain at least one replacement.");
@@ -123,46 +94,20 @@ function getMultiReplaceModeInput(input: EditToolInput): MultiReplaceModeInput |
return { path: input.path, edits: input.edits }; return { path: input.path, edits: input.edits };
} }
function getReplaceModeInput(input: EditToolInput): ReplaceModeInput | null { type RenderableEditArgs = {
const { oldText, newText } = input; path?: string;
if (oldText === undefined && newText === undefined) return null; file_path?: string;
if (input.edits !== undefined) { edits?: Edit[];
throw new Error("Edit tool input is invalid. Use either single replacement mode or edits mode, not both."); oldText?: string;
} newText?: string;
if (oldText === undefined || newText === undefined) { };
throw new Error("Edit tool input is invalid. Single replacement mode requires both oldText and newText.");
}
return { path: input.path, oldText, newText };
}
function normalizeEditInput(input: EditToolInput): NormalizedEditInput { function getRenderablePreviewInput(args: RenderableEditArgs | undefined): { path: string; edits: Edit[] } | null {
const multiReplaceModeInput = getMultiReplaceModeInput(input);
if (multiReplaceModeInput) {
return { path: multiReplaceModeInput.path, edits: multiReplaceModeInput.edits, mode: "multi" };
}
const replaceModeInput = getReplaceModeInput(input);
if (replaceModeInput) {
return {
path: replaceModeInput.path,
edits: [{ oldText: replaceModeInput.oldText, newText: replaceModeInput.newText }],
mode: "single",
};
}
throw new Error("Edit tool input is invalid. Provide either oldText and newText, or edits.");
}
function getRenderablePreviewInput(
args: { path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined,
): { path: string; edits: ReplaceEdit[] } | null {
if (!args || typeof args.path !== "string") { if (!args || typeof args.path !== "string") {
return null; return null;
} }
if ( if (
args.oldText === undefined &&
args.newText === undefined &&
Array.isArray(args.edits) && Array.isArray(args.edits) &&
args.edits.length > 0 && args.edits.length > 0 &&
args.edits.every((edit) => typeof edit?.oldText === "string" && typeof edit?.newText === "string") args.edits.every((edit) => typeof edit?.oldText === "string" && typeof edit?.newText === "string")
@@ -170,7 +115,7 @@ function getRenderablePreviewInput(
return { path: args.path, edits: args.edits }; return { path: args.path, edits: args.edits };
} }
if (typeof args.oldText === "string" && typeof args.newText === "string" && args.edits === undefined) { if (typeof args.oldText === "string" && typeof args.newText === "string") {
return { path: args.path, edits: [{ oldText: args.oldText, newText: args.newText }] }; return { path: args.path, edits: [{ oldText: args.oldText, newText: args.newText }] };
} }
@@ -178,7 +123,7 @@ function getRenderablePreviewInput(
} }
function formatEditCall( function formatEditCall(
args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, args: RenderableEditArgs | undefined,
state: EditRenderState, state: EditRenderState,
theme: typeof import("../../modes/interactive/theme/theme.js").theme, theme: typeof import("../../modes/interactive/theme/theme.js").theme,
): string { ): string {
@@ -200,7 +145,7 @@ function formatEditCall(
} }
function formatEditResult( function formatEditResult(
args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, args: RenderableEditArgs | undefined,
state: EditRenderState, state: EditRenderState,
result: { result: {
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>; content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
@@ -242,18 +187,18 @@ export function createEditToolDefinition(
name: "edit", name: "edit",
label: "edit", label: "edit",
description: description:
"Edit a single file using exact text replacement. Use oldText/newText only for one contiguous replacement. Use edits when changing multiple separate, disjoint regions in the same file. In edits mode, every edits[].oldText must match a unique, non-overlapping region of the original file. If two changes affect the same block or nearby lines, merge them into one edit instead of emitting overlapping edits. Do not include large unchanged regions just to connect distant changes. Do not provide both modes at once.", "Edit a single file using exact text replacement. Every edits[].oldText must match a unique, non-overlapping region of the original file. If two changes affect the same block or nearby lines, merge them into one edit instead of emitting overlapping edits. Do not include large unchanged regions just to connect distant changes.",
promptSnippet: promptSnippet:
"Make precise file edits with exact text replacement, including multiple disjoint edits in one call", "Make precise file edits with exact text replacement, including multiple disjoint edits in one call",
promptGuidelines: [ promptGuidelines: [
"Use edit for precise changes (old text must match exactly)", "Use edit for precise changes (edits[].oldText must match exactly)",
"When changing multiple separate locations in one file, use one edit call with edits[] instead of multiple edit calls", "When changing multiple separate locations in one file, use one edit call with multiple entries in edits[] instead of multiple edit calls",
"Each edits[].oldText is matched against the original file, not after earlier edits are applied. Do not emit overlapping or nested edits. Merge nearby changes into one edit.", "Each edits[].oldText is matched against the original file, not after earlier edits are applied. Do not emit overlapping or nested edits. Merge nearby changes into one edit.",
"Keep oldText as small as possible while still being unique in the file. Do not pad with large unchanged regions.", "Keep edits[].oldText as small as possible while still being unique in the file. Do not pad with large unchanged regions.",
], ],
parameters: editSchema, parameters: editSchema,
async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, _ctx?) { async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, _ctx?) {
const { path, edits, mode } = normalizeEditInput(input); const { path, edits } = validateEditInput(input);
const absolutePath = resolveToCwd(path, cwd); const absolutePath = resolveToCwd(path, cwd);
return withFileMutationQueue( return withFileMutationQueue(
@@ -342,10 +287,7 @@ export function createEditToolDefinition(
content: [ content: [
{ {
type: "text", type: "text",
text: text: `Successfully replaced ${edits.length} block(s) in ${path}.`,
mode === "single"
? `Successfully replaced text in ${path}.`
: `Successfully replaced ${edits.length} block(s) in ${path}.`,
}, },
], ],
details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine }, details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine },
@@ -366,9 +308,7 @@ export function createEditToolDefinition(
}, },
renderCall(args, theme, context) { renderCall(args, theme, context) {
if (context.argsComplete) { if (context.argsComplete) {
const previewInput = getRenderablePreviewInput( const previewInput = getRenderablePreviewInput(args as RenderableEditArgs);
args as { path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] },
);
if (previewInput) { if (previewInput) {
const argsKey = JSON.stringify({ path: previewInput.path, edits: previewInput.edits }); const argsKey = JSON.stringify({ path: previewInput.path, edits: previewInput.edits });
if (context.state.argsKey !== argsKey) { if (context.state.argsKey !== argsKey) {

View File

@@ -108,8 +108,8 @@ describe("built-in edit and write tools", () => {
}); });
await Promise.all([ await Promise.all([
editTool.execute("call-1", { path: filePath, oldText: "alpha", newText: "ALPHA" }), editTool.execute("call-1", { path: filePath, edits: [{ oldText: "alpha", newText: "ALPHA" }] }),
editTool.execute("call-2", { path: filePath, oldText: "beta", newText: "BETA" }), editTool.execute("call-2", { path: filePath, edits: [{ oldText: "beta", newText: "BETA" }] }),
]); ]);
const content = await readFile(filePath, "utf8"); const content = await readFile(filePath, "utf8");
@@ -147,8 +147,7 @@ describe("built-in edit and write tools", () => {
const editPromise = editTool.execute("call-1", { const editPromise = editTool.execute("call-1", {
path: filePath, path: filePath,
oldText: "original", edits: [{ oldText: "original", newText: "edited" }],
newText: "edited",
}); });
await delay(5); await delay(5);
const writePromise = writeTool.execute("call-2", { const writePromise = writeTool.execute("call-2", {

View File

@@ -220,8 +220,7 @@ describe("Coding Agent Tools", () => {
const result = await editTool.execute("test-call-5", { const result = await editTool.execute("test-call-5", {
path: testFile, path: testFile,
oldText: "world", edits: [{ oldText: "world", newText: "testing" }],
newText: "testing",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -239,8 +238,7 @@ describe("Coding Agent Tools", () => {
await expect( await expect(
editTool.execute("test-call-6", { editTool.execute("test-call-6", {
path: testFile, path: testFile,
oldText: "nonexistent", edits: [{ oldText: "nonexistent", newText: "testing" }],
newText: "testing",
}), }),
).rejects.toThrow(/Could not find the exact text/); ).rejects.toThrow(/Could not find the exact text/);
}); });
@@ -253,8 +251,7 @@ describe("Coding Agent Tools", () => {
await expect( await expect(
editTool.execute("test-call-7", { editTool.execute("test-call-7", {
path: testFile, path: testFile,
oldText: "foo", edits: [{ oldText: "foo", newText: "bar" }],
newText: "bar",
}), }),
).rejects.toThrow(/Found 3 occurrences/); ).rejects.toThrow(/Found 3 occurrences/);
}); });
@@ -315,20 +312,6 @@ describe("Coding Agent Tools", () => {
expect(readFileSync(testFile, "utf-8")).toBe("foo bar\nBAR\nbaz\n"); expect(readFileSync(testFile, "utf-8")).toBe("foo bar\nBAR\nbaz\n");
}); });
it("should fail when mixing single-edit mode with edits mode", async () => {
const testFile = join(testDir, "edit-invalid-mixed.txt");
writeFileSync(testFile, "hello\nworld\n");
await expect(
editTool.execute("test-call-10", {
path: testFile,
oldText: "hello\n",
newText: "HELLO\n",
edits: [{ oldText: "world\n", newText: "WORLD\n" }],
}),
).rejects.toThrow(/Use either/);
});
it("should fail when edits is empty", async () => { it("should fail when edits is empty", async () => {
const testFile = join(testDir, "edit-empty-edits.txt"); const testFile = join(testDir, "edit-empty-edits.txt");
writeFileSync(testFile, "hello\nworld\n"); writeFileSync(testFile, "hello\nworld\n");
@@ -569,8 +552,7 @@ describe("edit tool fuzzy matching", () => {
// oldText without trailing whitespace should still match // oldText without trailing whitespace should still match
const result = await editTool.execute("test-fuzzy-1", { const result = await editTool.execute("test-fuzzy-1", {
path: testFile, path: testFile,
oldText: "line one\nline two\n", edits: [{ oldText: "line one\nline two\n", newText: "replaced\n" }],
newText: "replaced\n",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -584,8 +566,7 @@ describe("edit tool fuzzy matching", () => {
const result = await editTool.execute("test-fuzzy-chinese", { const result = await editTool.execute("test-fuzzy-chinese", {
path: testFile, path: testFile,
oldText: "你好,世界\n你好(世界)\n", edits: [{ oldText: "你好,世界\n你好(世界)\n", newText: "你好pi\n你好(pi)\n" }],
newText: "你好pi\n你好(pi)\n",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -599,8 +580,7 @@ describe("edit tool fuzzy matching", () => {
const result = await editTool.execute("test-fuzzy-unicode", { const result = await editTool.execute("test-fuzzy-unicode", {
path: testFile, path: testFile,
oldText: "ABC123\ncafé\n", edits: [{ oldText: "ABC123\ncafé\n", newText: "XYZ789\ncoffee\n" }],
newText: "XYZ789\ncoffee\n",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -616,8 +596,7 @@ describe("edit tool fuzzy matching", () => {
// oldText with ASCII quotes should match // oldText with ASCII quotes should match
const result = await editTool.execute("test-fuzzy-2", { const result = await editTool.execute("test-fuzzy-2", {
path: testFile, path: testFile,
oldText: "console.log('hello');", edits: [{ oldText: "console.log('hello');", newText: "console.log('world');" }],
newText: "console.log('world');",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -633,8 +612,7 @@ describe("edit tool fuzzy matching", () => {
// oldText with ASCII quotes should match // oldText with ASCII quotes should match
const result = await editTool.execute("test-fuzzy-3", { const result = await editTool.execute("test-fuzzy-3", {
path: testFile, path: testFile,
oldText: 'const msg = "Hello World";', edits: [{ oldText: 'const msg = "Hello World";', newText: 'const msg = "Goodbye";' }],
newText: 'const msg = "Goodbye";',
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -650,8 +628,7 @@ describe("edit tool fuzzy matching", () => {
// oldText with ASCII hyphens should match // oldText with ASCII hyphens should match
const result = await editTool.execute("test-fuzzy-4", { const result = await editTool.execute("test-fuzzy-4", {
path: testFile, path: testFile,
oldText: "range: 1-5\nbreak-here", edits: [{ oldText: "range: 1-5\nbreak-here", newText: "range: 10-50\nbreak--here" }],
newText: "range: 10-50\nbreak--here",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -667,8 +644,7 @@ describe("edit tool fuzzy matching", () => {
// oldText with regular space should match // oldText with regular space should match
const result = await editTool.execute("test-fuzzy-5", { const result = await editTool.execute("test-fuzzy-5", {
path: testFile, path: testFile,
oldText: "hello world", edits: [{ oldText: "hello world", newText: "hello universe" }],
newText: "hello universe",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -683,8 +659,7 @@ describe("edit tool fuzzy matching", () => {
const result = await editTool.execute("test-fuzzy-6", { const result = await editTool.execute("test-fuzzy-6", {
path: testFile, path: testFile,
oldText: "const x = 'exact';", edits: [{ oldText: "const x = 'exact';", newText: "const x = 'changed';" }],
newText: "const x = 'changed';",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -699,8 +674,7 @@ describe("edit tool fuzzy matching", () => {
await expect( await expect(
editTool.execute("test-fuzzy-7", { editTool.execute("test-fuzzy-7", {
path: testFile, path: testFile,
oldText: "this does not exist", edits: [{ oldText: "this does not exist", newText: "replacement" }],
newText: "replacement",
}), }),
).rejects.toThrow(/Could not find the exact text/); ).rejects.toThrow(/Could not find the exact text/);
}); });
@@ -713,8 +687,7 @@ describe("edit tool fuzzy matching", () => {
await expect( await expect(
editTool.execute("test-fuzzy-8", { editTool.execute("test-fuzzy-8", {
path: testFile, path: testFile,
oldText: "hello world", edits: [{ oldText: "hello world", newText: "replaced" }],
newText: "replaced",
}), }),
).rejects.toThrow(/Found 2 occurrences/); ).rejects.toThrow(/Found 2 occurrences/);
}); });
@@ -754,8 +727,7 @@ describe("edit tool CRLF handling", () => {
const result = await editTool.execute("test-crlf-1", { const result = await editTool.execute("test-crlf-1", {
path: testFile, path: testFile,
oldText: "line two\n", edits: [{ oldText: "line two\n", newText: "replaced line\n" }],
newText: "replaced line\n",
}); });
expect(getTextOutput(result)).toContain("Successfully replaced"); expect(getTextOutput(result)).toContain("Successfully replaced");
@@ -767,8 +739,7 @@ describe("edit tool CRLF handling", () => {
await editTool.execute("test-crlf-2", { await editTool.execute("test-crlf-2", {
path: testFile, path: testFile,
oldText: "second\n", edits: [{ oldText: "second\n", newText: "REPLACED\n" }],
newText: "REPLACED\n",
}); });
const content = readFileSync(testFile, "utf-8"); const content = readFileSync(testFile, "utf-8");
@@ -781,8 +752,7 @@ describe("edit tool CRLF handling", () => {
await editTool.execute("test-lf-1", { await editTool.execute("test-lf-1", {
path: testFile, path: testFile,
oldText: "second\n", edits: [{ oldText: "second\n", newText: "REPLACED\n" }],
newText: "REPLACED\n",
}); });
const content = readFileSync(testFile, "utf-8"); const content = readFileSync(testFile, "utf-8");
@@ -797,8 +767,7 @@ describe("edit tool CRLF handling", () => {
await expect( await expect(
editTool.execute("test-crlf-dup", { editTool.execute("test-crlf-dup", {
path: testFile, path: testFile,
oldText: "hello\nworld\n", edits: [{ oldText: "hello\nworld\n", newText: "replaced\n" }],
newText: "replaced\n",
}), }),
).rejects.toThrow(/Found 2 occurrences/); ).rejects.toThrow(/Found 2 occurrences/);
}); });
@@ -809,8 +778,7 @@ describe("edit tool CRLF handling", () => {
await editTool.execute("test-bom", { await editTool.execute("test-bom", {
path: testFile, path: testFile,
oldText: "second\n", edits: [{ oldText: "second\n", newText: "REPLACED\n" }],
newText: "REPLACED\n",
}); });
const content = readFileSync(testFile, "utf-8"); const content = readFileSync(testFile, "utf-8");