From 3df8a9a5d3da868987dc8239ca722d24bbee4e69 Mon Sep 17 00:00:00 2001 From: Logan Rupe Date: Wed, 9 Sep 2026 15:20:31 +1000 Subject: [PATCH] fix(web): namespace the diff file tree by repo in multi-repo turn diffs Two repo roots in one turn diff that both changed the same relative path produced two tree entries at the same path, and Pierre's PathStore threw. The tree now builds from the per-repo groups the panel already renders, with each root's files under a folder named for that root. A repeated path is dropped instead of handed to the tree. Fixes #240 --- apps/web/src/components/DiffPanel.tsx | 40 +++++++++++-- .../diffs/diffFileTree.logic.test.ts | 41 +++++++++++++ .../components/diffs/diffFileTree.logic.ts | 58 ++++++++++++++++++- 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 44439d7a8fb0..402712cc1b37 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -63,7 +63,11 @@ import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./Dif import { DiffStatLabel } from "./chat/DiffStatLabel"; import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./diffs/AnnotatableCodeView"; import { DiffFileTree } from "./diffs/DiffFileTree"; -import { diffFileTreeEntries } from "./diffs/diffFileTree.logic"; +import { + diffFileTreeEntries, + groupedDiffFileTreeEntries, + groupedDiffFileTreePath, +} from "./diffs/diffFileTree.logic"; import { Button } from "./ui/button"; import { ToggleGroup, Toggle } from "./ui/toggle-group"; import { Switch } from "./ui/switch"; @@ -643,7 +647,6 @@ export default function DiffPanel({ const diffFileKeys = useMemo(() => codeViewFiles.map((file) => file.fileKey), [codeViewFiles]); const allDiffFilesCollapsed = areAllDiffFilesCollapsed(diffFileKeys, collapsedDiffFileKeys); const diffLineStat = useMemo(() => getDiffLineStat(renderableFiles), [renderableFiles]); - const fileTreeEntries = useMemo(() => diffFileTreeEntries(renderableFiles), [renderableFiles]); const selectedDiffFileKey = selectedFilePath ? (codeViewFiles.find((candidate) => candidate.filePath === selectedFilePath)?.fileKey ?? null) : null; @@ -703,9 +706,34 @@ export default function DiffPanel({ const visibleDiffTargets = effectiveRepoFilter ? diffRepoTargets.filter((entry) => repoRootBaseName(entry.repoRoot) === effectiveRepoFilter) : diffRepoTargets; - const visibleGroups = effectiveRepoFilter - ? renderableGroups.filter((group) => repoRootBaseName(group.repoRoot) === effectiveRepoFilter) - : renderableGroups; + const visibleGroups = useMemo( + () => + effectiveRepoFilter + ? renderableGroups.filter( + (group) => repoRootBaseName(group.repoRoot) === effectiveRepoFilter, + ) + : renderableGroups, + [effectiveRepoFilter, renderableGroups], + ); + + // The tree mirrors what the diff draws. A grouped view gets one folder per + // repo section (named like the section header), so two roots that changed the + // same relative path are two rows instead of a duplicate-path crash. + const fileTreeGroups = useMemo( + () => visibleGroups.map((group) => ({ label: group.displayName, files: group.files })), + [visibleGroups], + ); + const fileTreeEntries = useMemo( + () => + isGroupedDiffView + ? groupedDiffFileTreeEntries(fileTreeGroups) + : diffFileTreeEntries(renderableFiles), + [fileTreeGroups, isGroupedDiffView, renderableFiles], + ); + const selectedFileTreePath = + selectedFilePath && isGroupedDiffView + ? groupedDiffFileTreePath(fileTreeGroups, selectedFilePath) + : selectedFilePath; useEffect(() => { if (!selectedDiffFileKey || !codeView?.getInstance()) return; @@ -1463,7 +1491,7 @@ export default function DiffPanel({ diff --git a/apps/web/src/components/diffs/diffFileTree.logic.test.ts b/apps/web/src/components/diffs/diffFileTree.logic.test.ts index d8e24968dcea..782c327e1016 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.test.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.test.ts @@ -5,6 +5,8 @@ import { buildDiffFileTreeUpdates, collectDirectoryPaths, diffFileTreeEntries, + groupedDiffFileTreeEntries, + groupedDiffFileTreePath, } from "./diffFileTree.logic"; function file(type: FileDiffMetadata["type"], name: string, prevName = name): FileDiffMetadata { @@ -29,6 +31,45 @@ describe("diffFileTreeEntries", () => { { path: "README.md", status: "modified" }, ]); }); + + it("keeps the first of two files at the same path so the tree never throws on a duplicate", () => { + expect(diffFileTreeEntries([file("change", "README.md"), file("new", "README.md")])).toEqual([ + { path: "README.md", status: "modified" }, + ]); + }); +}); + +describe("groupedDiffFileTreeEntries", () => { + const groups = [ + { label: "api", files: [file("change", "README.md"), file("new", "src/a.ts")] }, + { label: "web", files: [file("change", "README.md")] }, + ]; + + it("files each repo's changes under a folder named for the repo", () => { + expect(groupedDiffFileTreeEntries(groups)).toEqual([ + { path: "api/README.md", status: "modified" }, + { path: "api/src/a.ts", status: "added" }, + { path: "web/README.md", status: "modified" }, + ]); + }); + + it("drops a repeat when two roots share a folder name", () => { + expect( + groupedDiffFileTreeEntries([ + { label: "app", files: [file("change", "README.md")] }, + { label: "app", files: [file("change", "README.md"), file("new", "b.ts")] }, + ]), + ).toEqual([ + { path: "app/README.md", status: "modified" }, + { path: "app/b.ts", status: "added" }, + ]); + }); + + it("resolves a repo-relative path to the first group that changed it", () => { + expect(groupedDiffFileTreePath(groups, "README.md")).toBe("api/README.md"); + expect(groupedDiffFileTreePath(groups, "src/a.ts")).toBe("api/src/a.ts"); + expect(groupedDiffFileTreePath(groups, "missing.ts")).toBeNull(); + }); }); describe("collectDirectoryPaths", () => { diff --git a/apps/web/src/components/diffs/diffFileTree.logic.ts b/apps/web/src/components/diffs/diffFileTree.logic.ts index 4535ece8b143..cd13ca586fe6 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.ts @@ -23,11 +23,65 @@ function toGitStatus(file: FileDiffMetadata): GitStatus { } } -/** Maps parsed diff files to tree entries, keeping the diff's own order. */ +/** + * Maps parsed diff files to tree entries, keeping the diff's own order. A path that repeats + * keeps its first entry: Pierre's path store throws on a duplicate, and a tree missing a row + * beats a diff panel that cannot render at all. + */ export function diffFileTreeEntries( files: ReadonlyArray, ): ReadonlyArray { - return files.map((file) => ({ path: resolveFileDiffPath(file), status: toGitStatus(file) })); + const entries: DiffFileTreeEntry[] = []; + appendDiffFileTreeEntries(entries, new Set(), files, ""); + return entries; +} + +function appendDiffFileTreeEntries( + entries: DiffFileTreeEntry[], + seen: Set, + files: ReadonlyArray, + pathPrefix: string, +): void { + for (const file of files) { + const path = `${pathPrefix}${resolveFileDiffPath(file)}`; + if (seen.has(path)) continue; + seen.add(path); + entries.push({ path, status: toGitStatus(file) }); + } +} + +/** A group of changed files under one repo root of a multi-repo diff. */ +export interface DiffFileTreeGroup { + /** Folder name the group's files sit under in the tree, matching the diff's section header. */ + readonly label: string; + readonly files: ReadonlyArray; +} + +/** + * Tree entries for a diff that spans several repo roots. Each root's files sit under a folder + * named for that root, so two roots that both changed `README.md` stay two rows, the same way + * the diff draws one section per root. + */ +export function groupedDiffFileTreeEntries( + groups: ReadonlyArray, +): ReadonlyArray { + const entries: DiffFileTreeEntry[] = []; + const seen = new Set(); + for (const group of groups) { + appendDiffFileTreeEntries(entries, seen, group.files, `${group.label}/`); + } + return entries; +} + +/** The tree path a repo-relative file takes inside a grouped tree, or null when no group has it. */ +export function groupedDiffFileTreePath( + groups: ReadonlyArray, + filePath: string, +): string | null { + const group = groups.find((candidate) => + candidate.files.some((file) => resolveFileDiffPath(file) === filePath), + ); + return group ? `${group.label}/${filePath}` : null; } /**