From c815262fe547213373a28fa5460fa9c11ae49113 Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:22:18 +0000 Subject: [PATCH] perf(computer): Scope the git diff worktree walk to requested paths diff({ paths }) filtered after statusMatrix had already walked the whole worktree and handed every path to its map callback. Once a synced node_modules is in the tree that walk dominates a diff that the caller scoped to a handful of files. Pass the requested paths to isomorphic-git's filepaths so the traversal is pruned up front. filepaths uses the same "exact path or directory prefix" rule as makePathFilter, so this is a traversal hint only: the filter stays the authority on what is emitted and results are unchanged. normalizeFilepaths returns undefined when the caller asked for no scoping, or when a path normalizes to the repo root, so isomorphic-git's own default of ['.'] stands rather than scoping to everything. --- .changeset/scoped-git-diff-status.md | 5 ++ packages/computer/src/git/diff.test.ts | 71 +++++++++++++++++++++++++- packages/computer/src/git/diff.ts | 29 ++++++++++- 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 .changeset/scoped-git-diff-status.md diff --git a/.changeset/scoped-git-diff-status.md b/.changeset/scoped-git-diff-status.md new file mode 100644 index 00000000..e3bb1238 --- /dev/null +++ b/.changeset/scoped-git-diff-status.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": patch +--- + +Improve performance of git diff when given named paths. diff --git a/packages/computer/src/git/diff.test.ts b/packages/computer/src/git/diff.test.ts index 923744fd..50f3dedb 100644 --- a/packages/computer/src/git/diff.test.ts +++ b/packages/computer/src/git/diff.test.ts @@ -31,6 +31,10 @@ async function init(): Promise { } async function commitFile(path: string, content: string, message: string): Promise { + const slash = path.lastIndexOf("/"); + if (slash > 0) { + await memfs.promises.mkdir(`${DIR}/${path.slice(0, slash)}`, { recursive: true }); + } await memfs.promises.writeFile(`${DIR}/${path}`, content); await git.add({ fs: memfs, dir: DIR, filepath: path }); return git.commit({ fs: memfs, dir: DIR, message, author: AUTHOR }); @@ -42,7 +46,7 @@ async function stageThenRemove(path: string): Promise { await memfs.promises.unlink(`${DIR}/${path}`); } -async function runDiff(opts: { ref?: string } = {}): Promise { +async function runDiff(opts: { ref?: string; paths?: string[] } = {}): Promise { return diffWith({ git: isomorphicGit, fs: memfs, @@ -50,6 +54,7 @@ async function runDiff(opts: { ref?: string } = {}): Promise { readFile: (path) => memfs.promises.readFile(path) as Promise, dir: DIR, ref: opts.ref, + paths: opts.paths, }); } @@ -156,6 +161,70 @@ describe("diffWith (real isomorphic-git + memfs)", () => { } }); + it("scopes the status walk to the requested paths", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("vendor/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/vendor/b.txt`, "two changed\n"); + + const statusSpy = vi.spyOn(git, "statusMatrix"); + try { + const out = await runDiff({ paths: ["src"] }); + // The walk must be told to stay inside `src` rather than + // scanning the whole tree and filtering afterwards. + expect(statusSpy.mock.calls[0][0]).toMatchObject({ filepaths: ["src"] }); + // Result is unchanged by the scoping. + expect(out).toContain("+one changed"); + expect(out).not.toContain("two changed"); + } finally { + statusSpy.mockRestore(); + } + }); + + it("walks the whole tree when no paths are given", async () => { + await init(); + await commitFile("a.txt", "one\n", "init"); + await memfs.promises.writeFile(`${DIR}/a.txt`, "one changed\n"); + + const statusSpy = vi.spyOn(git, "statusMatrix"); + try { + await runDiff(); + // No scope requested: isomorphic-git's default ('.') must stand. + expect(statusSpy.mock.calls[0][0]).not.toHaveProperty("filepaths"); + } finally { + statusSpy.mockRestore(); + } + }); + + it("produces the same diff scoped and unscoped", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("vendor/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/vendor/b.txt`, "two changed\n"); + + const scoped = await runDiff({ paths: ["src"] }); + const full = await runDiff(); + // The scoped run must equal the src-only slice of the full run. + expect(scoped).toContain("+one changed"); + expect(full).toContain("+one changed"); + expect(full).toContain("+two changed"); + expect(scoped).not.toContain("+two changed"); + }); + + it("scopes correctly for an exact file path", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("src/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/src/b.txt`, "two changed\n"); + + const out = await runDiff({ paths: ["src/a.txt"] }); + expect(out).toContain("+one changed"); + expect(out).not.toContain("two changed"); + }); + it("respects the `ref` argument when diffing against an older commit", async () => { await init(); const first = await commitFile("a.txt", "v1\n", "v1"); diff --git a/packages/computer/src/git/diff.ts b/packages/computer/src/git/diff.ts index e71c7b5e..25fd304e 100644 --- a/packages/computer/src/git/diff.ts +++ b/packages/computer/src/git/diff.ts @@ -29,6 +29,8 @@ export interface IsomorphicGitDiffClient { dir: string; ref?: string; cache?: object; + /** Prunes the worktree walk to these paths. Omit to walk it all. */ + filepaths?: string[]; }): Promise; readBlob(args: { fs: object; @@ -166,7 +168,21 @@ async function collectDiffEntries(opts: DiffWithDeps): Promise { // requested commit rather than always HEAD. Without this the // `ref` argument would only affect blob reads, leaving the // status walk silently skewed. - const status = await opts.git.statusMatrix({ fs: opts.fs, dir, ref, cache: opts.cache }); + // Scope the walk when the caller named paths. isomorphic-git prunes + // the traversal to these prefixes instead of visiting the whole + // worktree and handing every path to `map`, which matters once the + // tree carries a synced node_modules. `filepaths` uses the same + // "exact path or directory prefix" rule as makePathFilter below, so + // the filter stays as the authority on what is emitted and this is + // purely a traversal hint. + const scopedPaths = normalizeFilepaths(opts.paths); + const status = await opts.git.statusMatrix({ + fs: opts.fs, + dir, + ref, + cache: opts.cache, + ...(scopedPaths === undefined ? {} : { filepaths: scopedPaths }), + }); const pathFilter = makePathFilter(opts.paths); const entries: DiffEntry[] = []; for (const [filepath, headStatus, workdirStatus] of status) { @@ -262,6 +278,17 @@ async function listFilesAt( return git.listFiles({ fs, dir, ref }); } +// The `filepaths` form isomorphic-git wants: normalized, deduplicated, +// and undefined when the caller asked for no scoping (so the library's +// own default of ['.'] stands). An empty string normalizes to the repo +// root, which would scope to everything — treat it as no scope. +function normalizeFilepaths(paths: string[] | undefined): string[] | undefined { + if (paths === undefined || paths.length === 0) return undefined; + const out = [...new Set(paths.map((p) => normalizePath(p)))]; + if (out.some((p) => p === "" || p === ".")) return undefined; + return out; +} + function makePathFilter(paths: string[] | undefined): (p: string) => boolean { if (paths === undefined || paths.length === 0) return () => true; // Match either an exact path or a directory prefix. Globs are