From ed9af4c6bdad50a7043a9b2e64b2bed60360d6f3 Mon Sep 17 00:00:00 2001 From: urjitc <135136842+urjitc@users.noreply.github.com> Date: Fri, 10 Apr 2026 21:32:40 +0000 Subject: [PATCH 1/2] Simplify item_edit by migrating oldString/newString to multi-edit edits[] array and removing replace --- .../assistant-ui/EditItemToolUI.tsx | 4 +- .../ai/tools/__tests__/edit-item-tool.test.ts | 67 ++- src/lib/ai/tools/edit-item-tool.ts | 56 +- .../__tests__/workspace-worker.edit.test.ts | 96 +-- src/lib/ai/workers/workspace-worker.ts | 64 +- src/lib/utils/__tests__/edit-replace.test.ts | 184 ++++-- src/lib/utils/edit-replace.ts | 561 ++++-------------- src/lib/utils/format-workspace-context.ts | 2 +- 8 files changed, 372 insertions(+), 662 deletions(-) diff --git a/src/components/assistant-ui/EditItemToolUI.tsx b/src/components/assistant-ui/EditItemToolUI.tsx index 19e4c873..938986ef 100644 --- a/src/components/assistant-ui/EditItemToolUI.tsx +++ b/src/components/assistant-ui/EditItemToolUI.tsx @@ -21,9 +21,7 @@ import type { Item } from "@/lib/workspace-state/types"; type EditItemArgs = { itemName: string; - oldString: string; - newString: string; - replaceAll?: boolean; + edits: Array<{ oldText: string; newText: string }>; newName?: string; }; diff --git a/src/lib/ai/tools/__tests__/edit-item-tool.test.ts b/src/lib/ai/tools/__tests__/edit-item-tool.test.ts index bf026b80..530e4336 100644 --- a/src/lib/ai/tools/__tests__/edit-item-tool.test.ts +++ b/src/lib/ai/tools/__tests__/edit-item-tool.test.ts @@ -59,7 +59,7 @@ describe("createEditItemTool", () => { it("fails fast when itemName is missing", async () => { const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); expect(result.message).toMatch(/itemName is required/i); }); @@ -67,7 +67,7 @@ describe("createEditItemTool", () => { it("returns loadStateForTool failure", async () => { mockLoadStateForTool.mockResolvedValueOnce({ success: false, message: "No workspace context available" }); const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "My Document", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "My Document", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); expect(result.message).toMatch(/No workspace context available/); expect(mockWorkspaceWorker).not.toHaveBeenCalled(); @@ -76,7 +76,7 @@ describe("createEditItemTool", () => { it("returns item not found message when resolveItem misses", async () => { mockResolveItem.mockReturnValueOnce(undefined); const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "Unknown", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "Unknown", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); expect(result.message).toMatch(/Could not find item "Unknown"/); }); @@ -93,7 +93,7 @@ describe("createEditItemTool", () => { .mockReturnValueOnce("folder/My Document.md"); const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "My Document", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "My Document", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); expect(result.message).toMatch(/Multiple items named/); expect(result.message).toMatch(/Disambiguate using path/); @@ -105,7 +105,7 @@ describe("createEditItemTool", () => { mockLoadStateForTool.mockResolvedValueOnce({ success: true, state: { items: [imageItem] } }); const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "Figure", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "Figure", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); expect(result.message).toMatch(/not editable/); }); @@ -116,7 +116,7 @@ describe("createEditItemTool", () => { mockLoadStateForTool.mockResolvedValueOnce({ success: true, state: { items: [pdfItem] } }); const tool: any = createEditItemTool(ctx); - const contentEditResult = await tool.execute({ itemName: "Syllabus.pdf", oldString: "foo", newString: "bar" }); + const contentEditResult = await tool.execute({ itemName: "Syllabus.pdf", edits: [{ oldText: "foo", newText: "bar" }] }); expect(contentEditResult.success).toBe(false); expect(contentEditResult.message).toMatch(/rename only|can only be renamed/i); @@ -126,8 +126,7 @@ describe("createEditItemTool", () => { const renameResult = await tool.execute({ itemName: "Syllabus.pdf", - oldString: "", - newString: "", + edits: [], newName: "Course Syllabus.pdf", }); expect(renameResult.success).toBe(true); @@ -142,10 +141,8 @@ describe("createEditItemTool", () => { const tool: any = createEditItemTool(ctx); const result = await tool.execute({ itemName: "My Document", - oldString: "old", - newString: "new", + edits: [{ oldText: "old", newText: "new" }], newName: "Renamed Document", - replaceAll: true, sources: [{ title: "Ref", url: "https://example.com" }], }); @@ -155,9 +152,7 @@ describe("createEditItemTool", () => { workspaceId: "ws-1", itemId: "doc-1", itemType: "document", - oldString: "old", - newString: "new", - replaceAll: true, + edits: [{ oldText: "old", newText: "new" }], newName: "Renamed Document", }) ); @@ -165,12 +160,11 @@ describe("createEditItemTool", () => { expect(result.itemName).toBe("Renamed Document"); }); - it("supports rename-only with oldString='' and newString=''", async () => { + it("supports rename-only with empty edits array", async () => { const tool: any = createEditItemTool(ctx); const result = await tool.execute({ itemName: "My Document", - oldString: "", - newString: "", + edits: [], newName: "Renamed Document", }); @@ -180,8 +174,7 @@ describe("createEditItemTool", () => { workspaceId: "ws-1", itemId: "doc-1", itemType: "document", - oldString: "", - newString: "", + edits: [], newName: "Renamed Document", }) ); @@ -192,13 +185,41 @@ describe("createEditItemTool", () => { it("passes through worker failures", async () => { mockWorkspaceWorker.mockResolvedValueOnce({ success: false, - message: "Found multiple matches for oldString", + message: "Found 2 occurrences of the text", }); const tool: any = createEditItemTool(ctx); - const result = await tool.execute({ itemName: "My Document", oldString: "a", newString: "b" }); + const result = await tool.execute({ itemName: "My Document", edits: [{ oldText: "a", newText: "b" }] }); expect(result.success).toBe(false); - expect(result.message).toMatch(/multiple matches/i); + expect(result.message).toMatch(/occurrences/i); }); -}); + it("rejects empty edits array without newName", async () => { + const tool: any = createEditItemTool(ctx); + const result = await tool.execute({ itemName: "My Document", edits: [] }); + expect(result.success).toBe(false); + expect(result.message).toMatch(/edits array is empty/); + }); + + it("supports multiple edits in one call", async () => { + const tool: any = createEditItemTool(ctx); + const result = await tool.execute({ + itemName: "My Document", + edits: [ + { oldText: "first", newText: "FIRST" }, + { oldText: "second", newText: "SECOND" }, + ], + }); + + expect(mockWorkspaceWorker).toHaveBeenCalledWith( + "edit", + expect.objectContaining({ + edits: [ + { oldText: "first", newText: "FIRST" }, + { oldText: "second", newText: "SECOND" }, + ], + }) + ); + expect(result.success).toBe(true); + }); +}); diff --git a/src/lib/ai/tools/edit-item-tool.ts b/src/lib/ai/tools/edit-item-tool.ts index 6a0f9899..b2420c8f 100644 --- a/src/lib/ai/tools/edit-item-tool.ts +++ b/src/lib/ai/tools/edit-item-tool.ts @@ -12,30 +12,30 @@ const EDITABLE_TYPES = ["flashcard", "quiz", "pdf", "document"] as const; /** * Create the item_edit tool - unified edit for documents, flashcards, quizzes, and PDFs. - * Uses oldString/newString search-replace on raw content from workspace_read. - * PDFs support RENAME ONLY: oldString='', newString='', newName='new name'. + * Uses edits[] multi-edit pattern on raw content from workspace_read. + * PDFs support RENAME ONLY: empty edits array with newName. */ export function createEditItemTool(ctx: WorkspaceToolContext) { return withSanitizedModelOutput(tool({ description: - "Edit a document, flashcard deck, quiz, or PDF. You must use workspace_read at least once before editing. DOCUMENTS: content is markdown from workspace_read. PDFs: RENAME ONLY — pass oldString='', newString='', and newName='new name'. PDF content cannot be edited. RENAME ONLY (documents/flashcards/quizzes): same pattern to rename without editing content. QUIZZES: workspace_read may show '--- Progress (read-only) ---' at the top. That block is READ-ONLY. Never include it in oldString or newString. Only edit the {\"questions\":[...]} JSON. FULL REWRITE: oldString='' and newString=entire new content (quizzes: only the JSON); use when targeted matching fails or the change is large (re-read if needed). TARGETED EDIT: oldString must match exactly. Copy the content from workspace_read as-is (it has no line prefixes). Match exact whitespace, indentation, newlines. Do NOT minify JSON. Edit FAILS if oldString not found or matches multiple times — add more context or use replaceAll.", + "Edit a document, flashcard deck, quiz, or PDF. You must use workspace_read at least once before editing. " + + "TARGETED EDIT: provide edits array with one or more {oldText, newText} pairs. Each oldText must match exactly and uniquely in the original content. Copy from workspace_read as-is. When changing multiple separate sections, use one call with multiple edits[] entries instead of multiple calls. " + + "FULL REWRITE: single edit with oldText='' and newText=entire new content (quizzes: only the JSON). " + + "RENAME ONLY: empty edits array [] with newName='new name'. " + + "PDFs: RENAME ONLY — empty edits array with newName. " + + "QUIZZES: workspace_read may show '--- Progress (read-only) ---' at the top. That block is READ-ONLY. Only edit the {\"questions\":[...]} JSON.", inputSchema: zodSchema( z .object({ itemName: z .string() .describe("Name of the item to edit (document, flashcard deck, quiz, or PDF; matched by fuzzy search)"), - oldString: z - .string() - .describe( - "Text to find; '' = full rewrite. Targeted: copy from workspace_read as-is, match whitespace, enough context to be unique, or replaceAll." - ), - newString: z.string().describe("Replacement text (entire content if oldString is empty)"), - replaceAll: z - .boolean() - .optional() - .default(false) - .describe("Replace every occurrence of oldString; use for renaming or changing repeated text."), + edits: z.array(z.object({ + oldText: z.string().describe("Exact text to find in the original content. Must be unique. Copy from workspace_read as-is."), + newText: z.string().describe("Replacement text for this edit."), + })).describe( + "One or more targeted replacements. Each edit is matched against the original content, not incrementally. Do not include overlapping edits. If two changes touch the same block or nearby lines, merge them into one edit instead. For full rewrite: use a single edit with oldText='' and newText=entire new content." + ), newName: z.string().optional().describe("Rename the item to this. If not provided, the existing name is preserved."), sources: z .array(sourceSchema) @@ -47,13 +47,11 @@ export function createEditItemTool(ctx: WorkspaceToolContext) { strict: true, execute: async (input: { itemName: string; - oldString: string; - newString: string; - replaceAll?: boolean; + edits: Array<{ oldText: string; newText: string }>; newName?: string; sources?: Array<{ title: string; url: string; favicon?: string }>; }) => { - const { itemName, oldString, newString, replaceAll, newName } = input; + const { itemName, edits, newName } = input; if (!itemName) { return { @@ -62,25 +60,19 @@ export function createEditItemTool(ctx: WorkspaceToolContext) { }; } - const isRenameOnly = Boolean(newName) && (oldString === undefined || oldString === null || oldString === "") && (newString === undefined || newString === null || newString === ""); - if (!isRenameOnly && (oldString === undefined || oldString === null)) { - return { - success: false, - message: "oldString is required. Use '' for full rewrite, or for rename-only pass oldString='', newString='', and newName.", - }; - } + const isRenameOnly = Boolean(newName) && edits.length === 0; - if (!isRenameOnly && (newString === undefined || newString === null)) { + if (!isRenameOnly && edits.length === 0) { return { success: false, - message: "newString is required. For rename-only, pass oldString='', newString='', and newName.", + message: "edits array is empty and no newName provided. Provide at least one edit or a newName for rename-only.", }; } - if (oldString === newString && !isRenameOnly) { + if (!isRenameOnly && edits.length === 1 && edits[0].oldText !== "" && edits[0].oldText === edits[0].newText) { return { success: false, - message: "No changes to apply: oldString and newString are identical.", + message: "No changes to apply: oldText and newText are identical.", }; } @@ -134,7 +126,7 @@ export function createEditItemTool(ctx: WorkspaceToolContext) { if (matchedItem.type === "pdf" && !isRenameOnly) { return { success: false, - message: "PDFs can only be renamed, not edited. Use oldString='', newString='', and newName='new name'.", + message: "PDFs can only be renamed, not edited. Use empty edits array [] with newName='new name'.", }; } @@ -150,9 +142,7 @@ export function createEditItemTool(ctx: WorkspaceToolContext) { itemId: matchedItem.id, itemType: matchedItem.type as "flashcard" | "quiz" | "pdf" | "document", itemName: matchedItem.name, - oldString, - newString, - replaceAll, + edits, newName, sources: input.sources, }); diff --git a/src/lib/ai/workers/__tests__/workspace-worker.edit.test.ts b/src/lib/ai/workers/__tests__/workspace-worker.edit.test.ts index b3d61328..d7c2c3ff 100644 --- a/src/lib/ai/workers/__tests__/workspace-worker.edit.test.ts +++ b/src/lib/ai/workers/__tests__/workspace-worker.edit.test.ts @@ -170,19 +170,21 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "quiz-1", itemType: "quiz", itemName: "Quiz 1", - oldString: " ]\n}", - newString: [ - " ,", - " {", - ' "id": "q2",', - ' "type": "multiple_choice",', - ' "questionText": "Q2",', - ' "options": ["A", "B", "C", "D"],', - ' "correctIndex": 1,', - " }", - " ]", - "}", - ].join("\n"), + edits: [{ + oldText: " ]\n}", + newText: [ + " ,", + " {", + ' "id": "q2",', + ' "type": "multiple_choice",', + ' "questionText": "Q2",', + ' "options": ["A", "B", "C", "D"],', + ' "correctIndex": 1,', + " }", + " ]", + "}", + ].join("\n"), + }], }); expect(result.success).toBe(true); @@ -226,14 +228,15 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "deck-1", itemType: "flashcard", itemName: "Deck 1", - oldString: " ]\n}", - // intentionally malformed but repairable (single quotes + trailing comma) - newString: [ - " ,", - " {'id':'c2','front':'f2','back':'b2'},", - " ]", - "}", - ].join("\n"), + edits: [{ + oldText: " ]\n}", + newText: [ + " ,", + " {'id':'c2','front':'f2','back':'b2'},", + " ]", + "}", + ].join("\n"), + }], }); expect(result.success).toBe(true); @@ -283,20 +286,21 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "quiz-1", itemType: "quiz", itemName: "Quiz 1", - oldString: JSON.stringify({ questions: initialQuestions }, null, 2), - // repair may parse this, but schema should reject 3 options for MC - newString: JSON.stringify( - { - questions: [ - { - ...initialQuestions[0], - options: ["A", "B", "C"], - }, - ], - }, - null, - 2, - ), + edits: [{ + oldText: JSON.stringify({ questions: initialQuestions }, null, 2), + newText: JSON.stringify( + { + questions: [ + { + ...initialQuestions[0], + options: ["A", "B", "C"], + }, + ], + }, + null, + 2, + ), + }], }); expect(result.success).toBe(false); @@ -323,12 +327,14 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "deck-1", itemType: "flashcard", itemName: "Deck 1", - oldString: "{", - newString: "{ ???", + edits: [{ + oldText: '{\n "cards": [', + newText: "{ ??? invalid [", + }], }); expect(result.success).toBe(false); - expect(result.message).toMatch(/Invalid JSON after edit/i); + expect(result.message).toMatch(/Invalid JSON after edit|Invalid structure/i); }); it("rename-only: updates document name without touching content", async () => { @@ -354,8 +360,7 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "doc-1", itemType: "document", itemName: "My Document", - oldString: "", - newString: "", + edits: [], newName: "Renamed Document", }); @@ -382,7 +387,7 @@ describe("workspaceWorker edit end-to-end paths", () => { expect(eventPayload.changes).toEqual({ name: "Renamed Document" }); }); - it("fails with clear message when oldString is ambiguous", async () => { + it("fails with clear message when oldText is ambiguous", async () => { mockLoadWorkspaceState.mockResolvedValue({ items: [ { @@ -417,12 +422,13 @@ describe("workspaceWorker edit end-to-end paths", () => { itemId: "quiz-1", itemType: "quiz", itemName: "Quiz 1", - oldString: '"questionText": "Same"', - newString: '"questionText": "Updated"', - replaceAll: false, + edits: [{ + oldText: '"questionText": "Same"', + newText: '"questionText": "Updated"', + }], }); expect(result.success).toBe(false); - expect(result.message).toMatch(/multiple matches/i); + expect(result.message).toMatch(/occurrences/i); }); }); diff --git a/src/lib/ai/workers/workspace-worker.ts b/src/lib/ai/workers/workspace-worker.ts index 294f6bb7..0bfb6ab1 100644 --- a/src/lib/ai/workers/workspace-worker.ts +++ b/src/lib/ai/workers/workspace-worker.ts @@ -26,7 +26,7 @@ import { } from "@/lib/workspace/mutation-helpers"; import type { WorkspaceEvent } from "@/lib/workspace/events"; import { - replace as applyReplace, + applyEdits, trimDiff, normalizeLineEndings, } from "@/lib/utils/edit-replace"; @@ -224,10 +224,8 @@ export async function workspaceWorker( items?: CreateItemParams[]; title?: string; content?: string; // For create - /** Cline convention: oldString+newString. oldString='' = full rewrite, else targeted edit (edit action) */ - oldString?: string; - newString?: string; - replaceAll?: boolean; + /** Multi-edit array: each {oldText, newText} is matched against the original content */ + edits?: Array<{ oldText: string; newText: string }>; itemId?: string; /** Display name for diff header (e.g. item title) */ itemName?: string; @@ -612,15 +610,12 @@ export async function workspaceWorker( if (!params.itemId || !params.itemType) { throw new Error("Item ID and itemType required for edit"); } - const isRenameOnly = - params.newName && - (params.oldString === undefined || params.oldString === "") && - (params.newString === undefined || params.newString === ""); + const isRenameOnly = params.newName && (!params.edits || params.edits.length === 0); if ( !isRenameOnly && - (params.oldString === undefined || params.newString === undefined) + (!params.edits || params.edits.length === 0) ) { - throw new Error("oldString and newString required for edit"); + throw new Error("edits array required for edit"); } const currentState = await loadWorkspaceItemsForValidation(params.workspaceId, userId); @@ -632,9 +627,6 @@ export async function workspaceWorker( } const rename = params.newName; - const replaceAll = !!params.replaceAll; - const oldStr = String(params.oldString ?? ""); - const newStr = String(params.newString ?? ""); if (existingItem.type === "flashcard") { const data = existingItem.data as FlashcardData; @@ -649,12 +641,12 @@ export async function workspaceWorker( }; let serialized = JSON.stringify(payload, null, 2); if (!isRenameOnly) { - serialized = applyReplace( - normalizeLineEndings(serialized), - normalizeLineEndings(oldStr), - normalizeLineEndings(newStr), - replaceAll, - ); + if (params.edits!.length === 1 && params.edits![0].oldText === "") { + serialized = params.edits![0].newText; + } else { + const { newContent } = applyEdits(serialized, params.edits!); + serialized = newContent; + } } let parsed: { @@ -669,7 +661,7 @@ export async function workspaceWorker( const detail = e instanceof Error ? e.message : String(e); throw new Error( `Invalid JSON after edit. The edited flashcard content is not valid JSON (even after repair). ` + - `Try replacing a larger unique block, or use full rewrite with oldString='' and a complete {"cards":[...]} JSON object. ` + + `Try replacing a larger unique block, or use full rewrite with a single edit with oldText='' and a complete {"cards":[...]} JSON object. ` + `Details: ${detail}`, ); } @@ -725,12 +717,12 @@ export async function workspaceWorker( const payload = { questions }; let serialized = JSON.stringify(payload, null, 2); if (!isRenameOnly) { - serialized = applyReplace( - normalizeLineEndings(serialized), - normalizeLineEndings(oldStr), - normalizeLineEndings(newStr), - replaceAll, - ); + if (params.edits!.length === 1 && params.edits![0].oldText === "") { + serialized = params.edits![0].newText; + } else { + const { newContent } = applyEdits(serialized, params.edits!); + serialized = newContent; + } } let parsed: { questions?: QuizQuestion[] }; @@ -743,7 +735,7 @@ export async function workspaceWorker( const detail = e instanceof Error ? e.message : String(e); throw new Error( `Invalid JSON after edit. The edited quiz content is not valid JSON (even after repair). ` + - `Only edit the {"questions":[...]} JSON, and replace a larger unique block (or do full rewrite with oldString=''). ` + + `Only edit the {"questions":[...]} JSON, and replace a larger unique block (or do full rewrite with a single edit with oldText=''). ` + `Details: ${detail}`, ); } @@ -839,19 +831,13 @@ export async function workspaceWorker( contentOld = ""; contentNew = ""; } else { - if (oldStr === "") { + if (params.edits!.length === 1 && params.edits![0].oldText === "") { contentOld = ""; - contentNew = newStr; + contentNew = params.edits![0].newText; } else { contentOld = normalizeLineEndings(docData.markdown ?? ""); - const normOld = normalizeLineEndings(oldStr); - const normNew = normalizeLineEndings(newStr); - contentNew = applyReplace( - contentOld, - normOld, - normNew, - replaceAll, - ); + const { newContent } = applyEdits(contentOld, params.edits!); + contentNew = newContent; } changes.data = { markdown: contentNew, @@ -923,7 +909,7 @@ export async function workspaceWorker( return { success: false, message: - "PDFs can only be renamed. Use oldString='', newString='', and newName='new name'.", + "PDFs can only be renamed. Use empty edits array [] with newName='new name'.", }; } const dupError = checkDuplicateName(currentState, rename, "pdf", existingItem.folderId ?? null, params.itemId); diff --git a/src/lib/utils/__tests__/edit-replace.test.ts b/src/lib/utils/__tests__/edit-replace.test.ts index bb4de42b..401586f4 100644 --- a/src/lib/utils/__tests__/edit-replace.test.ts +++ b/src/lib/utils/__tests__/edit-replace.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect } from "vitest"; import { + applyEdits, replace, normalizeLineEndings, - unescapeString, trimDiff, } from "@/lib/utils/edit-replace"; @@ -20,76 +20,109 @@ describe("normalizeLineEndings", () => { }); }); -describe("replace (edit/replace for oldString/newString)", () => { - it("performs exact string replacement", () => { +describe("applyEdits", () => { + it("performs exact single replacement", () => { const content = "# Title\n\nHello world"; - expect(replace(content, "Hello world", "Goodbye world")).toBe( - "# Title\n\nGoodbye world" - ); + const result = applyEdits(content, [{ oldText: "Hello world", newText: "Goodbye world" }]); + expect(result.newContent).toBe("# Title\n\nGoodbye world"); }); - it("replace with normalized line endings (worker normalizes before calling)", () => { - const contentWithCrlf = "# Title\r\n\r\nHello world"; - const oldStr = "# Title\n\nHello world"; - // After normalizing both (as worker does), replace succeeds. - // Raw CRLF may or may not match depending on replacers; we always normalize. - const normalized = contentWithCrlf.replaceAll("\r\n", "\n"); - expect(replace(normalized, oldStr, "# Title\n\nHi")).toBe("# Title\n\nHi"); + it("applies two disjoint replacements in one call", () => { + const content = "aaa\nbbb\nccc"; + const result = applyEdits(content, [ + { oldText: "aaa", newText: "AAA" }, + { oldText: "ccc", newText: "CCC" }, + ]); + expect(result.newContent).toBe("AAA\nbbb\nCCC"); }); - it("throws when oldString not found", () => { + it("edits applied to original content, not incrementally", () => { + const content = "foo bar baz"; + const result = applyEdits(content, [ + { oldText: "foo", newText: "XXX" }, + { oldText: "baz", newText: "YYY" }, + ]); + expect(result.newContent).toBe("XXX bar YYY"); + }); + + it("throws on overlapping edits", () => { + const content = "abcdef"; expect(() => - replace("actual content", "typo contnet", "replacement") - ).toThrow("Could not find oldString"); + applyEdits(content, [ + { oldText: "abcd", newText: "X" }, + { oldText: "cdef", newText: "Y" }, + ]) + ).toThrow(/overlap/i); + }); + + it("throws on duplicate oldText (matches multiple locations)", () => { + const content = "foo bar foo bar"; + expect(() => + applyEdits(content, [{ oldText: "foo bar", newText: "replaced" }]) + ).toThrow(/occurrences/i); }); - it("throws when oldString matches multiple times without replaceAll", () => { + it("throws multi-edit duplicate error with index", () => { + const content = "foo bar foo bar"; expect(() => - replace("foo bar foo bar", "foo bar", "replaced") - ).toThrow("multiple matches"); + applyEdits(content, [ + { oldText: "foo bar", newText: "replaced" }, + { oldText: "something", newText: "else" }, + ]) + ).toThrow(/edits\[0\]/); }); - it("replaces all occurrences when replaceAll=true", () => { - expect( - replace("foo bar foo bar", "foo bar", "x", true) - ).toBe("x x"); + it("throws on empty oldText", () => { + expect(() => + applyEdits("content", [{ oldText: "", newText: "new" }]) + ).toThrow(/oldText must not be empty/i); }); - it("uses LineTrimmedReplacer when exact match has extra whitespace", () => { - const content = " line1 \n line2 \n line3 "; - const find = "line1\nline2\nline3"; - const result = replace(content, find, "replaced"); - expect(result).toBe("replaced"); + it("throws multi-edit empty oldText error with index", () => { + expect(() => + applyEdits("content", [ + { oldText: "content", newText: "x" }, + { oldText: "", newText: "new" }, + ]) + ).toThrow(/edits\[1\]\.oldText must not be empty/); }); - it("preserves newlines and indentation in replacement", () => { - const content = "before\n indented\nafter"; - expect( - replace(content, " indented", " modified") - ).toBe("before\n modified\nafter"); + it("throws when no changes made (replacement produces identical content)", () => { + const content = "hello"; + expect(() => + applyEdits(content, [{ oldText: "hello", newText: "hello" }]) + ).toThrow(/No changes made/i); }); - it("handles math block format from workspace_read", () => { - const content = "# Note\n\n$$\nx^2\n$$\n\nMore text"; - expect(replace(content, "$$\nx^2\n$$\n\n", "$$\n2x\n$$\n\n")).toBe( - "# Note\n\n$$\n2x\n$$\n\nMore text" - ); + it("throws when oldText not found (single edit)", () => { + expect(() => + applyEdits("actual content", [{ oldText: "typo contnet", newText: "replacement" }]) + ).toThrow(/Could not find the exact text/); + }); + + it("throws when oldText not found (multi edit)", () => { + expect(() => + applyEdits("actual content", [ + { oldText: "actual", newText: "new" }, + { oldText: "missing", newText: "x" }, + ]) + ).toThrow(/Could not find edits\[1\]/); }); - it("handles oldString wrapped in code fences", () => { - const content = "{\n \"cards\": []\n}"; - const oldFenced = "```json\n{\n \"cards\": []\n}\n```"; - const result = replace(content, oldFenced, "{\"cards\":[{\"front\":\"A\",\"back\":\"B\"}]}"); - expect(result).toBe("{\"cards\":[{\"front\":\"A\",\"back\":\"B\"}]}"); + it("unwraps code fence from oldText for JSON contexts", () => { + const content = '{\n "cards": []\n}'; + const oldFenced = '```json\n{\n "cards": []\n}\n```'; + const result = applyEdits(content, [{ oldText: oldFenced, newText: '{"cards":[{"front":"A","back":"B"}]}' }]); + expect(result.newContent).toBe('{"cards":[{"front":"A","back":"B"}]}'); }); - it("normalizes fenced JSON newString in JSON edit contexts", () => { - const content = "{\n \"questions\": []\n}"; + it("normalizes fenced JSON newText in JSON edit contexts", () => { + const content = '{\n "questions": []\n}'; const oldTail = "[]\n}"; - const fencedNew = "```json\n[\n {\n \"id\": \"q2\"\n }\n]\n```"; - const result = replace(content, oldTail, fencedNew); - expect(result).toContain('"id": "q2"'); - expect(result).not.toContain("```"); + const fencedNew = '```json\n[\n {\n "id": "q2"\n }\n]\n```'; + const result = applyEdits(content, [{ oldText: oldTail, newText: fencedNew }]); + expect(result.newContent).toContain('"id": "q2"'); + expect(result.newContent).not.toContain("```"); }); it("supports appending a quiz question near end of JSON", () => { @@ -121,9 +154,9 @@ describe("replace (edit/replace for oldString/newString)", () => { "}", ].join("\n"); - const result = replace(content, oldTail, newTail); - expect(result).toContain('"id": "q2"'); - expect(result).toContain('"questionText": "Q2"'); + const result = applyEdits(content, [{ oldText: oldTail, newText: newTail }]); + expect(result.newContent).toContain('"id": "q2"'); + expect(result.newContent).toContain('"questionText": "Q2"'); }); it("supports appending a flashcard near end of JSON", () => { @@ -151,24 +184,48 @@ describe("replace (edit/replace for oldString/newString)", () => { "}", ].join("\n"); - const result = replace(content, oldTail, newTail); - expect(result).toContain('"id": "c2"'); - expect(result).toContain('"front": "f2"'); + const result = applyEdits(content, [{ oldText: oldTail, newText: newTail }]); + expect(result.newContent).toContain('"id": "c2"'); + expect(result.newContent).toContain('"front": "f2"'); + }); + + it("preserves newlines and indentation in replacement", () => { + const content = "before\n indented\nafter"; + const result = applyEdits(content, [{ oldText: " indented", newText: " modified" }]); + expect(result.newContent).toBe("before\n modified\nafter"); + }); + + it("handles CRLF normalization", () => { + const contentWithCrlf = "# Title\r\n\r\nHello world"; + const result = applyEdits(contentWithCrlf, [{ oldText: "# Title\n\nHello world", newText: "# Title\n\nHi" }]); + expect(result.newContent).toBe("# Title\n\nHi"); }); - it("preserves ambiguity error when multiple matches remain", () => { - const content = "foo\nfoo\nfoo"; - expect(() => replace(content, "foo", "bar")).toThrow("multiple matches"); + it("returns baseContent as normalized content", () => { + const content = "hello\r\nworld"; + const result = applyEdits(content, [{ oldText: "hello", newText: "goodbye" }]); + expect(result.baseContent).toBe("hello\nworld"); }); }); -describe("unescapeString", () => { - it("converts literal \\n to newline", () => { - expect(unescapeString("a\\nb")).toBe("a\nb"); +describe("replace (deprecated wrapper)", () => { + it("performs exact string replacement", () => { + const content = "# Title\n\nHello world"; + expect(replace(content, "Hello world", "Goodbye world")).toBe( + "# Title\n\nGoodbye world" + ); + }); + + it("throws when oldString and newString are identical", () => { + expect(() => replace("content", "content", "content")).toThrow( + /No changes to apply/ + ); }); - it("converts \\$ to $", () => { - expect(unescapeString("\\$5")).toBe("$5"); + it("throws when oldString not found", () => { + expect(() => + replace("actual content", "typo contnet", "replacement") + ).toThrow(/Could not find/); }); }); @@ -176,7 +233,6 @@ describe("trimDiff", () => { it("trims common indentation from diff lines", () => { const diff = "--- a\n+++ b\n- content\n+ new"; const result = trimDiff(diff); - // trimDiff finds min leading whitespace (4) and trims that from each content line expect(result).toContain("-content"); expect(result).toContain("+new"); expect(result).not.toContain("- content"); diff --git a/src/lib/utils/edit-replace.ts b/src/lib/utils/edit-replace.ts index df73cb74..0600ad23 100644 --- a/src/lib/utils/edit-replace.ts +++ b/src/lib/utils/edit-replace.ts @@ -1,412 +1,12 @@ /** - * Robust edit/replace utilities for workspace text editing (e.g. documents, quizzes). - * Approaches sourced from Cline and Gemini CLI: - * - https://github.com/cline/cline/blob/main/evals/diff-edits/diff-apply/diff-06-23-25.ts - * - https://github.com/google-gemini/gemini-cli/blob/main/packages/core/src/utils/editCorrector.ts - * - https://github.com/cline/cline/blob/main/evals/diff-edits/diff-apply/diff-06-26-25.ts + * Edit/replace utilities for workspace text editing (documents, quizzes, flashcards). + * Multi-edit pattern modeled after pi-mono's edit tool. */ export function normalizeLineEndings(text: string): string { return text.replaceAll("\r\n", "\n"); } -export type Replacer = (content: string, find: string) => Generator; - -// Similarity thresholds for block anchor fallback matching -const SINGLE_CANDIDATE_SIMILARITY_THRESHOLD = 0.0; -const MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD = 0.3; - -function levenshtein(a: string, b: string): number { - if (a === "" || b === "") { - return Math.max(a.length, b.length); - } - const matrix = Array.from({ length: a.length + 1 }, (_, i) => - Array.from({ length: b.length + 1 }, (_, j) => (i === 0 ? j : j === 0 ? i : 0)) - ); - - for (let i = 1; i <= a.length; i++) { - for (let j = 1; j <= b.length; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - matrix[i][j] = Math.min(matrix[i - 1][j] + 1, matrix[i][j - 1] + 1, matrix[i - 1][j - 1] + cost); - } - } - return matrix[a.length][b.length]; -} - -export const SimpleReplacer: Replacer = function* (_content, find) { - yield find; -}; - -export const LineTrimmedReplacer: Replacer = function* (content, find) { - const originalLines = content.split("\n"); - const searchLines = find.split("\n"); - - if (searchLines[searchLines.length - 1] === "") { - searchLines.pop(); - } - - for (let i = 0; i <= originalLines.length - searchLines.length; i++) { - let matches = true; - - for (let j = 0; j < searchLines.length; j++) { - const originalTrimmed = originalLines[i + j].trim(); - const searchTrimmed = searchLines[j].trim(); - - if (originalTrimmed !== searchTrimmed) { - matches = false; - break; - } - } - - if (matches) { - let matchStartIndex = 0; - for (let k = 0; k < i; k++) { - matchStartIndex += originalLines[k].length + 1; - } - - let matchEndIndex = matchStartIndex; - for (let k = 0; k < searchLines.length; k++) { - matchEndIndex += originalLines[i + k].length; - if (k < searchLines.length - 1) { - matchEndIndex += 1; - } - } - - yield content.substring(matchStartIndex, matchEndIndex); - } - } -}; - -export const BlockAnchorReplacer: Replacer = function* (content, find) { - const originalLines = content.split("\n"); - const searchLines = find.split("\n"); - - if (searchLines.length < 3) { - return; - } - - if (searchLines[searchLines.length - 1] === "") { - searchLines.pop(); - } - - const firstLineSearch = searchLines[0].trim(); - const lastLineSearch = searchLines[searchLines.length - 1].trim(); - const searchBlockSize = searchLines.length; - - const candidates: Array<{ startLine: number; endLine: number }> = []; - for (let i = 0; i < originalLines.length; i++) { - if (originalLines[i].trim() !== firstLineSearch) { - continue; - } - - for (let j = i + 2; j < originalLines.length; j++) { - if (originalLines[j].trim() === lastLineSearch) { - candidates.push({ startLine: i, endLine: j }); - break; - } - } - } - - if (candidates.length === 0) { - return; - } - - if (candidates.length === 1) { - const { startLine, endLine } = candidates[0]; - const actualBlockSize = endLine - startLine + 1; - - let similarity = 0; - const linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2); - - if (linesToCheck > 0) { - for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { - const originalLine = originalLines[startLine + j].trim(); - const searchLine = searchLines[j].trim(); - const maxLen = Math.max(originalLine.length, searchLine.length); - if (maxLen === 0) { - continue; - } - const distance = levenshtein(originalLine, searchLine); - similarity += (1 - distance / maxLen) / linesToCheck; - - if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { - break; - } - } - } else { - similarity = 1.0; - } - - if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { - let matchStartIndex = 0; - for (let k = 0; k < startLine; k++) { - matchStartIndex += originalLines[k].length + 1; - } - let matchEndIndex = matchStartIndex; - for (let k = startLine; k <= endLine; k++) { - matchEndIndex += originalLines[k].length; - if (k < endLine) { - matchEndIndex += 1; - } - } - yield content.substring(matchStartIndex, matchEndIndex); - } - return; - } - - let bestMatch: { startLine: number; endLine: number } | null = null; - let maxSimilarity = -1; - - for (const candidate of candidates) { - const { startLine, endLine } = candidate; - const actualBlockSize = endLine - startLine + 1; - - let similarity = 0; - const linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2); - - if (linesToCheck > 0) { - for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { - const originalLine = originalLines[startLine + j].trim(); - const searchLine = searchLines[j].trim(); - const maxLen = Math.max(originalLine.length, searchLine.length); - if (maxLen === 0) { - continue; - } - const distance = levenshtein(originalLine, searchLine); - similarity += 1 - distance / maxLen; - } - similarity /= linesToCheck; - } else { - similarity = 1.0; - } - - if (similarity > maxSimilarity) { - maxSimilarity = similarity; - bestMatch = candidate; - } - } - - if (maxSimilarity >= MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD && bestMatch) { - const { startLine, endLine } = bestMatch; - let matchStartIndex = 0; - for (let k = 0; k < startLine; k++) { - matchStartIndex += originalLines[k].length + 1; - } - let matchEndIndex = matchStartIndex; - for (let k = startLine; k <= endLine; k++) { - matchEndIndex += originalLines[k].length; - if (k < endLine) { - matchEndIndex += 1; - } - } - yield content.substring(matchStartIndex, matchEndIndex); - } -}; - -export const WhitespaceNormalizedReplacer: Replacer = function* (content, find) { - const normalizeWhitespace = (text: string) => text.replace(/\s+/g, " ").trim(); - const normalizedFind = normalizeWhitespace(find); - - const lines = content.split("\n"); - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (normalizeWhitespace(line) === normalizedFind) { - yield line; - } else { - const normalizedLine = normalizeWhitespace(line); - if (normalizedLine.includes(normalizedFind)) { - const words = find.trim().split(/\s+/); - if (words.length > 0) { - const pattern = words.map((word) => word.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")).join("\\s+"); - try { - const regex = new RegExp(pattern); - const match = line.match(regex); - if (match) { - yield match[0]; - } - } catch { - // Invalid regex pattern, skip - } - } - } - } - } - - const findLines = find.split("\n"); - if (findLines.length > 1) { - for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length); - if (normalizeWhitespace(block.join("\n")) === normalizedFind) { - yield block.join("\n"); - } - } - } -}; - -export const IndentationFlexibleReplacer: Replacer = function* (content, find) { - const removeIndentation = (text: string) => { - const lines = text.split("\n"); - const nonEmptyLines = lines.filter((line) => line.trim().length > 0); - if (nonEmptyLines.length === 0) return text; - - const minIndent = Math.min( - ...nonEmptyLines.map((line) => { - const match = line.match(/^(\s*)/); - return match ? match[1].length : 0; - }) - ); - - return lines.map((line) => (line.trim().length === 0 ? line : line.slice(minIndent))).join("\n"); - }; - - const normalizedFind = removeIndentation(find); - const contentLines = content.split("\n"); - const findLines = find.split("\n"); - - for (let i = 0; i <= contentLines.length - findLines.length; i++) { - const block = contentLines.slice(i, i + findLines.length).join("\n"); - if (removeIndentation(block) === normalizedFind) { - yield block; - } - } -}; - -/** - * Normalizes common escape sequences that LLMs produce when double-escaping. - * e.g. literal \n → newline, \$ → $, \\ → \, \' → ' - */ -export function unescapeString(str: string): string { - return str.replace(/\\(n|t|r|'|"|`|\\|\n|\$)/g, (match, capturedChar: string) => { - switch (capturedChar) { - case "n": - return "\n"; - case "t": - return "\t"; - case "r": - return "\r"; - case "'": - return "'"; - case '"': - return '"'; - case "`": - return "`"; - case "\\": - return "\\"; - case "\n": - return "\n"; - case "$": - return "$"; - default: - return match; - } - }); -} - -export const EscapeNormalizedReplacer: Replacer = function* (content, find) { - - const unescapedFind = unescapeString(find); - - if (content.includes(unescapedFind)) { - yield unescapedFind; - } - - const lines = content.split("\n"); - const findLines = unescapedFind.split("\n"); - - for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length).join("\n"); - const unescapedBlock = unescapeString(block); - - if (unescapedBlock === unescapedFind) { - yield block; - } - } -}; - -export const MultiOccurrenceReplacer: Replacer = function* (content, find) { - let startIndex = 0; - - while (true) { - const index = content.indexOf(find, startIndex); - if (index === -1) break; - - yield find; - startIndex = index + find.length; - } -}; - -export const TrimmedBoundaryReplacer: Replacer = function* (content, find) { - const trimmedFind = find.trim(); - - if (trimmedFind === find) { - return; - } - - if (content.includes(trimmedFind)) { - yield trimmedFind; - } - - const lines = content.split("\n"); - const findLines = find.split("\n"); - - for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length).join("\n"); - - if (block.trim() === trimmedFind) { - yield block; - } - } -}; - -export const ContextAwareReplacer: Replacer = function* (content, find) { - const findLines = find.split("\n"); - if (findLines.length < 3) { - return; - } - - if (findLines[findLines.length - 1] === "") { - findLines.pop(); - } - - const contentLines = content.split("\n"); - const firstLine = findLines[0].trim(); - const lastLine = findLines[findLines.length - 1].trim(); - - for (let i = 0; i < contentLines.length; i++) { - if (contentLines[i].trim() !== firstLine) continue; - - for (let j = i + 2; j < contentLines.length; j++) { - if (contentLines[j].trim() === lastLine) { - const blockLines = contentLines.slice(i, j + 1); - const block = blockLines.join("\n"); - - if (blockLines.length === findLines.length) { - let matchingLines = 0; - let totalNonEmptyLines = 0; - - for (let k = 1; k < blockLines.length - 1; k++) { - const blockLine = blockLines[k].trim(); - const findLine = findLines[k].trim(); - - if (blockLine.length > 0 || findLine.length > 0) { - totalNonEmptyLines++; - if (blockLine === findLine) { - matchingLines++; - } - } - } - - if (totalNonEmptyLines === 0 || matchingLines / totalNonEmptyLines >= 0.5) { - yield block; - break; - } - } - break; - } - } - } -}; - export function trimDiff(diff: string): string { const lines = diff.split("\n"); const contentLines = lines.filter( @@ -454,9 +54,7 @@ function normalizeReplacementText(content: string, newString: string): string { let normalized = newString; const contentLooksJson = /"questions"\s*:|"cards"\s*:|^\s*[\[{]/.test(content); - // Only unwrap full-value code fences for JSON-like content to avoid - // stripping intentional markdown code blocks in workspace content edits. - if (contentLooksJson) { + if (contentLooksJson) { const unwrapped = unwrapSingleCodeFence(normalized); const unwrappedLooksJson = /^\s*[\[{]/.test(unwrapped.trim()); if (unwrapped !== normalized && unwrappedLooksJson) { @@ -478,66 +76,121 @@ function buildSearchCandidates(oldString: string): string[] { return Array.from(candidates); } -export function replace(content: string, oldString: string, newString: string, replaceAll = false): string { - if (oldString === newString) { - throw new Error("No changes to apply: oldString and newString are identical."); +export interface Edit { + oldText: string; + newText: string; +} + +export interface AppliedEditsResult { + baseContent: string; + newContent: string; +} + +function findText(content: string, oldText: string): { found: boolean; index: number; matchLength: number } { + const index = content.indexOf(oldText); + if (index !== -1) { + return { found: true, index, matchLength: oldText.length }; } + return { found: false, index: -1, matchLength: 0 }; +} - const replacementText = normalizeReplacementText(content, newString); - let notFound = true; +function countOccurrences(content: string, oldText: string): number { + return content.split(oldText).length - 1; +} - const searchCandidates = buildSearchCandidates(oldString); +export function applyEdits(content: string, edits: Edit[]): AppliedEditsResult { + const normalizedContent = normalizeLineEndings(content); + const normalizedEdits = edits.map(e => ({ + oldText: normalizeLineEndings(e.oldText), + newText: normalizeLineEndings(e.newText), + })); + + for (let i = 0; i < normalizedEdits.length; i++) { + if (normalizedEdits[i].oldText.length === 0) { + throw new Error( + normalizedEdits.length === 1 + ? `oldText must not be empty.` + : `edits[${i}].oldText must not be empty.` + ); + } + } - for (const candidate of searchCandidates) { - for (const replacer of [ - SimpleReplacer, - LineTrimmedReplacer, - BlockAnchorReplacer, - WhitespaceNormalizedReplacer, - IndentationFlexibleReplacer, - EscapeNormalizedReplacer, - TrimmedBoundaryReplacer, - ContextAwareReplacer, - MultiOccurrenceReplacer, - ]) { - for (const search of replacer(content, candidate)) { - const index = content.indexOf(search); - if (index === -1) continue; - notFound = false; - if (replaceAll) { - return content.replaceAll(search, replacementText); + const matchedEdits: Array<{ + editIndex: number; + matchIndex: number; + matchLength: number; + newText: string; + }> = []; + + for (let i = 0; i < normalizedEdits.length; i++) { + const edit = normalizedEdits[i]; + const candidates = buildSearchCandidates(edit.oldText); + const replacementText = normalizeReplacementText(normalizedContent, edit.newText); + let matched = false; + + for (const candidate of candidates) { + const result = findText(normalizedContent, candidate); + if (result.found) { + const occurrences = countOccurrences(normalizedContent, candidate); + if (occurrences > 1) { + throw new Error( + normalizedEdits.length === 1 + ? `Found ${occurrences} occurrences of the text. The text must be unique. Please provide more context to make it unique.` + : `Found ${occurrences} occurrences of edits[${i}]. Each oldText must be unique. Please provide more context to make it unique.` + ); } - const lastIndex = content.lastIndexOf(search); - if (index !== lastIndex) continue; - return content.substring(0, index) + replacementText + content.substring(index + search.length); + matchedEdits.push({ + editIndex: i, + matchIndex: result.index, + matchLength: result.matchLength, + newText: replacementText, + }); + matched = true; + break; } } - } - - if (notFound) { - const compactOld = oldString.replace(/\s+/g, " ").trim(); - const isLikelyTooShort = compactOld.length > 0 && compactOld.length <= 6; - const looksLikeTailToken = ["]", "}", "],", "},"].includes(compactOld); - const jsonLike = /"questions"\s*:|"cards"\s*:/.test(content); - const hints: string[] = []; - if (isLikelyTooShort || looksLikeTailToken) { - hints.push("oldString appears too short/ambiguous (for example: ']' or '}')."); + if (!matched) { + throw new Error( + normalizedEdits.length === 1 + ? `Could not find the exact text. The old text must match exactly including all whitespace and newlines.` + : `Could not find edits[${i}]. The oldText must match exactly including all whitespace and newlines.` + ); } - if (jsonLike) { - hints.push( - "For quiz/flashcard JSON edits, include a larger unique block (3-8 lines) with field names near the target section." + } + + matchedEdits.sort((a, b) => a.matchIndex - b.matchIndex); + for (let i = 1; i < matchedEdits.length; i++) { + const prev = matchedEdits[i - 1]; + const curr = matchedEdits[i]; + if (prev.matchIndex + prev.matchLength > curr.matchIndex) { + throw new Error( + `edits[${prev.editIndex}] and edits[${curr.editIndex}] overlap. Merge them into one edit or target disjoint regions.` ); - hints.push("If you're appending, replace the final JSON tail (for example, ' ]\\n}') with a longer unique tail."); } + } - throw new Error( - `Could not find oldString in the file. ${ - hints.join(" ") - } Ensure the snippet is copied exactly from workspace_read and is not truncated.` - ); + let newContent = normalizedContent; + for (let i = matchedEdits.length - 1; i >= 0; i--) { + const edit = matchedEdits[i]; + newContent = + newContent.substring(0, edit.matchIndex) + + edit.newText + + newContent.substring(edit.matchIndex + edit.matchLength); } - throw new Error( - "Found multiple matches for oldString. oldString is likely too generic. Use a longer, unique snippet (3-8 lines with nearby field names/structure), or set replaceAll=true only when changing every occurrence is intentional." - ); + + if (normalizedContent === newContent) { + throw new Error(`No changes made. The replacements produced identical content.`); + } + + return { baseContent: normalizedContent, newContent }; +} + +/** @deprecated Use applyEdits() for new code. Kept for any remaining callers. */ +export function replace(content: string, oldString: string, newString: string): string { + if (oldString === newString) { + throw new Error("No changes to apply: oldString and newString are identical."); + } + const { newContent } = applyEdits(content, [{ oldText: oldString, newText: newString }]); + return newContent; } diff --git a/src/lib/utils/format-workspace-context.ts b/src/lib/utils/format-workspace-context.ts index 1449289b..e732b93b 100644 --- a/src/lib/utils/format-workspace-context.ts +++ b/src/lib/utils/format-workspace-context.ts @@ -125,7 +125,7 @@ Rely only on facts from fetched content. Do not invent or assume information. RESPONSE STYLE (critical): When editing workspace items (documents, quizzes, flashcards, etc.), speak to the user in plain language. Do NOT expose internal mechanics. -- Never mention tool names (item_edit, workspace_read, workspace_search, code_execute, etc.) or parameters (oldString, newString, etc.) in your chat response. +- Never mention tool names (item_edit, workspace_read, workspace_search, code_execute, etc.) or parameters (edits, oldText, newText, etc.) in your chat response. - Never paste raw JSON, full question lists, or item content into the chat unless the user explicitly asks to see it. - Do not describe step-by-step reasoning (e.g. "Step 1: I read the quiz... Step 2: I called item_edit..."). Just state the outcome. - Use simple, user-facing language: "I've updated the quiz with harder questions" or "I've added 3 new flashcards" — not "I performed an item_edit operation with the following payload." From 37b217144da04b3764b5e9e205a1f572983fa13d Mon Sep 17 00:00:00 2001 From: urjitc <135136842+urjitc@users.noreply.github.com> Date: Fri, 10 Apr 2026 21:41:45 +0000 Subject: [PATCH 2/2] fix: multi-edit no-op validation loop + gate code-fence unwrap on JSON context - Extend oldText===newText guard to loop over all edits[] entries with indexed error messages - Pass contentLooksJson flag to buildSearchCandidates() so code-fence stripping only applies in JSON contexts, matching normalizeReplacementText() behavior - Add test for multi-edit no-op rejection Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- src/lib/ai/tools/__tests__/edit-item-tool.test.ts | 13 +++++++++++++ src/lib/ai/tools/edit-item-tool.ts | 15 ++++++++++----- src/lib/utils/edit-replace.ts | 14 +++++++------- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/lib/ai/tools/__tests__/edit-item-tool.test.ts b/src/lib/ai/tools/__tests__/edit-item-tool.test.ts index 530e4336..0afb00ac 100644 --- a/src/lib/ai/tools/__tests__/edit-item-tool.test.ts +++ b/src/lib/ai/tools/__tests__/edit-item-tool.test.ts @@ -222,4 +222,17 @@ describe("createEditItemTool", () => { ); expect(result.success).toBe(true); }); + + it("rejects no-op edit entry in multi-edit array", async () => { + const tool: any = createEditItemTool(ctx); + const result = await tool.execute({ + itemName: "My Document", + edits: [ + { oldText: "real", newText: "REAL" }, + { oldText: "same", newText: "same" }, + ], + }); + expect(result.success).toBe(false); + expect(result.message).toMatch(/edits\[1\].*identical/); + }); }); diff --git a/src/lib/ai/tools/edit-item-tool.ts b/src/lib/ai/tools/edit-item-tool.ts index b2420c8f..44e6950b 100644 --- a/src/lib/ai/tools/edit-item-tool.ts +++ b/src/lib/ai/tools/edit-item-tool.ts @@ -69,11 +69,16 @@ export function createEditItemTool(ctx: WorkspaceToolContext) { }; } - if (!isRenameOnly && edits.length === 1 && edits[0].oldText !== "" && edits[0].oldText === edits[0].newText) { - return { - success: false, - message: "No changes to apply: oldText and newText are identical.", - }; + for (let i = 0; i < edits.length; i++) { + const e = edits[i]; + if (e.oldText !== "" && e.oldText === e.newText) { + return { + success: false, + message: edits.length === 1 + ? `No changes to apply: oldText and newText are identical.` + : `No changes to apply: edits[${i}].oldText and newText are identical.`, + }; + } } if (!ctx.workspaceId) { diff --git a/src/lib/utils/edit-replace.ts b/src/lib/utils/edit-replace.ts index 0600ad23..0b796bd1 100644 --- a/src/lib/utils/edit-replace.ts +++ b/src/lib/utils/edit-replace.ts @@ -65,13 +65,12 @@ function normalizeReplacementText(content: string, newString: string): string { return normalized; } -function buildSearchCandidates(oldString: string): string[] { +function buildSearchCandidates(oldString: string, contentLooksJson: boolean): string[] { const candidates = new Set(); - const base = oldString; - const noFence = unwrapSingleCodeFence(base); - - for (const c of [base, noFence]) { - if (c.length > 0) candidates.add(c); + candidates.add(oldString); + if (contentLooksJson) { + const noFence = unwrapSingleCodeFence(oldString); + if (noFence.length > 0) candidates.add(noFence); } return Array.from(candidates); } @@ -100,6 +99,7 @@ function countOccurrences(content: string, oldText: string): number { export function applyEdits(content: string, edits: Edit[]): AppliedEditsResult { const normalizedContent = normalizeLineEndings(content); + const contentLooksJson = /"questions"\s*:|"cards"\s*:|^\s*[\[{]/.test(normalizedContent); const normalizedEdits = edits.map(e => ({ oldText: normalizeLineEndings(e.oldText), newText: normalizeLineEndings(e.newText), @@ -124,7 +124,7 @@ export function applyEdits(content: string, edits: Edit[]): AppliedEditsResult { for (let i = 0; i < normalizedEdits.length; i++) { const edit = normalizedEdits[i]; - const candidates = buildSearchCandidates(edit.oldText); + const candidates = buildSearchCandidates(edit.oldText, contentLooksJson); const replacementText = normalizeReplacementText(normalizedContent, edit.newText); let matched = false;