diff --git a/apps/web/components/review/review-file-tree.tsx b/apps/web/components/review/review-file-tree.tsx index 196b246156..f2587a8c1e 100644 --- a/apps/web/components/review/review-file-tree.tsx +++ b/apps/web/components/review/review-file-tree.tsx @@ -165,11 +165,16 @@ function TreeNode({ }; if (node.isDir) { + // Repo roots get a stronger label so the user sees the multi-repo grouping. + const fileCount = node.isRepoRoot ? countLeafFiles(node) : 0; return ( -
+
{expanded && node.children && (
@@ -193,3 +208,12 @@ function TreeNode({ return ; } + +/** Counts leaf (file) nodes anywhere under the given node. Used for the + * "N files" badge on multi-repo repo-root headers. */ +function countLeafFiles(node: FileTreeNode): number { + if (!node.isDir) return 1; + let total = 0; + for (const child of node.children ?? []) total += countLeafFiles(child); + return total; +} diff --git a/apps/web/components/review/types.multi-repo.test.ts b/apps/web/components/review/types.multi-repo.test.ts new file mode 100644 index 0000000000..cb51c9b01b --- /dev/null +++ b/apps/web/components/review/types.multi-repo.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect } from "vitest"; +import { buildFileTree, type ReviewFile } from "./types"; + +function file(overrides: Partial): ReviewFile { + return { + path: "src/app.tsx", + diff: "", + status: "modified", + additions: 1, + deletions: 0, + staged: false, + source: "uncommitted", + ...overrides, + }; +} + +describe("buildFileTree — multi-repo", () => { + it("falls back to flat tree when files have no repository_name", () => { + const tree = buildFileTree([file({ path: "src/a.ts" }), file({ path: "src/b.ts" })]); + expect(tree[0].isRepoRoot).toBeUndefined(); + expect(tree[0].name).toContain("src"); + }); + + it("falls back to flat tree when only one repository is present", () => { + const tree = buildFileTree([ + file({ path: "src/a.ts", repository_name: "frontend", repository_id: "f" }), + file({ path: "src/b.ts", repository_name: "frontend", repository_id: "f" }), + ]); + expect(tree[0].isRepoRoot).toBeUndefined(); + }); + + it("groups by repo when 2+ distinct repositories are present", () => { + const tree = buildFileTree([ + file({ path: "src/app.tsx", repository_name: "frontend", repository_id: "f" }), + file({ path: "src/api.ts", repository_name: "frontend", repository_id: "f" }), + file({ path: "handlers/task.go", repository_name: "backend", repository_id: "b" }), + ]); + expect(tree).toHaveLength(2); + expect(tree.map((n) => n.name).sort()).toEqual(["backend", "frontend"]); + for (const root of tree) { + expect(root.isRepoRoot).toBe(true); + expect(root.repositoryId).toBeDefined(); + } + }); + + it("repo roots are not collapsed even when they have a single child", () => { + const tree = buildFileTree([ + file({ path: "lonely.ts", repository_name: "shared", repository_id: "s" }), + file({ path: "src/x.ts", repository_name: "main", repository_id: "m" }), + ]); + const shared = tree.find((n) => n.name === "shared"); + expect(shared).toBeDefined(); + expect(shared?.isRepoRoot).toBe(true); + expect(shared?.children).toHaveLength(1); + expect(shared?.children?.[0].name).toBe("lonely.ts"); + }); + + it("preserves file paths inside each repo (no leakage between repos)", () => { + const tree = buildFileTree([ + file({ path: "README.md", repository_name: "frontend", repository_id: "f" }), + file({ path: "README.md", repository_name: "backend", repository_id: "b" }), + ]); + const frontend = tree.find((n) => n.name === "frontend"); + const backend = tree.find((n) => n.name === "backend"); + expect(frontend?.children?.[0].name).toBe("README.md"); + expect(backend?.children?.[0].name).toBe("README.md"); + // File node back-refs point to the right repo. + expect(frontend?.children?.[0].file?.repository_id).toBe("f"); + expect(backend?.children?.[0].file?.repository_id).toBe("b"); + }); +}); diff --git a/apps/web/components/review/types.ts b/apps/web/components/review/types.ts index c3d4e3b4d9..b7e5a70288 100644 --- a/apps/web/components/review/types.ts +++ b/apps/web/components/review/types.ts @@ -9,6 +9,14 @@ export type ReviewFile = { staged: boolean; source: "uncommitted" | "committed" | "pr"; diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded"; + /** + * Repository this file belongs to. Set on multi-repo task changes so the + * file tree groups files under per-repo top-level nodes. Optional for + * single-repo tasks; the tree falls back to flat path-only grouping. + */ + repository_id?: string; + /** Human-readable repo name for tree node labels (e.g. "frontend"). */ + repository_name?: string; }; export function diffSkipReasonLabel(reason?: string): string { @@ -32,6 +40,14 @@ export type FileTreeNode = { isDir: boolean; children?: FileTreeNode[]; file?: ReviewFile; + /** + * True when this node represents a repository root in a multi-repo file + * tree. Repo roots are pinned (never collapsed into their first child) so + * the user always sees the repo grouping at the top level. + */ + isRepoRoot?: boolean; + /** Repository id this node represents (only set when isRepoRoot is true). */ + repositoryId?: string; }; /** @@ -61,8 +77,54 @@ export const hashDiff = djb2Hash; /** * Build a hierarchical file tree from flat file paths. * Collapses single-child directories (e.g., `src/components` as one node if `src` has no other children). + * + * Multi-repo: when files carry repository_name and 2+ distinct repos are + * present, the tree gets a top-level repo node per repository so the user + * sees per-repo grouping. Repo roots are pinned (never collapsed) and the + * file paths inside them stay relative to the repo root. */ export function buildFileTree(files: ReviewFile[]): FileTreeNode[] { + const repoNames = new Set(); + for (const f of files) { + if (f.repository_name) repoNames.add(f.repository_name); + } + const isMultiRepo = repoNames.size > 1; + + if (isMultiRepo) return buildMultiRepoTree(files); + return buildFlatTree(files); +} + +function buildMultiRepoTree(files: ReviewFile[]): FileTreeNode[] { + // Group by repo first, then build a sub-tree for each. + const byRepo = new Map(); + for (const f of files) { + const name = f.repository_name ?? "(unspecified)"; + const id = f.repository_id ?? name; + const key = id; + let entry = byRepo.get(key); + if (!entry) { + entry = { name, id, files: [] }; + byRepo.set(key, entry); + } + entry.files.push(f); + } + const repoRoots: FileTreeNode[] = []; + for (const [, entry] of byRepo) { + const subtree = buildFlatTree(entry.files); + repoRoots.push({ + name: entry.name, + path: `__repo__:${entry.id}`, + isDir: true, + isRepoRoot: true, + repositoryId: entry.id, + children: subtree, + }); + } + // Stable alphabetical order by repo name. + return repoRoots.sort((a, b) => a.name.localeCompare(b.name)); +} + +function buildFlatTree(files: ReviewFile[]): FileTreeNode[] { const root: FileTreeNode = { name: "", path: "", isDir: true, children: [] }; for (const file of files) { @@ -92,13 +154,20 @@ export function buildFileTree(files: ReviewFile[]): FileTreeNode[] { } } - // Collapse single-child directories + // Collapse single-child directories. Repo roots are never collapsed — + // they encode user-meaningful grouping that survives even when the repo + // has only one changed file. function collapse(node: FileTreeNode): FileTreeNode { if (!node.isDir || !node.children) return node; node.children = node.children.map(collapse); - if (node.children.length === 1 && node.children[0].isDir && node.name !== "") { + if ( + node.children.length === 1 && + node.children[0].isDir && + node.name !== "" && + !node.isRepoRoot + ) { const child = node.children[0]; return { ...child,