[kensai] defer line counting from index build to manifest

PathIndex walk now does stat-only -- no file opens, no byte
reads. Line counting and binary content detection move to
buildManifest(), which runs only for changed files (~10-50).

172K-file repo: 22s -> 1s for index build.

Other changes in this commit:
- readFile falls through to direct fs read when file is
  not in the index (supports reading outside indexed paths)
- buildManifest runs in parallel with fetchDiffs and
  resolveInstructions
- Pool class and #countLines removed; standalone
  countFileLines() exported for on-demand use

Author: xobotyi

claude/kensai/mcp/review/src/repofs/pathindex/pathindex.test.ts

@@ -4,7 +4,7 @@ import { dirname, join } from "node:path";
 
 import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test";
 
-import { PathIndex, validateGlobs } from "./pathindex.ts";
+import { PathIndex, countFileLines, validateGlobs } from "./pathindex.ts";
 import type { IndexEntryDir, IndexEntryFile, IndexEntrySymlink } from "./pathindex.ts";
 
 describe("PathIndex", () => {
@@ -321,32 +321,21 @@ describe("PathIndex.new", () => {
 		expect(ix.get("nonexistent")).toBeUndefined();
 	});
 
-	it("counts lines in text files", async () => {
+	it("index is stat-only — lineCount and maxLineLen are zero", async () => {
 		await writeFile(join(tempDir, "three.txt"), "one\ntwo\nthree\n");
-		await writeFile(join(tempDir, "no-trailing.txt"), "one\ntwo");
 		const ix = await PathIndex.new(tempDir);
-		const three = ix.get("three.txt") as IndexEntryFile;
-		const noTrailing = ix.get("no-trailing.txt") as IndexEntryFile;
-		expect(three.lineCount).toBe(3);
-		expect(three.isBinary).toBe(false);
-		expect(noTrailing.lineCount).toBe(2);
-		expect(noTrailing.isBinary).toBe(false);
-	});
-
-	it("tracks max line length", async () => {
-		await writeFile(join(tempDir, "varied.txt"), "short\na longer line here\nhi\n");
-		const ix = await PathIndex.new(tempDir);
-		const entry = ix.get("varied.txt") as IndexEntryFile;
-		expect(entry.maxLineLen).toBe(Buffer.byteLength("a longer line here"));
+		const entry = ix.get("three.txt") as IndexEntryFile;
+		expect(entry.lineCount).toBe(0);
+		expect(entry.maxLineLen).toBe(0);
+		expect(entry.isBinary).toBe(false);
 	});
 
-	it("detects binary files", async () => {
-		await writeFile(join(tempDir, "binary.bin"), Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x0a, 0x1a]));
+	it("detects binary by extension", async () => {
+		await writeFile(join(tempDir, "image.png"), Buffer.from([0x89, 0x50, 0x4e, 0x47]));
+		await writeFile(join(tempDir, "code.ts"), "const x = 1;\n");
 		const ix = await PathIndex.new(tempDir);
-		const entry = ix.get("binary.bin") as IndexEntryFile;
-		expect(entry.isBinary).toBe(true);
-		expect(entry.lineCount).toBe(0);
-		expect(entry.maxLineLen).toBe(0);
+		expect((ix.get("image.png") as IndexEntryFile).isBinary).toBe(true);
+		expect((ix.get("code.ts") as IndexEntryFile).isBinary).toBe(false);
 	});
 
 	it("handles empty files", async () => {
@@ -401,3 +390,41 @@ describe("PathIndex.new", () => {
 		expect(root.children.map((c) => c.name).sort()).toEqual(["README.md", "lib", "main.go", "src"]);
 	});
 });
+
+describe("countFileLines", () => {
+	let tempDir: string;
+
+	beforeEach(async () => {
+		tempDir = await mkdtemp(join(tmpdir(), "countlines-"));
+	});
+
+	afterEach(async () => {
+		await rm(tempDir, { recursive: true, maxRetries: 3 });
+	});
+
+	it("counts lines in text file", async () => {
+		await writeFile(join(tempDir, "three.txt"), "one\ntwo\nthree\n");
+		const result = await countFileLines(join(tempDir, "three.txt"));
+		expect(result.lineCount).toBe(3);
+		expect(result.isBinary).toBe(false);
+	});
+
+	it("tracks max line length", async () => {
+		await writeFile(join(tempDir, "varied.txt"), "short\na longer line here\nhi\n");
+		const result = await countFileLines(join(tempDir, "varied.txt"));
+		expect(result.maxLineLen).toBe(Buffer.byteLength("a longer line here"));
+	});
+
+	it("detects binary content", async () => {
+		await writeFile(join(tempDir, "bin"), Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x0a]));
+		const result = await countFileLines(join(tempDir, "bin"));
+		expect(result.isBinary).toBe(true);
+		expect(result.lineCount).toBe(0);
+	});
+
+	it("handles file without trailing newline", async () => {
+		await writeFile(join(tempDir, "no-nl.txt"), "one\ntwo");
+		const result = await countFileLines(join(tempDir, "no-nl.txt"));
+		expect(result.lineCount).toBe(2);
+	});
+});

claude/kensai/mcp/review/src/repofs/pathindex/pathindex.ts

@@ -192,7 +192,6 @@ export class PathIndex {
 	readonly root: IndexEntryDir = { type: "dir", name: "", children: [] };
 	readonly paths: string[] = [];
 	readonly #entries = new Map<string, IndexEntry>();
-	readonly #fdPool = new Pool(256);
 
 	private constructor(rootPath: string) {
 		this.absRoot = nativePath.resolve(rootPath);
@@ -332,41 +331,21 @@ export class PathIndex {
 				continue;
 			}
 
-			const ext = path.extname(dirent.name).toLowerCase();
-
-			if (BINARY_EXTENSIONS.has(ext)) {
-				work.push(
-					fs.stat(absPath).then((s) => {
-						const entry: IndexEntryFile = {
-							type: "file",
-							name: dirent.name,
-							size: s.size,
-							lineCount: 0,
-							maxLineLen: 0,
-							isBinary: true,
-						};
-						dirEntry.children.push(entry);
-						this.paths.push(relPath);
-						this.#entries.set(relPath, entry);
-					}),
-				);
-			} else {
-				work.push(
-					Promise.all([fs.stat(absPath), this.#countLines(absPath)]).then(([s, lc]) => {
-						const entry: IndexEntryFile = {
-							type: "file",
-							name: dirent.name,
-							size: s.size,
-							lineCount: lc.lineCount,
-							maxLineLen: lc.maxLineLen,
-							isBinary: lc.isBinary,
-						};
-						dirEntry.children.push(entry);
-						this.paths.push(relPath);
-						this.#entries.set(relPath, entry);
-					}),
-				);
-			}
+			work.push(
+				fs.stat(absPath).then((s) => {
+					const entry: IndexEntryFile = {
+						type: "file",
+						name: dirent.name,
+						size: s.size,
+						lineCount: 0,
+						maxLineLen: 0,
+						isBinary: BINARY_EXTENSIONS.has(path.extname(dirent.name).toLowerCase()),
+					};
+					dirEntry.children.push(entry);
+					this.paths.push(relPath);
+					this.#entries.set(relPath, entry);
+				}),
+			);
 		}
 
 		await Promise.all(work);
@@ -395,57 +374,27 @@ export class PathIndex {
 			return undefined;
 		}
 	}
-
-	/** Body-less line scan via {@link LineIter}. Binary files are detected by the null byte probe on the first chunk. */
-	async #countLines(absPath: string): Promise<{ lineCount: number; maxLineLen: number; isBinary: boolean }> {
-		await this.#fdPool.acquire();
-		const stream = createReadStream(absPath);
-		try {
-			const iter = await LineIter.new(stream, { lineCap: 0 });
-			let maxLineLen = 0;
-			while (await iter.next()) {
-				if (iter.len() > maxLineLen) maxLineLen = iter.len();
-			}
-			return { lineCount: iter.num(), maxLineLen, isBinary: false };
-		} catch (err) {
-			if (err instanceof ErrBinaryContent) {
-				return { lineCount: 0, maxLineLen: 0, isBinary: true };
-			}
-			throw err;
-		} finally {
-			stream.destroy();
-			this.#fdPool.release();
-		}
-	}
 }
 
