From 7c86d8981031a613c264bb2cfbe3a4bb1e2d8775 Mon Sep 17 00:00:00 2001 From: murashit Date: Sun, 28 Jun 2026 17:46:17 +0900 Subject: [PATCH] Resolve settings display and selection diff design issues --- src/features/selection-rewrite/diff.ts | 45 +++++++++++++++++++ src/settings/dynamic-sections-controller.ts | 20 +++------ .../selection-rewrite.test.ts | 26 +++++++++++ tests/settings/settings-tab.test.ts | 2 +- 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/features/selection-rewrite/diff.ts b/src/features/selection-rewrite/diff.ts index e40acd6d..ae0d6e5e 100644 --- a/src/features/selection-rewrite/diff.ts +++ b/src/features/selection-rewrite/diff.ts @@ -14,6 +14,8 @@ export function buildSelectionUnifiedDiff(filePath: string, originalText: string ].join("\n"); } +const MAX_SELECTION_REWRITE_LCS_CELLS = 40_000; + interface DiffLine { prefix: " " | "+" | "-"; text: string; @@ -27,6 +29,10 @@ 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 lengths = lcsLengths(originalLines, replacementLines); const changes: DiffLine[] = []; let oldIndex = 0; @@ -52,6 +58,45 @@ function lineChanges(originalLines: string[], replacementLines: string[]): DiffL return changes.length > 0 ? changes : [{ prefix: " ", text: "" }]; } +function linearLineChanges(originalLines: string[], replacementLines: string[]): DiffLine[] { + const prefixLength = commonPrefixLength(originalLines, replacementLines); + const suffixLength = commonSuffixLength(originalLines, replacementLines, prefixLength); + const changes: DiffLine[] = []; + + pushLines(changes, " ", originalLines, 0, prefixLength); + pushLines(changes, "-", originalLines, prefixLength, originalLines.length - suffixLength); + pushLines(changes, "+", replacementLines, prefixLength, replacementLines.length - suffixLength); + pushLines(changes, " ", originalLines, originalLines.length - suffixLength, originalLines.length); + + return changes.length > 0 ? changes : [{ prefix: " ", text: "" }]; +} + +function commonPrefixLength(left: string[], right: string[]): number { + let prefixLength = 0; + while (prefixLength < left.length && prefixLength < right.length && left[prefixLength] === right[prefixLength]) { + prefixLength += 1; + } + return prefixLength; +} + +function commonSuffixLength(left: string[], right: string[], prefixLength: number): number { + let suffixLength = 0; + while ( + suffixLength < left.length - prefixLength && + suffixLength < right.length - prefixLength && + left[left.length - 1 - suffixLength] === right[right.length - 1 - suffixLength] + ) { + suffixLength += 1; + } + return suffixLength; +} + +function pushLines(changes: DiffLine[], prefix: DiffLine["prefix"], lines: string[], start: number, end: number): void { + for (let index = start; index < end; index += 1) { + 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) { diff --git a/src/settings/dynamic-sections-controller.ts b/src/settings/dynamic-sections-controller.ts index 83cac402..5aae85f5 100644 --- a/src/settings/dynamic-sections-controller.ts +++ b/src/settings/dynamic-sections-controller.ts @@ -13,12 +13,10 @@ import { } from "./lifecycle"; interface SettingsDynamicSectionsControllerCallbacks { - display(target: SettingsDynamicSectionsDisplayTarget): void; + display(): void; notify(message: string): void; } -type SettingsDynamicSectionsDisplayTarget = "all" | "helper" | "archived" | "hooks"; - export interface SettingsDynamicSectionsSnapshot { archivedThreads: readonly Thread[]; archivedThreadsLifecycle: SettingsDynamicSectionLifecycleState; @@ -119,7 +117,7 @@ export class SettingsDynamicSectionsController { const observedModels = observedValue(result); if (!observedModels) return; this.models = [...observedModels]; - this.callbacks.display("helper"); + this.callbacks.display(); } private receiveObservedArchivedThreadsResult(result: ObservedResult): void { @@ -134,7 +132,7 @@ export class SettingsDynamicSectionsController { operationToken: this.archivedThreadsOperationToken, }); } - this.callbacks.display("archived"); + this.callbacks.display(); } async refreshDynamicSections(options: { forceModels?: boolean } = {}): Promise { @@ -158,7 +156,7 @@ export class SettingsDynamicSectionsController { status: "Loading hooks...", operationToken: hooksOperationToken, }); - this.callbacks.display("all"); + this.callbacks.display(); let failedCount = 0; try { @@ -261,7 +259,7 @@ export class SettingsDynamicSectionsController { if (failedCount > 0) { this.callbacks.notify("Could not refresh all Codex details."); } - this.callbacks.display("all"); + this.callbacks.display(); } } } @@ -435,7 +433,7 @@ export class SettingsDynamicSectionsController { operationToken, }), ); - this.callbacks.display(displayTargetForDynamicOperationSection(options.section)); + this.callbacks.display(); try { await options.operation(operationToken); } catch (error) { @@ -449,7 +447,7 @@ export class SettingsDynamicSectionsController { ); this.callbacks.notify(options.failureNotice); } finally { - if (!stale()) this.callbacks.display(displayTargetForDynamicOperationSection(options.section)); + if (!stale()) this.callbacks.display(); } } @@ -490,10 +488,6 @@ export class SettingsDynamicSectionsController { } } -function displayTargetForDynamicOperationSection(section: "hooks" | "archivedThreads"): SettingsDynamicSectionsDisplayTarget { - return section === "hooks" ? "hooks" : "archived"; -} - function archivedThreadsStatus(count: number): string { return `Loaded ${String(count)} archived thread${count === 1 ? "" : "s"}.`; } diff --git a/tests/features/selection-rewrite/selection-rewrite.test.ts b/tests/features/selection-rewrite/selection-rewrite.test.ts index 07f19233..c7e39622 100644 --- a/tests/features/selection-rewrite/selection-rewrite.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite.test.ts @@ -133,6 +133,32 @@ describe("selection rewrite diff", () => { ); }); + it("bounds large selection diffs to a linear prefix and suffix comparison", () => { + const originalText = [ + "same start", + ...Array.from({ length: 160 }, (_unused, index) => `old before ${String(index)}`), + "shared middle", + ...Array.from({ length: 160 }, (_unused, index) => `old after ${String(index)}`), + "same end", + ].join("\n"); + const replacementText = [ + "same start", + ...Array.from({ length: 160 }, (_unused, index) => `new before ${String(index)}`), + "shared middle", + ...Array.from({ length: 160 }, (_unused, index) => `new after ${String(index)}`), + "same end", + ].join("\n"); + + const diff = buildSelectionUnifiedDiff("Note.md", originalText, replacementText); + + expect(diff).toContain("\n same start\n"); + 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).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"); diff --git a/tests/settings/settings-tab.test.ts b/tests/settings/settings-tab.test.ts index b089df4d..a347894b 100644 --- a/tests/settings/settings-tab.test.ts +++ b/tests/settings/settings-tab.test.ts @@ -300,7 +300,7 @@ describe("settings tab", () => { expect(controller.snapshot().archivedThreads.map((thread) => thread.preview)).toEqual(["Archived elsewhere"]); expect(controller.snapshot().archivedThreadsLifecycle.kind).toBe("loaded"); - expect(display).toHaveBeenCalledWith("archived"); + expect(display).toHaveBeenCalledOnce(); }); it("ignores stale dynamic sections refresh results after a newer refresh completes", async () => {