fix(diff): batch cold reads and refresh logical comparison inputs

Acquire bounded pages of tracked patches lazily after Begin while retaining per-file highlighting and awaited frame delivery. Keep legacy preferred-base selection live across revisions and include submodule Git state in worktree cache identity.
This commit is contained in:
Mohamed Boudra
2026-11-06 07:48:26 +01:00
parent e516961a09
commit 986ce285b0
7 changed files with 286 additions and 20 deletions
@@ -193,3 +193,61 @@ test("legacy diffs retain successful bodies when an oversized sibling is removed
subscription.unsubscribe();
});
});
test("legacy subscriptions reselect the preferred base when a remote ref appears", async () => {
await withCheckout(async ({ cwd, manager, git }) => {
git("checkout", "-b", "feature");
await writeFile(join(cwd, "b.ts"), "export const b = 1;\n");
git("add", ".");
git("-c", "commit.gpgsign=false", "commit", "-m", "feature");
const updates: CheckoutDiffSnapshotPayload[] = [];
const params = { cwd, compare: { mode: "base" as const, baseRef: "main" } };
const subscription = await manager.subscribe(params, (snapshot) => updates.push(snapshot));
try {
expect(subscription.initial.files.map((file) => file.path)).toEqual(["b.ts"]);
git("update-ref", "refs/remotes/origin/main", "HEAD");
manager.scheduleRefreshForCwd(cwd);
await expect.poll(() => updates.at(-1)?.files.length, { timeout: 5000 }).toBe(0);
expect(updates.at(-1)).toEqual(await manager.read(params));
} finally {
await subscription.unsubscribe();
}
});
});
test("cold many-file reads batch Git patches while matching the legacy reader", async () => {
const { getCheckoutDiff } = await import("../utils/checkout-git.js");
const { startGitCommandMetrics, stopGitCommandMetrics } =
await import("../utils/run-git-command.js");
await withCheckout(async ({ cwd, manager, git }) => {
const count = 1000;
for (let index = 0; index < count; index++)
await writeFile(join(cwd, `file-${index}.ts`), "export const value = 0;\n");
git("add", ".");
git("-c", "commit.gpgsign=false", "commit", "-m", "many files");
for (let index = 0; index < count; index++)
await writeFile(join(cwd, `file-${index}.ts`), "export const value = 1;\n");
const legacyStart = performance.now();
const legacy = await getCheckoutDiff(cwd, { mode: "uncommitted", includeStructured: true });
const legacyMs = performance.now() - legacyStart;
startGitCommandMetrics();
const streamedStart = performance.now();
const streamed = await manager.read({ cwd, compare: { mode: "uncommitted" } });
const streamedMs = performance.now() - streamedStart;
const metrics = stopGitCommandMetrics();
const patches = metrics.commands.filter((command) => command.args.includes("--patch"));
const artifactDirectory = join(import.meta.dirname, "../../../../.artifacts/diff-subscription");
await mkdir(artifactDirectory, { recursive: true });
await writeFile(
join(artifactDirectory, "cold-many-file-metrics.json"),
JSON.stringify({ count, legacyMs, streamedMs, patchCommands: patches.length }, null, 2),
);
expect(streamed.error).toBeNull();
expect(streamed.files).toHaveLength(count);
expect(legacy.structured).toHaveLength(count);
expect(streamed.files.map((file) => [file.path, file.hunks])).toEqual(
legacy.structured?.map((file) => [file.path, file.hunks]),
);
expect(patches.length).toBeLessThanOrEqual(Math.ceil(count / 64));
});
}, 60000);
@@ -129,6 +129,10 @@ export class CheckoutDiffManager {
{
cwd,
base: { kind: "rev", rev: baseRef },
checkoutBase:
params.compare.mode === "uncommitted"
? undefined
: { compare: params.compare, paseoHome: this.paseoHome },
head:
params.compare.mode === "base" ? { kind: "rev", rev: "HEAD" } : { kind: "worktree" },
mergeBase: params.compare.mode !== "uncommitted",
@@ -593,3 +593,38 @@ test("files removed between listing and highlighting retain their deletion patch
await rm(cwd, { recursive: true, force: true });
}
});
test("a batched oversized tracked patch does not hide its small sibling", async () => {
const cwd = await mkdtemp(join(tmpdir(), "diff-batch-budget-"));
const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" });
try {
git("init", "-b", "main");
git("config", "user.name", "Test");
git("config", "user.email", "test@example.com");
await writeFile(join(cwd, "a.txt"), "before\n");
await writeFile(join(cwd, "b.txt"), "before\n");
git("add", ".");
git("-c", "commit.gpgsign=false", "commit", "-m", "base");
await writeFile(join(cwd, "a.txt"), "oversized\n".repeat(1024 * 1024));
await writeFile(join(cwd, "b.txt"), "after\n");
const comparison = await listComparison({
cwd,
base: { kind: "rev", rev: "HEAD" },
head: { kind: "worktree" },
highlight: false,
});
try {
expect(await comparison.read(comparison.files[0])).toEqual({ kind: "too_large" });
const sibling = await comparison.read(comparison.files[1]);
if (sibling.kind !== "ready") throw new Error("Expected readable sibling");
expect(sibling.body.hunks.flatMap((hunk) => hunk.lines)).toContainEqual({
type: "add",
content: "after",
});
} finally {
await comparison.close();
}
} finally {
await rm(cwd, { recursive: true, force: true });
}
});
+45 -19
View File
@@ -1,4 +1,5 @@
import { parseGitRevParsePath } from "../../utils/git-rev-parse-path.js";
import { Patches } from "./patches.js";
import { HighlightSources } from "./sources.js";
import {
prepareAllChangesIndex,
@@ -6,6 +7,8 @@ import {
GIT_PATCH_FORMAT_ARGS,
inspectUntrackedFile,
snapshotComparisonIndex,
resolveCheckoutDiffBase,
type CheckoutDiffCompare,
} from "../../utils/checkout-git.js";
import { lstat, readFile, readlink } from "node:fs/promises";
import { resolve } from "node:path";
@@ -14,7 +17,9 @@ import type { DiffEntry, DiffBody } from "@getpaseo/protocol/binary-frames/index
import { runGitCommand, type GitCommandOptions } from "../../utils/run-git-command.js";
import { parseDiff, highlightDiffWithFileContent } from "../utils/diff-highlighter.js";
export type Compare = Omit<DiffSubscribeRequest, "type" | "requestId">;
export type Compare = Omit<DiffSubscribeRequest, "type" | "requestId"> & {
checkoutBase?: { compare: CheckoutDiffCompare; paseoHome: string };
};
export interface ListedFile {
entry: DiffEntry;
identity: string;
@@ -34,6 +39,7 @@ export interface Comparison extends Pick<
files: ListedFile[];
metrics: { highlightCalls: number };
close(): Promise<void>;
selectBodies(paths: ReadonlySet<string>): void;
read(
file: ListedFile,
budget?: { remainingBytes?: number; maxPatchBytes?: number },
@@ -125,6 +131,13 @@ function parseStats(
return stats;
}
export async function listComparison(compare: Compare): Promise<Comparison> {
if (compare.checkoutBase) {
const selected = await resolveCheckoutDiffBase(compare.cwd, compare.checkoutBase.compare, {
paseoHome: compare.checkoutBase.paseoHome,
});
if (!selected) throw new DiffReadError("REF_NOT_FOUND", "Comparison base is unavailable");
compare = { ...compare, base: { kind: "rev", rev: selected } };
}
validateCompare(compare);
const cwd = compare.cwd;
let privateIndex: Awaited<ReturnType<typeof prepareAllChangesIndex>> = null;
@@ -207,9 +220,9 @@ async function readComparisonList(
else if (oldPath !== null) change = "renamed";
let identity = header.join(" ");
if (compare.head.kind === "worktree" && code !== "D") {
const value = await lstat(resolve(cwd, path), { bigint: true });
identity += `:${value.size}:${value.mtimeNs}`;
identity += `:${await readWorktreeIdentity(path, header, options)}`;
}
files.push({
entry: {
path,
@@ -227,6 +240,7 @@ async function readComparisonList(
if (compare.head.kind === "worktree") files.push(...(await listUntrackedFiles(options)));
files.sort((a, b) => a.entry.path.localeCompare(b.entry.path));
const metrics = { highlightCalls: 0 };
const patches = new Patches(args, options, files);
const sources = new HighlightSources({
cwd,
baseSha: comparisonBaseSha,
@@ -239,32 +253,27 @@ async function readComparisonList(
mergeBaseSha,
files,
metrics,
selectBodies: (paths) => patches.select(paths),
close: async () => {
await privateIndex?.cleanup();
},
async read(file, budget = {}) {
const maxPatchBytes = budget.maxPatchBytes ?? PER_FILE_DIFF_MAX_BYTES;
const remainingBytes = budget.remainingBytes ?? maxPatchBytes;
const paths = [file.entry.path];
if (file.entry.oldPath !== null) paths.unshift(file.entry.oldPath);
if (file.source.kind === "untracked" && file.source.size > maxPatchBytes) {
return { kind: "too_large" };
}
const command =
const result =
file.source.kind === "untracked"
? ["diff", "--no-index", ...GIT_PATCH_FORMAT_ARGS, "--", "/dev/null", file.entry.path]
: [
...args,
...(file.entry.oldPath !== null ? ["--diff-filter=RC"] : []),
"--patch",
"--",
...paths.map((path) => `:(literal)${path}`),
];
const result = await runGitCommand(command, {
...options,
acceptExitCodes: [0, 1],
maxOutputBytes: maxPatchBytes + 1,
});
? await runGitCommand(
["diff", "--no-index", ...GIT_PATCH_FORMAT_ARGS, "--", "/dev/null", file.entry.path],
{
...options,
acceptExitCodes: [0, 1],
maxOutputBytes: maxPatchBytes + 1,
},
)
: await patches.read(file, maxPatchBytes);
const bytes = Buffer.byteLength(result.stdout);
if (result.truncated || bytes > remainingBytes || bytes > maxPatchBytes)
return { kind: "too_large" };
@@ -309,6 +318,23 @@ async function readComparisonList(
};
}
async function readWorktreeIdentity(
path: string,
header: string[],
options: GitCommandOptions,
): Promise<string> {
const value = await lstat(resolve(options.cwd, path), { bigint: true });
let identity = `${value.size}:${value.mtimeNs}`;
if (header[0] === ":160000" || header[1] === "160000") {
const submodule = await runGitCommand(
["status", "--porcelain=v2", "--branch", "--untracked-files=normal"],
{ cwd: resolve(options.cwd, path), envOverlay: { GIT_OPTIONAL_LOCKS: "0" } },
);
identity += `:${submodule.stdout}`;
}
return identity;
}
function diffArguments(compare: Compare, baseSha: string, headSha: string): string[] {
const revisions: string[] = [];
if (compare.base.kind !== "index") revisions.push(baseSha);
+7 -1
View File
@@ -100,6 +100,7 @@ export class DiffSubscriptions {
input.signal?.throwIfAborted();
const file = comparison.files.find((candidate) => candidate.entry.path === input.path);
if (!file || file.entry.content === "binary") return null;
comparison.selectBodies(new Set([file.entry.path]));
const record = await comparison.read(file, { maxPatchBytes: DEFAULT_MAX_OUTPUT_BYTES });
input.signal?.throwIfAborted();
return materializeDiffFile(
@@ -122,7 +123,9 @@ export class DiffSubscriptions {
};
const key = JSON.stringify([
compare.cwd,
endpointKey(compare.base),
compare.checkoutBase
? ["checkout", JSON.stringify(compare.checkoutBase)]
: endpointKey(compare.base),
endpointKey(compare.head),
compare.mergeBase,
compare.ignoreWhitespace,
@@ -373,6 +376,9 @@ export class DiffSubscriptions {
for (const entry of entries) if (entry.content === "text") state.omitted.push(entry.path);
return;
}
comparison.selectBodies(
new Set(entries.filter((entry) => entry.body).map((entry) => entry.path)),
);
for (let index = 0; index < comparison.files.length; index++) {
if (isInterrupted(target)) return;
const file = comparison.files[index];
@@ -0,0 +1,80 @@
import { runGitCommand, type GitCommandOptions } from "../../utils/run-git-command.js";
import { parseDiff } from "../utils/diff-highlighter.js";
import type { ListedFile } from "./git.js";
interface Patch {
stdout: string;
truncated: boolean;
}
// Acquire one bounded page lazily. Highlighting and delivery remain per file.
export class Patches {
private files: ListedFile[];
private pending = new Map<string, Promise<Map<string, Patch>>>();
constructor(
private args: string[],
private options: GitCommandOptions,
files: ListedFile[],
) {
this.files = files.filter((file) => file.source.kind === "tracked" && file.entry.body);
}
select(paths: ReadonlySet<string>): void {
this.files = this.files.filter((file) => paths.has(file.entry.path));
}
async read(file: ListedFile, maxBytes: number): Promise<Patch> {
const existing = this.pending.get(file.entry.path);
if (existing) return (await existing).get(file.entry.path) ?? { stdout: "", truncated: false };
const renamed = file.entry.oldPath !== null;
const candidates = this.files.filter(
(candidate) => (candidate.entry.oldPath !== null) === renamed,
);
const index = candidates.indexOf(file);
const page = renamed || index < 0 ? [file] : candidates.slice(index, index + 64);
this.pending.clear();
const reading = this.load(page, maxBytes);
for (const candidate of page) this.pending.set(candidate.entry.path, reading);
return (await reading).get(file.entry.path) ?? { stdout: "", truncated: false };
}
private async load(files: ListedFile[], maxBytes: number): Promise<Map<string, Patch>> {
const paths = [
...new Set(
files.flatMap((file) =>
file.entry.oldPath === null ? [file.entry.path] : [file.entry.oldPath, file.entry.path],
),
),
];
const result = await runGitCommand(
[
...this.args,
...(files[0].entry.oldPath !== null ? ["--diff-filter=RC"] : []),
"--patch",
"--",
...paths.map((path) => `:(literal)${path}`),
],
{
...this.options,
acceptExitCodes: [0, 1],
maxOutputBytes: (files.length === 1 ? maxBytes : 8 * maxBytes) + 1,
},
);
if (files.length === 1) return new Map([[files[0].entry.path, result]]);
if (result.truncated) {
const middle = Math.ceil(files.length / 2);
return new Map([
...(await this.load(files.slice(0, middle), maxBytes)),
...(await this.load(files.slice(middle), maxBytes)),
]);
}
const patches = new Map<string, Patch>();
for (const section of result.stdout.split(/(?=^diff --git )/m).filter(Boolean)) {
const hunkStart = section.indexOf("\n@@ ");
const parsed = parseDiff(hunkStart < 0 ? section : section.slice(0, hunkStart))[0];
if (!parsed) throw new Error("Git returned an invalid patch header");
const previous = patches.get(parsed.path)?.stdout ?? "";
const stdout = previous + section;
patches.set(parsed.path, { stdout, truncated: Buffer.byteLength(stdout) > maxBytes });
}
return patches;
}
}
@@ -359,3 +359,60 @@ test("a cached replay completes before a live refresh can depend on it", async (
await rm(cwd, { recursive: true, force: true });
}
});
test("submodule commits invalidate cached worktree gitlink bodies", async () => {
const { lstat } = await import("node:fs/promises");
const cwd = await mkdtemp(join(tmpdir(), "diff-gitlink-cache-"));
const sub = join(cwd, "sub");
const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim();
const streams = new DiffSubscriptions({
async requestWorkingTreeWatch(root) {
return { repoRoot: root, unsubscribe: async () => {} };
},
});
const frames: DiffFrame[] = [];
try {
git("init", "-b", "main");
git("config", "user.name", "Test");
git("config", "user.email", "test@example.com");
await mkdir(sub);
git("-C", "sub", "init", "-b", "main");
git("-C", "sub", "config", "user.name", "Test");
git("-C", "sub", "config", "user.email", "test@example.com");
await writeFile(join(sub, "a.txt"), "A\n");
git("-C", "sub", "add", ".");
git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-m", "A");
git("add", ".");
git("-c", "commit.gpgsign=false", "commit", "-m", "record A");
await writeFile(join(sub, "a.txt"), "B\n");
git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-am", "B");
const before = await lstat(sub, { bigint: true });
const subscription = await streams.subscribe(
{ cwd, base: { kind: "rev", rev: "HEAD" }, head: { kind: "worktree" } },
async (frame) => {
frames.push(frame);
},
);
await subscription.start();
expect(frames.at(-1)).toMatchObject({ opcode: DiffOpcode.End, error: null });
frames.length = 0;
await writeFile(join(sub, "a.txt"), "C\n");
git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-am", "C");
const sha = git("-C", "sub", "rev-parse", "HEAD");
expect((await lstat(sub, { bigint: true })).mtimeNs).toBe(before.mtimeNs);
streams.invalidateForCwd(cwd);
await expect
.poll(() => frames.some((frame) => frame.opcode === DiffOpcode.End), { timeout: 5000 })
.toBe(true);
const bodies = frames.flatMap((frame) =>
frame.opcode === DiffOpcode.Bodies ? frame.files : [],
);
expect(bodies.flatMap((file) => file.hunks.flatMap((hunk) => hunk.lines))).toContainEqual(
expect.objectContaining({ type: "add", content: `Subproject commit ${sha}` }),
);
await subscription.release();
} finally {
await streams.close();
await rm(cwd, { recursive: true, force: true });
}
});