From d412972cd5e961ac37e66eb36db018ad1b0862c7 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Sat, 28 Feb 2026 16:31:51 +0000 Subject: [PATCH] refactor(site): use `diff` library for inline tool diffs (#22423) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hand-rolled LCS diffing in `buildEditDiff` and the manual patch-string assembly in `buildWriteFileDiff` with [`Diff.createPatch()`](https://www.npmjs.com/package/diff) from the `diff` npm package. Both functions now just call `Diff.createPatch()` and feed the result straight into `parsePatchFiles()`, removing all the manual line splitting, prefix tagging, hunk-header arithmetic, and trailing-newline cleanup. ### Changes - Add `diff` as a dependency - `buildWriteFileDiff`: replaced ~20 lines of manual patch assembly with a single `Diff.createPatch()` call - `buildEditDiff`: replaced ~60 lines (line splitting, `Diff.diffLines` → prefixed strings, hunk counting) with a `Diff.createPatch()` call per edit - Removed the `chunkLines` helper and the `diffLines` wrapper + its test block Net: +21 / -157 lines across source and tests. --- site/package.json | 1 + site/pnpm-lock.yaml | 3 + .../components/ai-elements/tool.stories.tsx | 175 ++++++++++++++++++ .../components/ai-elements/tool/utils.test.ts | 15 ++ site/src/components/ai-elements/tool/utils.ts | 72 ++----- 5 files changed, 211 insertions(+), 55 deletions(-) diff --git a/site/package.json b/site/package.json index ceff82a28d..0a2bc26baf 100644 --- a/site/package.json +++ b/site/package.json @@ -83,6 +83,7 @@ "cron-parser": "4.9.0", "cronstrue": "2.59.0", "dayjs": "1.11.19", + "diff": "8.0.3", "emoji-mart": "5.6.0", "file-saver": "2.0.5", "formik": "2.4.9", diff --git a/site/pnpm-lock.yaml b/site/pnpm-lock.yaml index a21ba16ce1..2c075a6e31 100644 --- a/site/pnpm-lock.yaml +++ b/site/pnpm-lock.yaml @@ -163,6 +163,9 @@ importers: dayjs: specifier: 1.11.19 version: 1.11.19 + diff: + specifier: 8.0.3 + version: 8.0.3 emoji-mart: specifier: 5.6.0 version: 5.6.0 diff --git a/site/src/components/ai-elements/tool.stories.tsx b/site/src/components/ai-elements/tool.stories.tsx index fc210dd966..85e581cbc6 100644 --- a/site/src/components/ai-elements/tool.stories.tsx +++ b/site/src/components/ai-elements/tool.stories.tsx @@ -468,3 +468,178 @@ export const TaskNameGenericRendering: Story = { expect(canvas.queryByRole("link", { name: "View agent" })).toBeNull(); }, }; + +// --------------------------------------------------------------------------- +// WriteFile stories +// --------------------------------------------------------------------------- + +export const WriteFileRunning: Story = { + args: { + name: "write_file", + status: "running", + args: { + path: "src/utils/helpers.ts", + content: + "export function greet(name: string): string {\n return `Hello, ${name}!`;\n}\n", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Writing helpers\.ts/)).toBeInTheDocument(); + }, +}; + +export const WriteFileSuccess: Story = { + args: { + name: "write_file", + status: "completed", + args: { + path: "src/utils/helpers.ts", + content: + "export function greet(name: string): string {\n return `Hello, ${name}!`;\n}\n", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Wrote helpers\.ts/)).toBeInTheDocument(); + }, +}; + +// --------------------------------------------------------------------------- +// EditFiles stories +// --------------------------------------------------------------------------- + +export const EditFilesSingleRunning: Story = { + args: { + name: "edit_files", + status: "running", + args: { + files: [ + { + path: "src/config.ts", + edits: [ + { + search: "const timeout = 30;", + replace: "const timeout = 60;", + }, + ], + }, + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Editing config\.ts/)).toBeInTheDocument(); + }, +}; + +export const EditFilesSingleSuccess: Story = { + args: { + name: "edit_files", + status: "completed", + args: { + files: [ + { + path: "src/config.ts", + edits: [ + { + search: "const timeout = 30;", + replace: "const timeout = 60;", + }, + ], + }, + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Edited config\.ts/)).toBeInTheDocument(); + }, +}; + +export const EditFilesMultipleSuccess: Story = { + args: { + name: "edit_files", + status: "completed", + args: { + files: [ + { + path: "src/config.ts", + edits: [ + { + search: "const timeout = 30;", + replace: "const timeout = 60;", + }, + ], + }, + { + path: "src/server.ts", + edits: [ + { + search: 'const host = "localhost";', + replace: 'const host = "0.0.0.0";', + }, + ], + }, + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Edited 2 files/)).toBeInTheDocument(); + }, +}; + +/** + * Exercises the LCS-based interleaved diff: only the first and last + * lines change while the middle line stays the same, so the viewer + * should show context around the modifications instead of removing + * everything then re-adding everything. + */ +export const EditFilesInterleavedContext: Story = { + args: { + name: "edit_files", + status: "completed", + args: { + files: [ + { + path: "src/constants.ts", + edits: [ + { + search: + 'const API_URL = "http://localhost:3000";\nconst RETRY_COUNT = 3;\nconst TIMEOUT_MS = 5000;', + replace: + 'const API_URL = "https://api.prod.example.com";\nconst RETRY_COUNT = 3;\nconst TIMEOUT_MS = 10000;', + }, + ], + }, + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Edited constants\.ts/)).toBeInTheDocument(); + }, +}; + +export const EditFilesError: Story = { + args: { + name: "edit_files", + status: "error", + isError: true, + args: { + files: [ + { + path: "src/missing.ts", + edits: [ + { + search: "old", + replace: "new", + }, + ], + }, + ], + }, + result: { error: "File not found" }, + }, +}; diff --git a/site/src/components/ai-elements/tool/utils.test.ts b/site/src/components/ai-elements/tool/utils.test.ts index fdb36a07a4..b899712bcb 100644 --- a/site/src/components/ai-elements/tool/utils.test.ts +++ b/site/src/components/ai-elements/tool/utils.test.ts @@ -553,6 +553,21 @@ describe("buildEditDiff", () => { ]); expect(diff).not.toBeNull(); }); + + it("preserves unchanged lines as context in hunks", () => { + const diff = buildEditDiff("file.ts", [ + { + search: "const x = 1;\nconst y = 2;\nconst z = 3;", + replace: "const x = 10;\nconst y = 2;\nconst z = 30;", + }, + ]); + expect(diff).not.toBeNull(); + const hunk = diff!.hunks[0]; + // The hunk should contain context blocks for the unchanged + // middle line rather than removing and re-adding everything. + const hasContext = hunk.hunkContent.some((c) => c.type === "context"); + expect(hasContext).toBe(true); + }); }); describe("constants", () => { diff --git a/site/src/components/ai-elements/tool/utils.ts b/site/src/components/ai-elements/tool/utils.ts index 4aaf2e92a7..44f909e599 100644 --- a/site/src/components/ai-elements/tool/utils.ts +++ b/site/src/components/ai-elements/tool/utils.ts @@ -1,5 +1,6 @@ import type { FileDiffMetadata } from "@pierre/diffs"; import { parsePatchFiles } from "@pierre/diffs"; +import * as Diff from "diff"; import type React from "react"; import { asRecord, asString } from "../runtimeTypeUtils"; @@ -256,29 +257,10 @@ export const buildWriteFileDiff = ( path: string, content: string, ): FileDiffMetadata | null => { - const lines = content.split("\n"); - // Remove trailing empty line produced by a final newline. - if (lines.length > 0 && lines[lines.length - 1] === "") { - lines.pop(); - } - if (lines.length === 0) { - return null; - } - - const patchLines = [ - `diff --git a/${path} b/${path}`, - "new file mode 100644", - "--- /dev/null", - `+++ b/${path}`, - `@@ -0,0 +1,${lines.length} @@`, - ...lines.map((l) => `+${l}`), - ]; - const patch = `${patchLines.join("\n")}\n`; - + if (!content) return null; + const patch = Diff.createPatch(path, "", content, "", ""); const parsed = parsePatchFiles(patch); - if (!parsed.length || !parsed[0].files.length) { - return null; - } + if (!parsed.length || !parsed[0].files.length) return null; return parsed[0].files[0]; }; @@ -331,9 +313,9 @@ export const parseEditFilesArgs = (args: unknown): EditFilesFileEntry[] => { /** * Builds a synthetic unified diff from search/replace edit pairs - * for a single file. Each pair becomes a separate hunk in the - * diff. Line numbers are synthetic since we don't have the full - * file content. + * for a single file. Each edit becomes a separate + * `Diff.createPatch` call; the patches are concatenated and + * parsed into a single FileDiffMetadata. */ export const buildEditDiff = ( path: string, @@ -345,41 +327,21 @@ export const buildEditDiff = ( // produce a double-slash that confuses the diff parser. const diffPath = path.startsWith("/") ? path.slice(1) : path; - const patchLines: string[] = [ - `diff --git a/${diffPath} b/${diffPath}`, - `--- a/${diffPath}`, - `+++ b/${diffPath}`, - ]; - - let lineOffset = 1; + const patches: string[] = []; for (const edit of edits) { if (!edit.search) continue; - const searchLines = edit.search.split("\n"); - const replaceLines = edit.replace.split("\n"); - - // Remove trailing empty line produced by a final newline. - if (searchLines.length > 0 && searchLines[searchLines.length - 1] === "") { - searchLines.pop(); - } - if ( - replaceLines.length > 0 && - replaceLines[replaceLines.length - 1] === "" - ) { - replaceLines.pop(); - } - if (searchLines.length === 0 && replaceLines.length === 0) continue; - - patchLines.push( - `@@ -${lineOffset},${searchLines.length} +${lineOffset},${replaceLines.length} @@`, + patches.push(Diff.createPatch(diffPath, edit.search, edit.replace, "", "")); + } + if (!patches.length) { + // All edits were skipped (empty search). Produce a + // header-only patch so the parser still returns a file + // entry with zero hunks. + patches.push( + `Index: ${diffPath}\n===================================================================\n--- ${diffPath}\n+++ ${diffPath}\n`, ); - for (const l of searchLines) patchLines.push(`-${l}`); - for (const l of replaceLines) patchLines.push(`+${l}`); - - lineOffset += Math.max(searchLines.length, replaceLines.length) + 1; } - const patch = `${patchLines.join("\n")}\n`; - const parsed = parsePatchFiles(patch); + const parsed = parsePatchFiles(patches.join("")); if (!parsed.length || !parsed[0].files.length) return null; return parsed[0].files[0]; };