-/** Bounds concurrent async operations. {@link acquire} blocks when the limit is reached. */
-class Pool {
-	readonly #limit: number;
-	#active = 0;
-	#queue: (() => void)[] = [];
-
-	constructor(limit: number) {
-		this.#limit = limit;
-	}
-
-	/** Take a slot. Returns a promise that resolves when a slot is available. */
-	acquire(): Promise<void> | void {
-		if (this.#active < this.#limit) {
-			this.#active++;
-			return;
+/** Count lines, detect binary, and measure max line length for a single file. */
+export async function countFileLines(
+	absPath: string,
+): Promise<{ lineCount: number; maxLineLen: number; isBinary: boolean }> {
+	const stream = createReadStream(absPath);
+	try {
+		const iter = await LineIter.new(stream, { lineCap: 0 });
+		let maxLineLen = 0;
+		while (await iter.next()) {
+			if (iter.len() > maxLineLen) maxLineLen = iter.len();
 		}
-		return new Promise<void>((resolve) => this.#queue.push(resolve));
-	}
-
-	/** Return a slot. Wakes the next waiter if any. */
-	release(): void {
-		const next = this.#queue.shift();
-		if (next) {
-			next();
-		} else {
-			this.#active--;
+		return { lineCount: iter.num(), maxLineLen, isBinary: false };
+	} catch (err) {
+		if (err instanceof ErrBinaryContent) {
+			return { lineCount: 0, maxLineLen: 0, isBinary: true };
 		}
+		throw err;
+	} finally {
+		stream.destroy();
 	}
 }
 

claude/kensai/mcp/review/src/repofs/repofs.ts

@@ -44,7 +44,6 @@ export class RepoFs {
 		}
 
 		const [index, git] = await Promise.all([PathIndex.new(absRoot, signal), Repo.open(absRoot, signal)]);
-
 		return new RepoFs(absRoot, index, git);
 	}
 
@@ -81,22 +80,26 @@ export class RepoFs {
 		return this.#index.dir(this.resolve(p)) !== undefined;
 	}
 
-	/** Resolves path, validates file entry in index, returns raw bytes. */
+	/** Resolves path, validates via index or filesystem, returns raw bytes. Falls through to direct read for files not in the index. */
 	async readFile(p: string): Promise<Buffer> {
 		const rel = this.resolve(p);
 		const entry = this.#index.get(rel);
 
-		if (!entry) {
-			throw new RepoFsError(`file not found: ${rel}`);
+		if (entry && entry.type !== "file") {
+			throw new RepoFsError(`${rel}: not a regular file (${entry.type})`);
 		}
 
-		if (entry.type !== "file") {
-			throw new RepoFsError(`${rel}: not a regular file (${entry.type})`);
+		const absPath = path.resolve(this.#root, rel);
+
+		if (!entry) {
+			const stat = await fsp.stat(absPath).catch(() => null);
+			if (!stat?.isFile()) {
+				throw new RepoFsError(`file not found: ${rel}`);
+			}
 		}
 
 		this.#pendingReadPaths.push(rel);
-
-		return fsp.readFile(path.resolve(this.#root, rel));
+		return fsp.readFile(absPath);
 	}
 
 	/** Fuzzy search via {@link PathIndex.fuzzySearch}. */

claude/kensai/mcp/review/src/session/session.ts

@@ -1,6 +1,8 @@
 import { createHash } from "node:crypto";
+import path from "node:path";
 
 import type { GitFileStat, GitLogEntry } from "../repofs/git/git.ts";
+import { countFileLines } from "../repofs/pathindex/pathindex.ts";
 import { RepoFs } from "../repofs/repofs.ts";
 import { FindingsStorage } from "./findings-storage.ts";
 import { GroundingStorage } from "./grounding-storage.ts";
@@ -82,11 +84,11 @@ export class Session {
 			mode !== "uncommitted" ? rfs.git.log("HEAD", 1).then((entries) => entries[0] ?? null) : Promise.resolve(null),
 		]);
 
-		const manifest = buildManifest(rfs, changedFiles);
 		const reviewable = filterReviewableFiles(changedFiles);
 		const changedPaths = new Set(changedFiles.map((f) => f.path));
 
-		const [diffs, instructions] = await Promise.all([
+		const [manifest, diffs, instructions] = await Promise.all([
+			buildManifest(rfs, changedFiles),
 			fetchDiffs(rfs, base, head, reviewable),
 			resolveInstructions(
 				rfs,
@@ -179,26 +181,38 @@ export function filterReviewableFiles(files: readonly GitFileStat[]): GitFileSta
 	return files.filter((f) => f.status !== "deleted" && !shouldSkipDiff(f.path));
 }
 
-/** Builds per-file shape metrics from PathIndex for all non-deleted changed files. No I/O — index is pre-built. */
-function buildManifest(rfs: RepoFs, files: readonly GitFileStat[]): ManifestEntry[] {
-	const entries: ManifestEntry[] = [];
+/** Builds per-file shape metrics for all non-deleted changed files. Counts lines on demand (only for changed files). */
+async function buildManifest(rfs: RepoFs, files: readonly GitFileStat[]): Promise<ManifestEntry[]> {
+	const candidates: Array<{ path: string; entry: { size: number; isBinary: boolean } }> = [];
 
 	for (const f of files) {
 		if (f.status === "deleted") continue;
-
 		const entry = rfs.index.get(f.path);
 		if (!entry || entry.type !== "file") continue;
-
-		entries.push({
-			path: f.path,
-			bytes: entry.size,
-			lines: entry.lineCount,
-			maxLineLen: entry.maxLineLen,
-			binary: entry.isBinary,
-		});
+		candidates.push({ path: f.path, entry });
 	}
 
-	return entries;
+	const results = await Promise.all(
+		candidates.map(async ({ path: filePath, entry }) => {
+			if (entry.isBinary) {
+				return { path: filePath, bytes: entry.size, lines: 0, maxLineLen: 0, binary: true };
+			}
+			try {
+				const lc = await countFileLines(path.resolve(rfs.root, filePath));
+				return {
+					path: filePath,
+					bytes: entry.size,
+					lines: lc.lineCount,
+					maxLineLen: lc.maxLineLen,
+					binary: lc.isBinary,
+				};
+			} catch {
+				return { path: filePath, bytes: entry.size, lines: 0, maxLineLen: 0, binary: false };
+			}
+		}),
+	);
+
+	return results;
 }
 
 /**