refactor(diff): replace custom LCS with jsdiff

This commit is contained in:
murashit 2026-07-21 19:48:42 +09:00
parent 488344a74d
commit ab0a9047f4
7 changed files with 83 additions and 119 deletions

10
package-lock.json generated
View file

@ -11,6 +11,7 @@
"dependencies": {
"@tanstack/query-core": "^5.101.2",
"defuddle": "^0.19.1",
"diff": "^9.0.0",
"mdast-util-from-markdown": "^2.0.3",
"mdast-util-to-markdown": "^2.1.2",
"micromark": "^4.0.2",
@ -5368,6 +5369,15 @@
"url": "https://github.com/sponsors/wooorm"
}
},
"node_modules/diff": {
"version": "9.0.0",
"resolved": "https://registry.npmjs.org/diff/-/diff-9.0.0.tgz",
"integrity": "sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==",
"license": "BSD-3-Clause",
"engines": {
"node": ">=0.3.1"
}
},
"node_modules/diff-match-patch": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/diff-match-patch/-/diff-match-patch-1.0.5.tgz",

View file

@ -66,6 +66,7 @@
"dependencies": {
"@tanstack/query-core": "^5.101.2",
"defuddle": "^0.19.1",
"diff": "^9.0.0",
"mdast-util-from-markdown": "^2.0.3",
"mdast-util-to-markdown": "^2.1.2",
"micromark": "^4.0.2",

View file

@ -1,20 +1,13 @@
export function buildSelectionUnifiedDiff(filePath: string, originalText: string, replacementText: string): string {
import { diffArrays } from "diff";
export function buildSelectionDiffLines(originalText: string, replacementText: string): string[] {
const originalLines = textLines(originalText);
const replacementLines = textLines(replacementText);
const changes = lineChanges(originalLines, replacementLines);
const oldCount = Math.max(originalLines.length, 1);
const newCount = Math.max(replacementLines.length, 1);
return [
`diff --git a/${filePath} b/${filePath}`,
`--- a/${filePath}`,
`+++ b/${filePath}`,
`@@ -1,${String(oldCount)} +1,${String(newCount)} @@`,
...changes.map((change) => `${change.prefix}${change.text}`),
].join("\n");
return changes.map((change) => `${change.prefix}${change.text}`);
}
const MAX_SELECTION_REWRITE_LCS_CELLS = 40_000;
const MAX_SELECTION_REWRITE_EDIT_LENGTH = 400;
interface DiffLine {
prefix: " " | "+" | "-";
@ -29,31 +22,17 @@ function textLines(text: string): string[] {
}
function lineChanges(originalLines: string[], replacementLines: string[]): DiffLine[] {
if (originalLines.length * replacementLines.length > MAX_SELECTION_REWRITE_LCS_CELLS) {
return linearLineChanges(originalLines, replacementLines);
}
const arrayChanges = diffArrays(originalLines, replacementLines, {
maxEditLength: MAX_SELECTION_REWRITE_EDIT_LENGTH,
});
if (!arrayChanges) return linearLineChanges(originalLines, replacementLines);
const lengths = lcsLengths(originalLines, replacementLines);
const changes: DiffLine[] = [];
let oldIndex = 0;
let newIndex = 0;
while (oldIndex < originalLines.length || newIndex < replacementLines.length) {
if (oldIndex < originalLines.length && newIndex < replacementLines.length && originalLines[oldIndex] === replacementLines[newIndex]) {
changes.push({ prefix: " ", text: originalLines[oldIndex] ?? "" });
oldIndex += 1;
newIndex += 1;
} else if (
newIndex < replacementLines.length &&
(oldIndex === originalLines.length || (lengths[oldIndex]?.[newIndex + 1] ?? 0) > (lengths[oldIndex + 1]?.[newIndex] ?? 0))
) {
changes.push({ prefix: "+", text: replacementLines[newIndex] ?? "" });
newIndex += 1;
} else if (oldIndex < originalLines.length) {
changes.push({ prefix: "-", text: originalLines[oldIndex] ?? "" });
oldIndex += 1;
}
}
const changes = arrayChanges.flatMap<DiffLine>((change) =>
change.value.map((text) => ({
prefix: change.added ? "+" : change.removed ? "-" : " ",
text,
})),
);
return changes.length > 0 ? changes : [{ prefix: " ", text: "" }];
}
@ -96,18 +75,3 @@ function pushLines(changes: DiffLine[], prefix: DiffLine["prefix"], lines: strin
changes.push({ prefix, text: lines[index] ?? "" });
}
}
function lcsLengths(left: string[], right: string[]): number[][] {
const rows = Array.from({ length: left.length + 1 }, () => Array.from({ length: right.length + 1 }, () => 0));
for (let leftIndex = left.length - 1; leftIndex >= 0; leftIndex -= 1) {
for (let rightIndex = right.length - 1; rightIndex >= 0; rightIndex -= 1) {
const row = rows[leftIndex];
if (row === undefined) continue;
row[rightIndex] =
left[leftIndex] === right[rightIndex]
? (rows[leftIndex + 1]?.[rightIndex + 1] ?? 0) + 1
: Math.max(rows[leftIndex + 1]?.[rightIndex] ?? 0, rows[leftIndex]?.[rightIndex + 1] ?? 0);
}
}
return rows;
}

View file

@ -6,8 +6,8 @@ import { renderUiRoot, unmountUiRoot } from "../../shared/dom/preact-root.dom";
import { syncTextareaHeight } from "../../shared/dom/textarea-autogrow.measure";
import { textareaCursorAtVisualBoundary } from "../../shared/dom/textarea-caret.measure";
import { IconButton } from "../../shared/obsidian/components.obsidian";
import { DiffLineList, unifiedDiffDisplayLines } from "../../shared/ui/diff-view";
import { buildSelectionUnifiedDiff } from "./diff";
import { DiffLineList } from "../../shared/ui/diff-view";
import { buildSelectionDiffLines } from "./diff";
import {
canApplySelectionRewrite,
type SelectionRewriteInstructionHistoryDirection,
@ -201,7 +201,7 @@ export class SelectionRewritePopover {
elements.applyButton = element;
}}
debugText={state.debugText}
diff={replacement === null ? null : buildSelectionUnifiedDiff(state.filePath, state.originalText, replacement)}
diffLines={replacement === null ? null : buildSelectionDiffLines(state.originalText, replacement)}
generating={state.status === "generating"}
hasInstruction={this.session.hasInstruction}
hasReplacement={replacement !== null}
@ -306,7 +306,7 @@ function selectionRewriteInstructionCursorOnLogicalBoundary(
interface SelectionRewritePopoverViewProps {
applyButtonRef: (element: HTMLButtonElement | null) => void;
debugText: string | null;
diff: string | null;
diffLines: readonly string[] | null;
generating: boolean;
hasInstruction: boolean;
hasReplacement: boolean;
@ -324,7 +324,7 @@ interface SelectionRewritePopoverViewProps {
function SelectionRewritePopoverView({
applyButtonRef,
debugText,
diff,
diffLines,
generating,
hasInstruction,
hasReplacement,
@ -365,7 +365,7 @@ function SelectionRewritePopoverView({
<SelectionRewriteStatus status={status} />
<pre className={`codex-panel-selection-rewrite__stream-preview${streamPreview ? "" : " is-hidden"}`}>{streamPreview}</pre>
<div className={`codex-panel-selection-rewrite__result${hasReplacement ? "" : " is-hidden"}`}>
<div className="codex-panel-selection-rewrite__diff">{diff ? <SelectionRewriteDiff diff={diff} /> : null}</div>
<div className="codex-panel-selection-rewrite__diff">{diffLines ? <SelectionRewriteDiff lines={diffLines} /> : null}</div>
<div className="codex-panel-selection-rewrite__result-actions">
<IconButton
buttonRef={applyButtonRef}
@ -407,11 +407,6 @@ function SelectionRewriteStatus({ status }: { status: SelectionRewriteSessionSta
);
}
function SelectionRewriteDiff({ diff }: { diff: string }): UiNode {
return (
<DiffLineList
lines={unifiedDiffDisplayLines(diff).filter((line) => line.kind !== "file" && !line.text.startsWith("@@"))}
className="codex-panel-selection-rewrite__diff-body"
/>
);
function SelectionRewriteDiff({ lines }: { lines: readonly string[] }): UiNode {
return <DiffLineList lines={lines.map((text) => ({ text }))} className="codex-panel-selection-rewrite__diff-body" />;
}

View file

@ -1,7 +1,9 @@
import { diffArrays } from "diff";
import type { ComponentChild as UiNode } from "preact";
const MAX_INLINE_DIFF_CHARS = 4000;
const MAX_INLINE_DIFF_TOKENS = 500;
const MAX_INLINE_DIFF_EDIT_LENGTH = MAX_INLINE_DIFF_TOKENS;
export interface DiffDisplayLine {
text: string;
@ -229,29 +231,18 @@ function inlineDiff(removedText: string, addedText: string): InlineDiff | null {
const addedTokens = segmentWords(addedText);
if (removedTokens.length + addedTokens.length > MAX_INLINE_DIFF_TOKENS) return null;
const lengths = lcsLengths(removedTokens, addedTokens);
const changes = diffArrays(removedTokens, addedTokens, {
maxEditLength: MAX_INLINE_DIFF_EDIT_LENGTH,
});
if (!changes) return null;
const removed: InlineDiffPart[] = [];
const added: InlineDiffPart[] = [];
let removedIndex = 0;
let addedIndex = 0;
while (removedIndex < removedTokens.length || addedIndex < addedTokens.length) {
const removedToken = removedTokens[removedIndex];
const addedToken = addedTokens[addedIndex];
if (removedToken !== undefined && addedToken !== undefined && removedToken === addedToken) {
removed.push({ text: removedToken, changed: false });
added.push({ text: addedToken, changed: false });
removedIndex += 1;
addedIndex += 1;
} else if (
addedToken !== undefined &&
(removedToken === undefined || (lengths[removedIndex]?.[addedIndex + 1] ?? 0) >= (lengths[removedIndex + 1]?.[addedIndex] ?? 0))
) {
added.push({ text: addedToken, changed: true });
addedIndex += 1;
} else if (removedToken !== undefined) {
removed.push({ text: removedToken, changed: true });
removedIndex += 1;
for (const change of changes) {
const changed = change.added || change.removed;
for (const text of change.value) {
if (!change.added) removed.push({ text, changed });
if (!change.removed) added.push({ text, changed });
}
}
@ -263,21 +254,6 @@ function segmentWords(text: string): string[] {
return Array.from(segmenter.segment(text), (segment) => segment.segment);
}
function lcsLengths(left: string[], right: string[]): number[][] {
const rows = Array.from({ length: left.length + 1 }, () => Array.from({ length: right.length + 1 }, () => 0));
for (let leftIndex = left.length - 1; leftIndex >= 0; leftIndex -= 1) {
for (let rightIndex = right.length - 1; rightIndex >= 0; rightIndex -= 1) {
const row = rows[leftIndex];
if (!row) continue;
row[rightIndex] =
left[leftIndex] === right[rightIndex]
? (rows[leftIndex + 1]?.[rightIndex + 1] ?? 0) + 1
: Math.max(rows[leftIndex + 1]?.[rightIndex] ?? 0, rows[leftIndex]?.[rightIndex + 1] ?? 0);
}
}
return rows;
}
function mergeInlineParts(parts: InlineDiffPart[]): InlineDiffPart[] {
const merged: InlineDiffPart[] = [];
for (const part of parts) {

View file

@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { TurnRecord } from "../../../src/app-server/protocol/turn";
import type { EphemeralStructuredTurnRunner } from "../../../src/app-server/services/ephemeral-structured-turn";
import { createAppServerSelectionRewriteTransport } from "../../../src/features/selection-rewrite/app-server-transport";
import { buildSelectionUnifiedDiff } from "../../../src/features/selection-rewrite/diff";
import { buildSelectionDiffLines } from "../../../src/features/selection-rewrite/diff";
import { canApplySelectionRewrite, type SelectionRewriteState } from "../../../src/features/selection-rewrite/model";
import { SelectionRewriteOutputError, selectionRewriteOutputParseResultFromText } from "../../../src/features/selection-rewrite/output";
import { SelectionRewritePopover } from "../../../src/features/selection-rewrite/popover.dom";
@ -90,24 +90,17 @@ describe("selection rewrite prompt", () => {
});
describe("selection rewrite diff", () => {
it("renders replacements in a selection-scoped unified diff", () => {
const diff = buildSelectionUnifiedDiff("Note.md", "alpha\nbeta", "alpha\ngamma");
expect(diff).toContain("diff --git a/Note.md b/Note.md");
expect(diff).toContain("@@ -1,2 +1,2 @@");
expect(diff).toContain(" alpha");
expect(diff).toContain("-beta");
expect(diff).toContain("+gamma");
it("builds selection-scoped display lines", () => {
expect(buildSelectionDiffLines("alpha\nbeta", "alpha\ngamma")).toEqual([" alpha", "-beta", "+gamma"]);
});
it("orders full replacement blocks as removals before additions", () => {
const diff = buildSelectionUnifiedDiff(
"Note.md",
const diff = buildSelectionDiffLines(
"これはdiffのテストです。\n今日は元気です。\nとても元気です。",
"これはdiffのてすとです。\nきょうはげんきです。\nとてもげんきです。",
);
expect(diff).toContain(
expect(diff.join("\n")).toContain(
[
"-これはdiffのテストです。",
"-今日は元気です。",
@ -119,7 +112,18 @@ describe("selection rewrite diff", () => {
);
});
it("bounds large selection diffs to a linear prefix and suffix comparison", () => {
it("keeps accurate context for large selections with a small edit distance", () => {
const commonBefore = Array.from({ length: 160 }, (_unused, index) => `common before ${String(index)}`);
const commonAfter = Array.from({ length: 160 }, (_unused, index) => `common after ${String(index)}`);
const originalText = ["same start", ...commonBefore, "old line", "shared middle", ...commonAfter, "same end"].join("\n");
const replacementText = ["same start", ...commonBefore, "new line", "shared middle", ...commonAfter, "same end"].join("\n");
const diff = buildSelectionDiffLines(originalText, replacementText).join("\n");
expect(diff).toContain("\n-old line\n+new line\n shared middle\n");
});
it("bounds high-edit-distance selection diffs with a linear prefix and suffix fallback", () => {
const originalText = [
"same start",
...Array.from({ length: 160 }, (_unused, index) => `old before ${String(index)}`),
@ -135,23 +139,23 @@ describe("selection rewrite diff", () => {
"same end",
].join("\n");
const diff = buildSelectionUnifiedDiff("Note.md", originalText, replacementText);
const diff = buildSelectionDiffLines(originalText, replacementText).join("\n");
expect(diff).toContain("\n same start\n");
expect(diff.startsWith(" same start\n")).toBe(true);
expect(diff).toContain("\n-old before 0\n");
expect(diff).toContain("\n-shared middle\n");
expect(diff).toContain("\n+shared middle\n");
expect(diff).toContain("\n same end");
expect(diff.endsWith("\n same end")).toBe(true);
expect(diff).not.toContain("\n shared middle\n");
});
it("renders additions and deletions", () => {
expect(buildSelectionUnifiedDiff("Note.md", "", "added")).toContain("+added");
expect(buildSelectionUnifiedDiff("Note.md", "removed", "")).toContain("-removed");
expect(buildSelectionDiffLines("", "added")).toContain("+added");
expect(buildSelectionDiffLines("removed", "")).toContain("-removed");
});
it("renders unchanged text as context", () => {
expect(buildSelectionUnifiedDiff("Note.md", "same", "same")).toContain(" same");
expect(buildSelectionDiffLines("same", "same")).toContain(" same");
});
});

View file

@ -82,6 +82,20 @@ describe("turn diff view decisions", () => {
expect(parent.querySelector(".codex-panel-diff__word--added")?.textContent).toBe("犬");
});
it("preserves whitespace changes from Intl.Segmenter tokenization", () => {
const parent = document.createElement("div");
renderTurnDiffView(parent, {
threadId: "thread",
turnId: "turn",
files: ["Note.md"],
diff: "diff --git a/Note.md b/Note.md\n@@\n-alpha beta\n+alpha beta",
});
expect(parent.querySelector(".codex-panel-diff__word--removed")?.textContent).toBe(" ");
expect(parent.querySelector(".codex-panel-diff__word--added")?.textContent).toBe(" ");
});
it("pairs changed words by line inside multi-line replacement blocks", () => {
const parent = document.createElement("div");