From d615a2609f1eb4f690e4185e295684d0469c233f Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:49:55 -0600 Subject: [PATCH 1/8] fix(generic): free web-tree-sitter parsers and trees, detect an aborted runtime extractGeneric and parseWasm created a Parser per file and never deleted it or the Tree. Both live in the WASM heap (2 GB cap, no finalizer), so a 65k-file C repo aborted after ~12k files and every later parse failed with Aborted(). Reuse one parser per grammar and delete every tree. After a real abort, fail every later parse with one message instead of one per file. parseWasm now returns the tree for the container tier to free, and rethrows instead of caching a symbol-less file as a success. --- src/graph/container.ts | 117 +++++++++++-------- src/graph/generic.ts | 193 ++++++++++++++++++++++---------- test/container-extract.test.ts | 16 +++ test/generic-extract.test.ts | 199 +++++++++++++++++++++++++++++++-- 4 files changed, 407 insertions(+), 118 deletions(-) diff --git a/src/graph/container.ts b/src/graph/container.ts index 3015069e7..2e1c63ac0 100644 --- a/src/graph/container.ts +++ b/src/graph/container.ts @@ -22,7 +22,7 @@ * fixtures whose true line numbers are known. */ import { extractFile, mintId, type ExtractResult, type Language, type RawEdge } from "./extract.js"; -import { loadWasmLanguage, parseWasm, type TsNode } from "./generic.js"; +import { loadWasmLanguage, parseWasm, type TsNode, type WasmTree } from "./generic.js"; import { contentHash } from "../util/id.js"; import type { NodeV1 } from "./types.js"; @@ -66,6 +66,14 @@ export function containerExtensions(): string[] { const loaded = new Map(); +/** Test seam, same shape as generic.ts's `swapGrammarForTest`. */ +export function swapContainerGrammarForTest(name: string, language: unknown | null): unknown | null { + const prev = loaded.get(name) ?? null; + if (language) loaded.set(name, language); + else loaded.delete(name); + return prev; +} + /** Warm the container grammars this repo needs. Same contract as * `warmGenericGrammars`: await once before the synchronous parse loop, and an * unavailable grammar is skipped rather than fatal (its files then extract to a @@ -144,9 +152,11 @@ function blocks(root: TsNode, lang: ContainerLang): TsNode[] { /** * Extract one container file. Synchronous; needs the grammar pre-warmed. * - * Never throws: a missing grammar, an unparseable SFC or a script block the - * inner extractor chokes on all degrade to "fewer nodes", because a build must - * not fail over one component. + * A missing grammar, or a script block the inner extractor rejects, still + * degrades to "fewer nodes", because a build must not fail over one component. + * But a wrapper grammar that THROWS (a WASM abort, not a per-file syntax error) + * propagates as a build error: swallowing it would cache a clean symbol-less + * file as success, the exact mistake the generic tier used to make. */ export function extractContainer(rel: string, source: string, lang: ContainerLang): ExtractResult { const nodes: NodeV1[] = []; @@ -154,51 +164,66 @@ export function extractContainer(rel: string, source: string, lang: ContainerLan const residuals: string[] = []; const language = loaded.get(lang.name); - const root = language ? parseWasm(language, source) : null; - - if (root) { - // Ids are minted per file by the inner extractor, so two script blocks that - // both define `setup` would collide. Threading one set across the blocks - // makes the second one `path#setup~2`, and the rename is applied to that - // block's edges too so nothing points at an id that no longer exists. - const minted = new Set([rel]); - - for (const body of blocks(root, lang)) { - const script = source.slice(body.startIndex, body.endIndex); - - let inner: ExtractResult; - try { - inner = extractFile(rel, script, lang.inner); - } catch { - continue; // one bad block, not a bad build - } - - // `raw_text` starts immediately after the `>` of the opening tag, so its - // row IS the tag's row and the slice begins with that line's newline. - // Script line 1 is therefore the tail of the tag line, and script line N - // lands on `.vue` line row + N — which is exactly "add the start row to a - // 1-based span". Taking the row from the tag node instead would look - // equivalent and be right only when the tag has no attributes. - const shift = body.startPosition.row; - - // nodes[0] is the script's own file node: it describes the block, not the - // file, so it is dropped and its residual folded into the .vue file node. - const [scriptFile, ...symbols] = inner.nodes; - if (scriptFile?.body_text) residuals.push(scriptFile.body_text); - - const renamed = new Map(); - for (const node of symbols) { - const id = mintId(node.id, minted); - if (id !== node.id) renamed.set(node.id, id); - nodes.push({ ...node, id, span: shiftSpan(node.span, shift) }); - } + let tree: WasmTree | null = null; + if (language) { + try { + tree = parseWasm(language, source); + } catch (err) { + // Same contract as the generic tier: a grammar that throws is a build error + // for this file, never a clean file node cached as success. + throw new Error(`${lang.name} grammar threw: ${err instanceof Error ? err.message : String(err)}`); + } + } - for (const edge of inner.rawEdges) { - const source_ = renamed.get(edge.source) ?? edge.source; - const targetId = edge.targetId === undefined ? undefined : (renamed.get(edge.targetId) ?? edge.targetId); - rawEdges.push({ ...edge, source: source_, ...(targetId === undefined ? {} : { targetId }) }); + try { + if (tree) { + const root = tree.rootNode; + // Ids are minted per file by the inner extractor, so two script blocks that + // both define `setup` would collide. Threading one set across the blocks + // makes the second one `path#setup~2`, and the rename is applied to that + // block's edges too so nothing points at an id that no longer exists. + const minted = new Set([rel]); + + for (const body of blocks(root, lang)) { + const script = source.slice(body.startIndex, body.endIndex); + + let inner: ExtractResult; + try { + inner = extractFile(rel, script, lang.inner); + } catch { + continue; // one bad block, not a bad build + } + + // `raw_text` starts immediately after the `>` of the opening tag, so its + // row IS the tag's row and the slice begins with that line's newline. + // Script line 1 is therefore the tail of the tag line, and script line N + // lands on `.vue` line row + N — which is exactly "add the start row to a + // 1-based span". Taking the row from the tag node instead would look + // equivalent and be right only when the tag has no attributes. + const shift = body.startPosition.row; + + // nodes[0] is the script's own file node: it describes the block, not the + // file, so it is dropped and its residual folded into the .vue file node. + const [scriptFile, ...symbols] = inner.nodes; + if (scriptFile?.body_text) residuals.push(scriptFile.body_text); + + const renamed = new Map(); + for (const node of symbols) { + const id = mintId(node.id, minted); + if (id !== node.id) renamed.set(node.id, id); + nodes.push({ ...node, id, span: shiftSpan(node.span, shift) }); + } + + for (const edge of inner.rawEdges) { + const source_ = renamed.get(edge.source) ?? edge.source; + const targetId = edge.targetId === undefined ? undefined : (renamed.get(edge.targetId) ?? edge.targetId); + rawEdges.push({ ...edge, source: source_, ...(targetId === undefined ? {} : { targetId }) }); + } } } + } finally { + // The wrapper tree is a WASM-heap object; nothing frees it but this. + tree?.delete(); } // Built last so it can carry the residual, but unshifted first so the file node diff --git a/src/graph/generic.ts b/src/graph/generic.ts index b599deb77..9202064c8 100644 --- a/src/graph/generic.ts +++ b/src/graph/generic.ts @@ -83,13 +83,50 @@ const KIND: Record = { object: "class", property: "variable", // Scala object; Swift/Scala property }; +/** The parse-side of web-tree-sitter graft touches. Both are Emscripten-heap + * objects: nothing frees them but an explicit `delete()`. Declared here rather + * than imported so a fake can stand in for tests. `new Parser()` is assignable + * to `WasmParser` without a cast; if an upgrade breaks that, this is where it + * shows. */ +export interface WasmTree { rootNode: TsNode; delete(): void } +export interface WasmParser { + parse(cb: (index: number) => string): WasmTree | null; + reset(): void; + delete(): void; +} // Loaded grammars + compiled tags queries, keyed by graft lang name. Populated by // warmGenericGrammars; read synchronously by extractGeneric. -export interface Loaded { language: unknown; query: unknown | null } +/** One warmed grammar. `parser` is created once per grammar and reused for every + * file: creating one per parse and never deleting it is what exhausted the 2 GB + * WASM heap on a 65k-file repo (`Aborted()` on every file after ~12k). */ +export interface Loaded { language: unknown; query: unknown | null; parser: WasmParser } const loaded = new Map(); let tsMod: typeof import("web-tree-sitter") | null = null; let initPromise: Promise | null = null; +/** The first abort message web-tree-sitter produced in this process, if any. + * Emscripten's abort() sets a process-wide flag and the heap never recovers, so + * after one real abort every further parse is doomed: better one clear error + * than one per remaining file. */ +let poisoned: string | null = null; +export function wasmRuntimePoisoned(): string | null { return poisoned; } +export function clearWasmPoisonForTest(): void { poisoned = null; } +function notePoison(err: unknown): string { + const msg = err instanceof Error ? err.message : String(err); + // WebAssembly is a real Node runtime global but is NOT in this project's TS lib + // (ES2022, no DOM), so read its RuntimeError constructor off globalThis through a + // locally typed view. A `declare global { namespace WebAssembly … }` augmentation + // would conflict with lib.dom's `interface`+`var` for downstream consumers and be + // emitted verbatim into dist/*.d.ts; this keeps the type strictly local. + const RuntimeError = (globalThis as { WebAssembly?: { RuntimeError?: abstract new (...args: never[]) => Error } }) + .WebAssembly?.RuntimeError; + if ((RuntimeError !== undefined && err instanceof RuntimeError) || /^Aborted\(/.test(msg)) poisoned = poisoned ?? msg; + return msg; +} +function poisonedError(langName: string): Error { + return new Error(`${langName} grammar unavailable: web-tree-sitter aborted earlier in this process (${poisoned}); restart the process and rebuild`); +} + function requireWasm(wasm: string): Buffer | null { // Resolve the grammar wasm from the tree-sitter-wasm bundle (its package.json // `exports` maps the bare "/…" subpath to the actual "out//…" file). @@ -141,7 +178,9 @@ export async function warmGenericGrammars(langNames: Iterable): Promise< if (scm) { try { query = new Query(language, scm); } catch { query = null; } } - loaded.set(name, { language, query }); + const parser = new tsMod.Parser(); + parser.setLanguage(language); + loaded.set(name, { language, query, parser }); } catch { /* grammar failed to instantiate — skip; files extract as file-only */ } @@ -189,18 +228,30 @@ export async function loadWasmLanguage(wasm: string): Promise { const PARSE_CHUNK = 16384; // <32KB slices — same tree-sitter limit workaround as extract.ts -/** Parse with an already-loaded grammar. Returns the root node, or null if the - * grammar was never warmed or the parse blew up. Companion to - * `loadWasmLanguage` for callers outside this module. */ -export function parseWasm(language: unknown, source: string): TsNode | null { - if (!tsMod) return null; +/** One shared parser per language object, for callers that loaded a grammar via + * `loadWasmLanguage` (the container tier) rather than `warmGenericGrammars`. */ +const parsersByLanguage = new WeakMap(); + +/** Parse with an already-loaded grammar. Returns the tree, or null only when + * web-tree-sitter was never initialised. Throws when the parse blows up, so a + * WASM abort is a build error and not a silently symbol-less file. The CALLER + * owns the tree and must `delete()` it: it lives in the WASM heap, not V8's. */ +export function parseWasm(language: unknown, source: string): WasmTree | null { + if (!tsMod || typeof language !== "object" || language === null) return null; + if (poisoned) throw poisonedError("wasm"); + let parser = parsersByLanguage.get(language); + if (!parser) { + const p = new tsMod.Parser(); + p.setLanguage(language as never); + parser = p; + parsersByLanguage.set(language, parser); + } try { - const parser = new tsMod.Parser(); - parser.setLanguage(language as never); - const tree = parser.parse((i: number) => source.slice(i, i + PARSE_CHUNK)); - return (tree?.rootNode as TsNode) ?? null; - } catch { - return null; + return parser.parse((i: number) => source.slice(i, i + PARSE_CHUNK)); + } catch (err) { + const msg = notePoison(err); + try { parser.reset(); } catch { /* runtime gone */ } + throw new Error(msg); } } @@ -225,63 +276,85 @@ export function extractGeneric(rel: string, source: string, langName: string): E const entry = loaded.get(langName); if (!entry || !tsMod) return { nodes, rawEdges }; - let tree; + if (poisoned) throw poisonedError(langName); + let tree: WasmTree | null; try { - const parser = new tsMod.Parser(); - parser.setLanguage(entry.language as never); - tree = parser.parse((i: number) => source.slice(i, i + PARSE_CHUNK)); + tree = entry.parser.parse((i: number) => source.slice(i, i + PARSE_CHUNK)); } catch (err) { // A grammar that throws (e.g. a WASM "memory access out of bounds" from an // external scanner) is not a per-file syntax problem: swallowing it here left // the file with only its file node and the build reporting success. Rethrow so - // build.ts records it in `errors`, the CLI prints it, and the extract cache - // remembers the failure instead of caching an empty result as clean. - throw new Error(`${langName} grammar threw: ${err instanceof Error ? err.message : String(err)}`); + // build.ts records it in `errors` and the CLI prints it. + // + // The parser is shared across files, so put it back to a known state first: + // a parse that threw mid-way can leave the stack half-built, and the next + // file would inherit it. + const msg = notePoison(err); + try { entry.parser.reset(); } catch { /* the runtime is already gone */ } + throw new Error(`${langName} grammar threw: ${msg}`); } if (!tree) return { nodes, rawEdges }; - const minted = new Set([rel]); - const lines = source.split("\n"); - const defs: Def[] = []; - // One definition per source span: a grammar's tags.scm can capture the same node - // under two @definition kinds (Swift `func` is method AND function), and the - // walker can revisit a nested match — both would emit near-duplicate nodes. - const spanSeen = new Set(); - // Mint one definition node from a whole-definition tree node. Shared by both - // the tags.scm path and the walker fallback so id/span/signature/body_text are - // built identically. - const mkDef = (name: string, kind: Kind, whole: TsNode): void => { - if (spanSeen.has(whole.startIndex)) return; - spanSeen.add(whole.startIndex); - const idBase = `${rel}#${name}`; - let id = idBase, n = 2; - while (minted.has(id)) id = `${idBase}~${n++}`; - minted.add(id); - const startRow = whole.startPosition.row, endRow = whole.endPosition.row; - const sigLine = (lines[startRow] ?? "").trim().replace(/\s*\{?\s*$/, ""); - nodes.push({ - id, name, kind, path: rel, - span: `L${startRow + 1}-L${endRow + 1}`, - signature: sigLine || null, exported: true, origin: "generic", - body_hash: contentHash(source.slice(whole.startIndex, whole.endIndex)), - body_text: source.slice(whole.startIndex, whole.endIndex).replace(/\s+/g, " ").slice(0, 5000), - summary_state: "pending", summary: null, crux: null, - }); - defs.push({ id, startIndex: whole.startIndex, endIndex: whole.endIndex }); - }; + try { + const minted = new Set([rel]); + const lines = source.split("\n"); + const defs: Def[] = []; + // One definition per source span: a grammar's tags.scm can capture the same node + // under two @definition kinds (Swift `func` is method AND function), and the + // walker can revisit a nested match — both would emit near-duplicate nodes. + const spanSeen = new Set(); + // Mint one definition node from a whole-definition tree node. Shared by both + // the tags.scm path and the walker fallback so id/span/signature/body_text are + // built identically. + const mkDef = (name: string, kind: Kind, whole: TsNode): void => { + if (spanSeen.has(whole.startIndex)) return; + spanSeen.add(whole.startIndex); + const idBase = `${rel}#${name}`; + let id = idBase, n = 2; + while (minted.has(id)) id = `${idBase}~${n++}`; + minted.add(id); + const startRow = whole.startPosition.row, endRow = whole.endPosition.row; + const sigLine = (lines[startRow] ?? "").trim().replace(/\s*\{?\s*$/, ""); + nodes.push({ + id, name, kind, path: rel, + span: `L${startRow + 1}-L${endRow + 1}`, + signature: sigLine || null, exported: true, origin: "generic", + body_hash: contentHash(source.slice(whole.startIndex, whole.endIndex)), + body_text: source.slice(whole.startIndex, whole.endIndex).replace(/\s+/g, " ").slice(0, 5000), + summary_state: "pending", summary: null, crux: null, + }); + defs.push({ id, startIndex: whole.startIndex, endIndex: whole.endIndex }); + }; - if (entry.query) { - tagsExtract(entry.query, tree.rootNode as TsNode, rel, mkDef, defs, rawEdges, langName); - } else { - walkExtract(tree.rootNode as TsNode, mkDef); // no tags.scm → symbols only + if (entry.query) { + tagsExtract(entry.query, tree.rootNode as TsNode, rel, mkDef, defs, rawEdges, langName); + } else { + walkExtract(tree.rootNode as TsNode, mkDef); // no tags.scm → symbols only + } + // The preprocessor is invisible to tags.scm, but in C/C++ a local `#include "x.h"` + // IS the dependency graph — capture it as a file→file import. Likewise a Rust + // `use crate::…` is an in-crate module dependency. + if (langName === "c" || langName === "cpp") extractIncludes(tree.rootNode as TsNode, rel, rawEdges); + else if (langName === "rust") extractUses(tree.rootNode as TsNode, rel, rawEdges); + else if (langName === "php") extractPhpUses(tree.rootNode as TsNode, rel, rawEdges); + return { nodes, rawEdges }; + } catch (err) { + // The walk is WASM-backed too (query.matches() runs in the grammar's runtime), + // so a real abort can surface here, not only from parse(). Route it through + // notePoison so one abort poisons the runtime and every later file gets the + // stable "aborted earlier" error instead of re-attempting a dead runtime. + // Rethrow the ORIGINAL error object unchanged so an ordinary walk failure keeps + // its message (build.ts records it per file). + notePoison(err); + throw err; + } finally { + // Every tree lives in the Emscripten heap until deleted. Without this, a + // 65k-file build leaks ~2 GB and tree-sitter's allocator calls abort(). + // Swallow a throw from delete(): the primary error (from the try or the catch) + // must never be masked by a delete failure, and a delete() that throws means the + // runtime is already gone — there is nothing left to free. + try { tree.delete(); } catch { /* runtime already gone; keep the real error */ } } - // The preprocessor is invisible to tags.scm, but in C/C++ a local `#include "x.h"` - // IS the dependency graph — capture it as a file→file import. Likewise a Rust - // `use crate::…` is an in-crate module dependency. - if (langName === "c" || langName === "cpp") extractIncludes(tree.rootNode as TsNode, rel, rawEdges); - else if (langName === "rust") extractUses(tree.rootNode as TsNode, rel, rawEdges); - else if (langName === "php") extractPhpUses(tree.rootNode as TsNode, rel, rawEdges); - return { nodes, rawEdges }; } /** PHP `use App\Models\User;` → a file→class-file `imports` raw edge, one per imported diff --git a/test/container-extract.test.ts b/test/container-extract.test.ts index f5855fc3b..ffc8ff898 100644 --- a/test/container-extract.test.ts +++ b/test/container-extract.test.ts @@ -23,6 +23,8 @@ import { containerLangOf, containerExtensions, isContainerWarm, + CONTAINER_LANGS, + swapContainerGrammarForTest, } from "../src/graph/container.js"; import { supportedExtensions } from "../src/graph/source-files.js"; import { buildGraph } from "../src/graph/build.js"; @@ -290,3 +292,17 @@ test("container: a clean build of a .vue file checks as in sync", async () => { assert.deepEqual(check.changed, []); assert.equal(check.ok, true, "a clean build checks OK"); }); + +test("a wrapper grammar that throws is a build error, not a clean symbol-less file", async () => { + await warmContainerGrammars(["vue"]); + const vue = CONTAINER_LANGS.find((l) => l.name === "vue")!; + const prev = swapContainerGrammarForTest("vue", new Proxy({}, { get(): never { throw new Error("memory access out of bounds (fake)"); } })); + try { + assert.throws( + () => extractContainer("App.vue", "\n", vue), + /vue grammar threw: memory access out of bounds/, + ); + } finally { + swapContainerGrammarForTest("vue", prev); + } +}); diff --git a/test/generic-extract.test.ts b/test/generic-extract.test.ts index aa7498264..22be35889 100644 --- a/test/generic-extract.test.ts +++ b/test/generic-extract.test.ts @@ -17,7 +17,10 @@ import { loadWasmLanguage, parseWasm, swapGrammarForTest, + wasmRuntimePoisoned, + clearWasmPoisonForTest, type TsNode, + type Loaded, } from "../src/graph/generic.js"; import { resolveEdges } from "../src/graph/resolve.js"; import { buildGraph } from "../src/graph/build.js"; @@ -397,13 +400,18 @@ function namedOfType(root: TsNode, type: string): string[] { async function assertPhpWasmExtractsClassAndMethods(source: string, className: string, label: string): Promise { const language = await loadWasmLanguage("php"); assert.ok(language, "tree-sitter-wasm must ship a php grammar"); - const root = parseWasm(language, source); - assert.ok(root, `${label}: PHP wasm parse must not crash (1.1.4 threw on heredoc/nowdoc)`); - const classes = namedOfType(root, "class_declaration"); - const methods = namedOfType(root, "method_declaration"); - assert.ok(classes.includes(className), `${label}: expected class ${className}, got: ${classes.join(", ") || "(none)"}`); - assert.ok(methods.includes("sql"), `${label}: expected method sql, got: ${methods.join(", ") || "(none)"}`); - assert.ok(methods.includes("other"), `${label}: expected method other, got: ${methods.join(", ") || "(none)"}`); + const tree = parseWasm(language, source); + assert.ok(tree, `${label}: PHP wasm parse must not crash (1.1.4 threw on heredoc/nowdoc)`); + try { + const root = tree.rootNode; + const classes = namedOfType(root, "class_declaration"); + const methods = namedOfType(root, "method_declaration"); + assert.ok(classes.includes(className), `${label}: expected class ${className}, got: ${classes.join(", ") || "(none)"}`); + assert.ok(methods.includes("sql"), `${label}: expected method sql, got: ${methods.join(", ") || "(none)"}`); + assert.ok(methods.includes("other"), `${label}: expected method other, got: ${methods.join(", ") || "(none)"}`); + } finally { + tree.delete(); + } } test("PHP wasm grammar extracts class + methods from a heredoc file (#139)", async () => { @@ -421,12 +429,17 @@ test("PHP wasm grammar extracts class + methods from a nowdoc file (#139)", asyn * clean parse. The real crash is heap-state dependent, so a deterministically * throwing fake grammar is swapped in via the test seam instead. */ -const THROWING_GRAMMAR = { - // web-tree-sitter's setLanguage() inspects the language object, so any - // property access blowing up makes extractGeneric's try block throw exactly - // like a crashing wasm scanner does. - language: new Proxy({}, { get(): never { throw new Error("memory access out of bounds (fake)"); } }), +const THROWING_GRAMMAR: Loaded = { + // The parser is created once per grammar at warm time, so the per-file failure + // an external scanner produces is a throw out of parse(). reset() is what + // extractGeneric calls on the shared parser before rethrowing. + language: {}, query: null, + parser: { + parse(): never { throw new Error("memory access out of bounds (fake)"); }, + reset(): void {}, + delete(): void {}, + }, }; test("extractGeneric rethrows a throwing grammar with the language named (#139)", async () => { @@ -471,3 +484,165 @@ test("a throwing grammar is a per-file build error, cached as a failure (#139)", swapGrammarForTest("rust", prev); } }); + +/** Wrap the real rust grammar so every tree it hands out is counted on the way + * out and on delete(). The parse itself is real; only the bookkeeping is fake. */ +function countingGrammar(real: Loaded, hooks: { created: number; deleted: number; resets: number }): Loaded { + return { + language: real.language, + query: real.query, + parser: { + parse(cb) { + const t = real.parser.parse(cb); + if (!t) return null; + hooks.created++; + return { rootNode: t.rootNode, delete: () => { hooks.deleted++; t.delete(); } }; + }, + reset: () => { hooks.resets++; real.parser.reset(); }, + delete: () => real.parser.delete(), + }, + }; +} + +test("extractGeneric deletes every tree it parses", async () => { + await warmGenericGrammars(["rust"]); + const hooks = { created: 0, deleted: 0, resets: 0 }; + const real = swapGrammarForTest("rust", null)!; + swapGrammarForTest("rust", countingGrammar(real, hooks)); + try { + for (let i = 0; i < 25; i++) extractGeneric("src/lib.rs", `pub fn f${i}() { g() }\n`, "rust"); + assert.equal(hooks.created, 25, "every call parsed"); + assert.equal(hooks.deleted, 25, "every tree deleted"); + assert.equal(hooks.resets, 0, "no reset on the happy path"); + } finally { + swapGrammarForTest("rust", real); + } +}); + +test("extractGeneric deletes the tree even when the walk throws", async () => { + await warmGenericGrammars(["rust"]); + const hooks = { created: 0, deleted: 0, resets: 0 }; + const real = swapGrammarForTest("rust", null)!; + // tagsExtract calls query.matches(root); a query that throws there throws mid-walk. + const grammar = countingGrammar(real, hooks); + grammar.query = { matches(): never { throw new Error("walk boom"); } }; + swapGrammarForTest("rust", grammar); + try { + assert.throws(() => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), /walk boom/); + assert.equal(hooks.created, 1); + assert.equal(hooks.deleted, 1, "the tree was deleted in finally"); + } finally { + swapGrammarForTest("rust", real); + } +}); + +test("a parse() that throws resets the shared parser before rethrowing", async () => { + await warmGenericGrammars(["rust"]); + const hooks = { created: 0, deleted: 0, resets: 0 }; + const real = swapGrammarForTest("rust", null)!; + const grammar = countingGrammar(real, hooks); + grammar.parser = { ...grammar.parser, parse(): never { throw new Error("memory access out of bounds (fake)"); } }; + swapGrammarForTest("rust", grammar); + try { + assert.throws(() => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), /rust grammar threw: memory access out of bounds/); + assert.equal(hooks.resets, 1, "reset() called once"); + assert.equal(wasmRuntimePoisoned(), null, "an ordinary throw does not poison the runtime"); + } finally { + swapGrammarForTest("rust", real); + } +}); + +test("a WebAssembly.RuntimeError poisons the runtime: every later call fails with one distinct message", async () => { + await warmGenericGrammars(["rust"]); + const real = swapGrammarForTest("rust", null)!; + const grammar: Loaded = { + language: real.language, query: real.query, + parser: { parse(): never { throw new WebAssembly.RuntimeError("Aborted(). Build with -sASSERTIONS for more info."); }, reset() {}, delete() {} }, + }; + swapGrammarForTest("rust", grammar); + try { + assert.throws(() => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), /rust grammar threw: Aborted\(\)/); + assert.match(wasmRuntimePoisoned() ?? "", /^Aborted\(\)/); + // The real grammar is back, but the process is poisoned: no parse is attempted. + swapGrammarForTest("rust", real); + assert.throws( + () => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), + /web-tree-sitter aborted earlier in this process .*restart/, + ); + } finally { + clearWasmPoisonForTest(); + swapGrammarForTest("rust", real); + } +}); + +test("a WebAssembly.RuntimeError from the walk poisons the runtime too", async () => { + await warmGenericGrammars(["rust"]); + const real = swapGrammarForTest("rust", null)!; + // query.matches() is WASM-backed, so an abort can surface from the walk, not + // only from parse(). It must poison the runtime the same way a parse abort does, + // and the thrown error must be the original object (its message carries Aborted()). + const grammar: Loaded = { + language: real.language, + query: { matches(): never { throw new WebAssembly.RuntimeError("Aborted(). Build with -sASSERTIONS for more info."); } }, + parser: real.parser, + }; + swapGrammarForTest("rust", grammar); + try { + assert.throws(() => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), /Aborted\(\)/); + assert.match(wasmRuntimePoisoned() ?? "", /^Aborted\(\)/); + // The real grammar is back, but the process is poisoned: no parse is attempted. + swapGrammarForTest("rust", real); + assert.throws( + () => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), + /web-tree-sitter aborted earlier in this process .*restart/, + ); + } finally { + clearWasmPoisonForTest(); + swapGrammarForTest("rust", real); + } +}); + +test("a tree.delete() that throws does not mask the walk's error", async () => { + await warmGenericGrammars(["rust"]); + const hooks = { created: 0, deleted: 0, resets: 0 }; + const real = swapGrammarForTest("rust", null)!; + // Like countingGrammar, but the tree's delete() throws AFTER counting, and the + // walk throws first. The delete throw must be swallowed so the walk's real error + // (not the delete error) is what propagates. + const grammar: Loaded = { + language: real.language, + query: { matches(): never { throw new Error("walk boom"); } }, + parser: { + parse(cb) { + const t = real.parser.parse(cb); + if (!t) return null; + hooks.created++; + return { rootNode: t.rootNode, delete: () => { hooks.deleted++; t.delete(); throw new Error("delete boom"); } }; + }, + reset: () => { hooks.resets++; real.parser.reset(); }, + delete: () => real.parser.delete(), + }, + }; + swapGrammarForTest("rust", grammar); + try { + assert.throws(() => extractGeneric("src/lib.rs", "pub fn f() {}\n", "rust"), /walk boom/); + assert.equal(hooks.deleted, 1, "delete() ran (and threw) but the throw was swallowed"); + } finally { + swapGrammarForTest("rust", real); + } +}); + +test("extractGeneric frees WASM memory: 400 parses of a 40 KB file do not grow RSS", async () => { + await warmGenericGrammars(["rust"]); + assert.ok(isWarm("rust"), "rust grammar warmed"); + const fn = (i: number) => `pub fn f${i}(a: u32, b: u32) -> u32 {\n let c = a + b;\n if c > 10 { helper(c) } else { c }\n}\n`; + const source = Array.from({ length: 400 }, (_, i) => fn(i)).join("\n"); + for (let i = 0; i < 20; i++) extractGeneric("src/lib.rs", source, "rust"); // warm-up + const before = process.memoryUsage().rss; + for (let i = 0; i < 400; i++) extractGeneric("src/lib.rs", source, "rust"); + const grewMb = (process.memoryUsage().rss - before) / 1048576; + // Leaking, each parse retains ~1.9 MB of WASM heap: 400 parses grow RSS by + // ~750 MB (and 1,100 parses would hit the 2 GB abort, so the loop stays at + // 400). Freed, growth is V8 noise. 100 MB is the line. + assert.ok(grewMb < 100, `RSS grew by ${grewMb.toFixed(0)} MB over 400 parses: the tree is not being deleted`); +}); From 79b96c9a9ea021898f867da48cd8b488953e8972 Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:55:28 -0600 Subject: [PATCH 2/8] fix(build): retry a file whose cached parse failed A cached error was replayed until the file's bytes changed, so one abort pinned 52k files as failed. Skip the cache hit for an errored entry. The fingerprint keeps its hash, so a refresh does not rebuild just to retry it. --- src/graph/build.ts | 16 ++++++++++------ test/generic-extract.test.ts | 29 ++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/graph/build.ts b/src/graph/build.ts index b3094051d..55f300b39 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -107,7 +107,7 @@ export interface GraphBuildResult { /** Per-file wiring cards written (Tier-2 passive surface). */ cards: number; files: number; - /** Files re-parsed this run (the rest were replayed from the extraction cache). */ + /** Files parsed this run (the rest were replayed from the extraction cache). Includes files whose previous build recorded a parse error: those are always re-attempted. */ parsed: number; /** Files replayed from the extraction cache. */ reused: number; @@ -244,14 +244,18 @@ export async function buildGraph( } const hash = contentHash(source); - if (cached && hash === cached.hash) { + // A cached FAILURE is never replayed. Most parse failures are not properties + // of the file: a WASM grammar that aborted because the heap was exhausted by + // the files before it fails again only if the heap is exhausted again. Replaying + // the error would pin it to the file until its bytes changed; on a 65k-file + // repo that was 52k files stuck failing after a single abort. Re-parsing a + // genuinely broken file costs one parse per build, which is nothing. (The + // pre-query refresh is unaffected: fingerprint.ts keeps an errored entry with + // a real hash on its stat fast path, so only an explicit build retries.) + if (cached && hash === cached.hash && !cached.error) { entries[rel] = { ...cached, size: f.size, mtimeMs: f.mtimeMs }; sources.set(rel, source); reused++; - if (cached.error) { - errors.push(cached.error); // this file failed to parse last time too - return; - } nodes.push(...cached.nodes); rawEdges.push(...cached.rawEdges); langs.add(label); diff --git a/test/generic-extract.test.ts b/test/generic-extract.test.ts index 22be35889..a129071d2 100644 --- a/test/generic-extract.test.ts +++ b/test/generic-extract.test.ts @@ -460,7 +460,7 @@ test("extractGeneric rethrows a throwing grammar with the language named (#139)" } }); -test("a throwing grammar is a per-file build error, cached as a failure (#139)", async () => { +test("a throwing grammar is a per-file build error, and the file is retried next build (#139)", async () => { const dir = mkdtempSync(join(tmpdir(), "graft-throwing-grammar-")); writeFileSync(join(dir, "lib.rs"), "pub fn f() {}\n"); await warmGenericGrammars(["rust"]); // so buildGraph's own warm call is a no-op @@ -474,17 +474,36 @@ test("a throwing grammar is a per-file build error, cached as a failure (#139)", assert.ok(g, "graph built"); assert.ok(!g!.nodes.some((n) => n.path === "lib.rs"), "failed file has no file node"); - // The extract cache must remember the failure, not an empty success: an - // incremental rebuild of the unchanged file replays the error. + // The failure is recorded (so the freshness probe is quiet) but NOT replayed: + // the second build parses the unchanged file again and, the grammar still + // throwing, reports the error again as a fresh parse, not a cache hit. const second = await buildGraph(dir, { reuse: true }); - assert.equal(second.parsed, 0, "unchanged file is not re-parsed"); - assert.equal(second.errors.length, 1, `error replayed (got: ${second.errors.join("; ")})`); + assert.equal(second.errors.length, 1, `error reported again (got: ${second.errors.join("; ")})`); assert.match(second.errors[0], /rust grammar threw/); + assert.equal(second.parsed, 1, "the errored file was re-parsed, not replayed"); + assert.equal(second.reused, 0, "an errored entry never counts as reused"); } finally { swapGrammarForTest("rust", prev); } }); +test("a file that failed for an environmental reason recovers on the next build without a cache delete", async () => { + const dir = mkdtempSync(join(tmpdir(), "graft-recovering-grammar-")); + writeFileSync(join(dir, "lib.rs"), "pub fn f() {}\npub fn g() { f() }\n"); + await warmGenericGrammars(["rust"]); + const prev = swapGrammarForTest("rust", THROWING_GRAMMAR); + try { + const first = await buildGraph(dir, { reuse: false }); + assert.equal(first.errors.length, 1, "first build fails"); + } finally { + swapGrammarForTest("rust", prev); // the "environment" is healthy again + } + const second = await buildGraph(dir, { reuse: true }); + assert.equal(second.errors.length, 0, `second build clean (got: ${second.errors.join("; ")})`); + const g = readGraph(wiringPath(contextDirFor(dir))); + assert.ok(g!.nodes.some((n) => n.id === "lib.rs#f"), "the file's symbols are in the graph now"); +}); + /** Wrap the real rust grammar so every tree it hands out is counted on the way * out and on delete(). The parse itself is real; only the bookkeeping is fake. */ function countingGrammar(real: Loaded, hooks: { created: number; deleted: number; resets: number }): Loaded { From 9559d0f08871fa8a247658dd2b18cdefc02921d7 Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:50:13 -0600 Subject: [PATCH 3/8] fix(cli): cap the build error listing at 20 lines; changelog --- CHANGELOG.md | 18 +++++++++++++++ src/cli.ts | 6 ++++- src/graph/build.ts | 5 ++-- src/graph/generic.ts | 4 ++++ test/generic-extract.test.ts | 44 ++++++++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ef248a39..73388d552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## Unreleased + +### Fixed + +- **`graft build` no longer aborts partway through very large repos.** The + breadth-tier extractor created a web-tree-sitter parser per file and never + freed it or the syntax tree; those live in the WASM heap, which is capped at + 2 GB, so on a C source base of ~65,000 files every parse after ~12,000 failed + with `Aborted()`. One parser per grammar is now reused and every tree is + deleted after extraction, in the container (`.vue`) tier as well as the + breadth tier. If the runtime does abort, the build reports it once and says + to restart, instead of once per remaining file. +- **A parse failure is retried on the next `graft build`** instead of being + replayed from the extract cache until the file's bytes change, so a one-off + failure no longer needs a manual cache delete to clear. +- **A `.vue` wrapper grammar that throws is now a build error** for that file, + not a silently symbol-less file cached as a success. + ## 0.19.0 ### Added diff --git a/src/cli.ts b/src/cli.ts index 54e97a9f2..e7656d563 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -583,7 +583,11 @@ program }, { repo: buildRoot }, ); - for (const e of g.errors) console.error(`✗ ${e}`); + // A poisoned WASM runtime or a broken grammar can fail tens of thousands of + // files identically; the first 20 say everything the rest would. + const MAX_ERRORS_SHOWN = 20; + for (const e of g.errors.slice(0, MAX_ERRORS_SHOWN)) console.error(`✗ ${e}`); + if (g.errors.length > MAX_ERRORS_SHOWN) console.error(`✗ … and ${g.errors.length - MAX_ERRORS_SHOWN} more (${g.errors.length} files failed to parse)`); const rel = relative(process.cwd(), g.contextDir) || "graft"; if (process.env.GRAFT_NO_GITIGNORE) { diff --git a/src/graph/build.ts b/src/graph/build.ts index 55f300b39..cf316d793 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -250,8 +250,9 @@ export async function buildGraph( // the error would pin it to the file until its bytes changed; on a 65k-file // repo that was 52k files stuck failing after a single abort. Re-parsing a // genuinely broken file costs one parse per build, which is nothing. (The - // pre-query refresh is unaffected: fingerprint.ts keeps an errored entry with - // a real hash on its stat fast path, so only an explicit build retries.) + // pre-query refresh never rebuilds solely to retry an errored file: + // fingerprint.ts keeps the entry's real hash on its stat fast path. A rebuild + // triggered by other drift re-parses it like any build.) if (cached && hash === cached.hash && !cached.error) { entries[rel] = { ...cached, size: f.size, mtimeMs: f.mtimeMs }; sources.set(rel, source); diff --git a/src/graph/generic.ts b/src/graph/generic.ts index 9202064c8..36ca98f8d 100644 --- a/src/graph/generic.ts +++ b/src/graph/generic.ts @@ -204,6 +204,10 @@ export function swapGrammarForTest(name: string, entry: Loaded | null): Loaded | return prev; } +/** Test seam: pre-seed the parser parseWasm will use for `language`, so a + * throwing parse() can be driven without a real crashing grammar. */ +export function seedWasmParserForTest(language: object, parser: WasmParser): void { parsersByLanguage.set(language, parser); } + /** Load one grammar from the tree-sitter-wasms bundle, initialising * web-tree-sitter on first call. Null when the wasm is missing or won't * instantiate — never throws, so a caller degrades instead of failing the build. diff --git a/test/generic-extract.test.ts b/test/generic-extract.test.ts index a129071d2..554c5ed71 100644 --- a/test/generic-extract.test.ts +++ b/test/generic-extract.test.ts @@ -17,10 +17,12 @@ import { loadWasmLanguage, parseWasm, swapGrammarForTest, + seedWasmParserForTest, wasmRuntimePoisoned, clearWasmPoisonForTest, type TsNode, type Loaded, + type WasmParser, } from "../src/graph/generic.js"; import { resolveEdges } from "../src/graph/resolve.js"; import { buildGraph } from "../src/graph/build.js"; @@ -665,3 +667,45 @@ test("extractGeneric frees WASM memory: 400 parses of a 40 KB file do not grow R // 400). Freed, growth is V8 noise. 100 MB is the line. assert.ok(grewMb < 100, `RSS grew by ${grewMb.toFixed(0)} MB over 400 parses: the tree is not being deleted`); }); + +// parseWasm has its OWN parse-catch path (generic.ts, distinct from extractGeneric's): +// notePoison → parser.reset() → throw. It is the path the container (.vue) tier hits. +// parseWasm keys a WeakMap by the language object and creates the +// parser itself on first use, so a fake language cannot make parse() throw on its own — +// seedWasmParserForTest pre-seeds the parser parseWasm will use, so a throwing parse() +// can be driven without a real crashing grammar. +test("parseWasm rethrows a throwing parse and resets the shared parser (an ordinary throw does not poison)", async () => { + await warmGenericGrammars(["rust"]); // initialises web-tree-sitter + const language = {}; + let resets = 0; + const parser: WasmParser = { + parse(): never { throw new Error("memory access out of bounds (fake)"); }, + reset(): void { resets++; }, + delete(): void {}, + }; + seedWasmParserForTest(language, parser); + assert.throws(() => parseWasm(language, "x"), /memory access out of bounds/); + assert.equal(resets, 1, "reset() called once before rethrow"); + assert.equal(wasmRuntimePoisoned(), null, "an ordinary throw does not poison the runtime"); +}); + +test("parseWasm poisons the runtime on a WebAssembly.RuntimeError; every later call fails with the aborted-earlier error", async () => { + await warmGenericGrammars(["rust"]); + const language = {}; + // The ES2022 lib has no WebAssembly type; read its RuntimeError constructor off globalThis. + const RuntimeError = (globalThis as { WebAssembly: { RuntimeError: new (m: string) => Error } }).WebAssembly.RuntimeError; + const parser: WasmParser = { + parse(): never { throw new RuntimeError("Aborted()."); }, + reset(): void {}, + delete(): void {}, + }; + seedWasmParserForTest(language, parser); + try { + assert.throws(() => parseWasm(language, "x"), /Aborted\(/); + assert.match(wasmRuntimePoisoned() ?? "", /^Aborted\(/); + // The runtime is poisoned now: a further parseWasm never attempts a parse. + assert.throws(() => parseWasm(language, "x"), /aborted earlier in this process/); + } finally { + clearWasmPoisonForTest(); + } +}); From 487513a21f303abe5a8ca3fa03cd4d8a748764ce Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Sun, 4 Oct 2026 11:24:53 -0600 Subject: [PATCH 4/8] fix(generic): walk syntax trees with an explicit stack A C++ header misparsed by the C grammar can nest one level per initializer element; the recursive walks overflowed the call stack. --- CHANGELOG.md | 4 +++ src/graph/generic.ts | 52 +++++++++++++++++------------------- test/generic-extract.test.ts | 20 ++++++++++++++ 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73388d552..d51a45cff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ failure no longer needs a manual cache delete to clear. - **A `.vue` wrapper grammar that throws is now a build error** for that file, not a silently symbol-less file cached as a success. +- **A deeply nested syntax tree no longer fails with `Maximum call stack size + exceeded`.** A C++ header with a large brace initializer misparses under the + C grammar into a tree one level deep per element; the breadth-tier walks now + use an explicit stack instead of recursing. ## 0.19.0 diff --git a/src/graph/generic.ts b/src/graph/generic.ts index 36ca98f8d..840418652 100644 --- a/src/graph/generic.ts +++ b/src/graph/generic.ts @@ -361,21 +361,32 @@ export function extractGeneric(rel: string, source: string, langName: string): E } } +/** Pre-order DFS over `root` and its named descendants, on an explicit stack: a + * misparse can nest one level per element (a C++ array initializer under the C + * grammar runs thousands deep), which overflows V8's stack if the walk recurses. Children + * are pushed in reverse so the visit order matches a recursive walk. */ +function forEachNamedNode(root: TsNode, fn: (n: TsNode) => void): void { + const stack: TsNode[] = [root]; + while (stack.length) { + const n = stack.pop()!; + fn(n); + for (let i = (n.namedChildCount ?? 0) - 1; i >= 0; i--) { + const c = n.namedChild?.(i); + if (c) stack.push(c); + } + } +} + /** PHP `use App\Models\User;` → a file→class-file `imports` raw edge, one per imported * name (a `{ … }` group expands to several). `use function`/`use const` are skipped — * those name a symbol, not a PSR-4 class file. resolve.ts settles the fully-qualified * name to the in-repo file by namespace suffix, and drops it when it can't. */ function extractPhpUses(root: TsNode, rel: string, rawEdges: RawEdge[]): void { - const visit = (n: TsNode): void => { + forEachNamedNode(root, (n) => { if (n.type === "namespace_use_declaration") { for (const fqn of phpUseNames(n.text)) rawEdges.push({ source: rel, relation: "imports", specifier: fqn, file: rel }); } - for (let i = 0; i < (n.namedChildCount ?? 0); i++) { - const c = n.namedChild?.(i); - if (c) visit(c); - } - }; - visit(root); + }); } /** The fully-qualified class names a PHP `use` declaration imports. Handles a plain @@ -399,17 +410,12 @@ function phpUseNames(text: string): string[] { * `super::`, `self::`, external crates, and globs are skipped — resolve.ts settles the * path against the file's crate root, and drops it when it can't. */ function extractUses(root: TsNode, rel: string, rawEdges: RawEdge[]): void { - const visit = (n: TsNode): void => { + forEachNamedNode(root, (n) => { if (n.type === "use_declaration") { const spec = rustUseModule(n.text); if (spec !== null) rawEdges.push({ source: rel, relation: "imports", specifier: spec ? `crate/${spec}` : "crate", file: rel }); } - for (let i = 0; i < (n.namedChildCount ?? 0); i++) { - const c = n.namedChild?.(i); - if (c) visit(c); - } - }; - visit(root); + }); } /** The crate-relative path a Rust `use crate::…` names (`::`→`/`), or null when it is not @@ -434,7 +440,7 @@ function rustUseModule(text: string): string | null { * in-repo header (relative to the including file, else a unique path-suffix match), and * keeps it as an external string when it cannot — never a guessed edge. */ function extractIncludes(root: TsNode, rel: string, rawEdges: RawEdge[]): void { - const visit = (n: TsNode): void => { + forEachNamedNode(root, (n) => { if (n.type === "preproc_include") { const raw = n.childForFieldName?.("path")?.text ?? ""; if (raw.startsWith('"')) { @@ -442,12 +448,7 @@ function extractIncludes(root: TsNode, rel: string, rawEdges: RawEdge[]): void { if (spec) rawEdges.push({ source: rel, relation: "imports", specifier: spec, file: rel }); } } - for (let i = 0; i < (n.namedChildCount ?? 0); i++) { - const c = n.namedChild?.(i); - if (c) visit(c); - } - }; - visit(root); + }); } /** tags.scm path: @definition. → nodes, @reference.call/@reference.send → @@ -548,18 +549,13 @@ function nodeName(node: TsNode): string | null { /** Walker fallback: DFS every named node, emit a def for each classified one * (symbols only — no call resolution without a query). */ function walkExtract(root: TsNode, mkDef: (name: string, kind: Kind, whole: TsNode) => void): void { - const visit = (n: TsNode): void => { + forEachNamedNode(root, (n) => { const kind = classifyKind(n.type); if (kind) { const name = nodeName(n); if (name) mkDef(name, kind, n); } - for (let i = 0; i < (n.namedChildCount ?? 0); i++) { - const c = n.namedChild?.(i); - if (c) visit(c); - } - }; - visit(root); + }); } /** Minimal structural view of a web-tree-sitter node — shared with container.ts. */ diff --git a/test/generic-extract.test.ts b/test/generic-extract.test.ts index 554c5ed71..ce8323752 100644 --- a/test/generic-extract.test.ts +++ b/test/generic-extract.test.ts @@ -218,6 +218,26 @@ test("breadth tier: C #include becomes a resolved file→file import (local only assert.deepEqual(targetsOf("src/amb.c"), ["util.h"], "ambiguous include kept external, never guessed"); }); +// A C++ header routed to the C grammar misparses a big brace initializer into +// right-nested comma_expressions: the tree is as deep as the array is long. The +// walks over it must not recurse per level, or V8's stack overflows (RangeError). +test("breadth tier: a C header with a 20,000-element initializer extracts without overflowing the stack", async () => { + await warmGenericGrammars(["c"]); + const n = 20000; + const values = Array.from({ length: n }, (_, i) => String(i + 1)).join(", "); + const src = + '#include "dep.h"\n' + + "#include \n" + + `std::array data = { ${values} };\n` + + "int after(void) { return 0; }\n"; + const { nodes, rawEdges } = extractGeneric("big.h", src, "c"); + assert.ok( + rawEdges.some((e) => e.relation === "imports" && e.specifier === "dep.h"), + "the quoted include before the array is captured", + ); + assert.ok(nodes.some((x) => x.kind === "function" && x.name === "after"), "the function after the array is extracted"); +}); + // Rust `use crate::…` → a file→module import, resolved against the file's crate root // (the lib.rs/main.rs dir). The longest-prefix rule disambiguates a module from an item // and a `foo.rs` from a `foo/mod.rs`; std/super/external/glob are skipped, and an From b5469ed447b6b084352ad434a2a49ae50adddafa Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:34:30 -0600 Subject: [PATCH 5/8] perf(graph): stream every build artifact past V8's string cap Each artifact was one JSON.stringify and one JSON.parse; V8 caps a string at 512 MB, and a 65k-file C repo produces a 1.9 GB extract cache, a 700 MB wiring.json and a 590 MB ask index. The extract cache is now NDJSON read one line at a time from a Buffer. wiring.json and the ask index are written one element per line (still plain JSON for every reader) and read back line by line when they are over the cap. --- src/ask/index-file.ts | 118 +++++++++++++++++------- src/graph/extract-cache.ts | 60 ++++++++++--- src/graph/write.ts | 96 ++++++++++++++------ src/util/json-stream.ts | 67 ++++++++++++++ test/graph-artifacts-stream.test.ts | 134 ++++++++++++++++++++++++++++ test/graph-incremental.test.ts | 20 ++--- test/graph-refresh.test.ts | 8 +- test/json-stream.test.ts | 49 ++++++++++ 8 files changed, 465 insertions(+), 87 deletions(-) create mode 100644 src/util/json-stream.ts create mode 100644 test/graph-artifacts-stream.test.ts create mode 100644 test/json-stream.test.ts diff --git a/src/ask/index-file.ts b/src/ask/index-file.ts index a418620dd..df3bcb425 100644 --- a/src/ask/index-file.ts +++ b/src/ask/index-file.ts @@ -19,9 +19,11 @@ * contribution is folded into the stored `df` at query time (see `ask.ts`), * which is why `df` here counts symbol/file nodes only. */ -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { constants as bufferConstants } from "node:buffer"; +import { readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; import type { GraphV1 } from "../graph/types.js"; +import { forEachLine, openAtomic } from "../util/json-stream.js"; import { CACHE_DIR } from "../context/node-file.js"; /** Words too common/short to carry query intent — dropped before scoring. */ @@ -116,44 +118,94 @@ export function writeAskIndex(outDir: string, graph: GraphV1): string { ? docs.reduce((a, d) => a + bagLen(d.body), 0) / docs.length : 0; - const index: AskIndex = { - version: 1, - avgBodyLen, - df: pairs(df), - docCount: nodes.length, - docs, - }; - const outPath = askIndexPath(outDir); - mkdirSync(dirname(outPath), { recursive: true }); - writeFileSync(outPath, JSON.stringify(index) + "\n"); + // Streamed one doc per line: still valid JSON for JSON.parse readers, but the + // newline at each doc boundary lets readAskIndexFromBuffer walk a file over + // V8's string cap without ever holding it as one string. On a 65k-file repo + // the one-shot string was ~360 MB, within the cap but with no headroom. Key + // order matches AskIndex (version, avgBodyLen, df, docCount, docs). + const w = openAtomic(outPath); + try { + w.write(`{"version":1,"avgBodyLen":${JSON.stringify(avgBodyLen)},"df":${JSON.stringify(pairs(df))},"docCount":${nodes.length},"docs":[`); + docs.forEach((doc, i) => w.write("\n" + JSON.stringify(doc) + (i < docs.length - 1 ? "," : ""))); + w.write("\n]}\n"); + w.commit(); + } catch (e) { + w.abort(); + throw e; + } return outPath; } -/** Read the ask sidecar. Returns null on a missing file, unparseable JSON, an - * unrecognized shape, an unknown `version`, or a `docCount` that doesn't match - * the number of docs actually stored (a corrupted/truncated sidecar would - * otherwise silently skew IDF) — any of which means the caller should fall - * back to live tokenization, never crash or trust bad data. */ -export function readAskIndex(outDir: string): AskIndex | null { +export interface ReadAskIndexOptions { maxStringLength?: number } + +/** Validate the shape of a parsed sidecar. Returns null on an unrecognized shape, + * an unknown `version`, or a `docCount` that doesn't match the number of docs + * actually stored (a corrupted/truncated sidecar would otherwise silently skew + * IDF). Runs on the result of both the JSON.parse and the streamed read path. */ +function validateAskIndex(raw: unknown): AskIndex | null { + if (!raw || typeof raw !== "object") return null; + const r = raw as Record; + if ( + r.version !== 1 || + typeof r.docCount !== "number" || + typeof r.avgBodyLen !== "number" || + !Array.isArray(r.df) || + !Array.isArray(r.docs) || + r.docCount !== r.docs.length + ) { + return null; + } + return raw as AskIndex; +} + +/** Read the ask sidecar. Returns null on a missing file, unparseable JSON, or a + * shape {@link validateAskIndex} rejects — any of which means the caller should + * fall back to live tokenization, never crash or trust bad data. A file over + * V8's string cap is walked line by line ({@link readAskIndexFromBuffer}); + * `opts.maxStringLength` lets a test force that path. */ +export function readAskIndex(outDir: string, opts: ReadAskIndexOptions = {}): AskIndex | null { const path = askIndexPath(outDir); - if (!existsSync(path)) return null; + let size: number; + try { size = statSync(path).size; } catch { return null; } + const cap = opts.maxStringLength ?? bufferConstants.MAX_STRING_LENGTH; + if (size > cap) { + try { return readAskIndexFromBuffer(readFileSync(path)); } catch { return null; } + } + let raw: unknown; try { - const raw = JSON.parse(readFileSync(path, "utf8")); - if ( - !raw || - typeof raw !== "object" || - raw.version !== 1 || - typeof raw.docCount !== "number" || - typeof raw.avgBodyLen !== "number" || - !Array.isArray(raw.df) || - !Array.isArray(raw.docs) || - raw.docCount !== raw.docs.length - ) { - return null; - } - return raw as AskIndex; + raw = JSON.parse(readFileSync(path, "utf8")); } catch { return null; } + return validateAskIndex(raw); +} + +/** Parse the line-per-doc shape `writeAskIndex` produces without ever holding + * the whole file as one string. Null when the buffer is not in that shape or + * fails {@link validateAskIndex}. */ +export function readAskIndexFromBuffer(buf: Buffer): AskIndex | null { + const DOCS_OPEN = ',"docs":['; + const END = "]}"; + let head: Record | null = null; + const docs: AskIndexDoc[] = []; + let closed = false; + let bad = false; + forEachLine(buf, (line, i) => { + if (bad || closed) { if (line !== "") bad = true; return; } + try { + if (i === 0) { + if (!line.startsWith("{") || !line.endsWith(DOCS_OPEN)) { bad = true; return; } + head = JSON.parse(line.slice(0, -DOCS_OPEN.length) + "}") as Record; + return; + } + if (line === END) { closed = true; return; } + const body = line.endsWith(",") ? line.slice(0, -1) : line; + docs.push(JSON.parse(body) as AskIndexDoc); + } catch { + bad = true; + } + }); + if (bad || !closed || head === null) return null; + return validateAskIndex({ ...(head as Record), docs }); } diff --git a/src/graph/extract-cache.ts b/src/graph/extract-cache.ts index 2acf8b258..aec139d5d 100644 --- a/src/graph/extract-cache.ts +++ b/src/graph/extract-cache.ts @@ -1,5 +1,5 @@ /** - * Per-file memo for Tier-1 extraction — `/.cache/extract.json`. + * Per-file memo for Tier-1 extraction — `/.cache/extract..ndjson`. * * `extractFile` is pure and file-local (`rel`, `source`, `lang` → `{nodes, * rawEdges}`), which is the whole trick: an unchanged file's parse result can be @@ -28,12 +28,12 @@ import { createHash } from "node:crypto"; import { fileURLToPath } from "node:url"; import { dirname, extname, join } from "node:path"; import { CACHE_DIR } from "../context/node-file.js"; -import { readJson, writeJsonAtomic } from "../util/state.js"; +import { forEachLine, openAtomic, type AtomicWriter } from "../util/json-stream.js"; import type { RawEdge } from "./extract.js"; import type { NodeV1 } from "./types.js"; /** Bump when the on-disk shape below changes. */ -const CACHE_VERSION = 1; +const CACHE_VERSION = 2; export const EXTRACT_CACHE_PREFIX = "extract"; export interface ExtractEntry { @@ -59,7 +59,7 @@ export interface ExtractCache { } /** - * Where this graft's memo lives: `/.cache/extract..json`. + * Where this graft's memo lives: `/.cache/extract..ndjson`. * * The stamp is in the *filename*, not just inside the file, so two grafts working * on one repo keep separate memos instead of evicting each other. That is the @@ -74,7 +74,7 @@ export interface ExtractCache { */ export function extractCachePath(outDir: string): string | null { const stamp = extractorStamp(); - return stamp === null ? null : join(outDir, CACHE_DIR, `${EXTRACT_CACHE_PREFIX}.${stamp}.json`); + return stamp === null ? null : join(outDir, CACHE_DIR, `${EXTRACT_CACHE_PREFIX}.${stamp}.ndjson`); } /** Keep `.cache/` from growing a file per version forever: after writing, drop all @@ -83,7 +83,7 @@ export function extractCachePath(outDir: string): string | null { export function pruneSidecars(cacheDir: string, prefix: string, keep = 2): void { try { const mine = readdirSync(cacheDir) - .filter((f) => f.startsWith(`${prefix}.`) && f.endsWith(".json")) + .filter((f) => f.startsWith(`${prefix}.`) && (f.endsWith(".json") || f.endsWith(".ndjson"))) .map((f) => { const full = join(cacheDir, f); return { full, mtimeMs: statSync(full).mtimeMs }; @@ -199,25 +199,61 @@ export function readExtractCache(outDir: string): ExtractCache { const path = extractCachePath(outDir); const stamp = extractorStamp(); if (path === null || stamp === null) return emptyExtractCache(); - const c = readJson(path); - if (!c || c.version !== CACHE_VERSION || c.extractor !== stamp || typeof c.files !== "object") { - return emptyExtractCache(); + let buf: Buffer; + try { buf = readFileSync(path); } catch { return emptyExtractCache(); } + // Decoded a line at a time: the file may be larger than any string V8 holds. + const files: Record = {}; + let ok = true; + try { + forEachLine(buf, (line, i) => { + if (!ok || line === "") return; + const parsed = JSON.parse(line) as { version?: number; extractor?: string; rel?: string; e?: ExtractEntry }; + if (i === 0) { + if (parsed.version !== CACHE_VERSION || parsed.extractor !== stamp) ok = false; + return; + } + if (typeof parsed.rel !== "string" || !parsed.e || typeof parsed.e !== "object") { ok = false; return; } + files[parsed.rel] = parsed.e; + }); + } catch { + ok = false; // a corrupt line: the whole memo is suspect } - return { version: c.version, extractor: c.extractor, files: c.files ?? {} }; + return ok ? { version: CACHE_VERSION, extractor: stamp, files } : emptyExtractCache(); } /** Best-effort write — a full graph is already on disk by the time this runs, so an * unwritable cache dir must never fail the build (it only costs the next build its * reuse). Returns false when nothing was written, including the deliberate case of - * having no extractor identity: a parse we can't attribute must never be replayed. */ + * having no extractor identity: a parse we can't attribute must never be replayed. + * + * NDJSON: a header line, then one line per entry. Each element is stringified on its + * own, so the writer never builds a string larger than one entry, and the reader — via + * {@link forEachLine} — never decodes more than one line at a time. The atomic rename + * means a half-written file is never visible under `path`, so it can never be read as a + * valid (but truncated) cache. */ export function writeExtractCache(outDir: string, cache: ExtractCache): boolean { const path = extractCachePath(outDir); if (path === null) return false; + let w: AtomicWriter | null = null; try { - writeJsonAtomic(path, cache, true); + w = openAtomic(path); + w.write(`${JSON.stringify({ version: CACHE_VERSION, extractor: cache.extractor })}\n`); + for (const rel of Object.keys(cache.files)) { + w.write(`${JSON.stringify({ rel, e: cache.files[rel] })}\n`); + } + w.commit(); pruneSidecars(join(outDir, CACHE_DIR), EXTRACT_CACHE_PREFIX); return true; } catch { + w?.abort(); return false; } } + +/** Test seam: read, mutate, rewrite. Tests used to JSON.parse the cache file + * directly; the format is now line-oriented and this keeps them format-agnostic. */ +export function rewriteExtractCacheForTest(outDir: string, mutate: (files: Record) => void): void { + const c = readExtractCache(outDir); + mutate(c.files); + writeExtractCache(outDir, c); +} diff --git a/src/graph/write.ts b/src/graph/write.ts index ede60bb60..5f4c27f57 100644 --- a/src/graph/write.ts +++ b/src/graph/write.ts @@ -8,8 +8,10 @@ * timestamps, so rebuilding an unchanged repo produces a byte-identical file and * git diffs stay minimal. */ -import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { constants as bufferConstants } from "node:buffer"; +import { readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { forEachLine, openAtomic } from "../util/json-stream.js"; import type { EdgeV1, GraphV1, NodeV1 } from "./types.js"; /** Hidden subdir under the context dir that holds machine-only graph artifacts. */ @@ -21,11 +23,21 @@ export function wiringPath(outDir: string): string { return join(outDir, GRAPH_DIR, GRAPH_FILE); } +export interface ReadGraphOptions { maxStringLength?: number } + /** * Read an existing wiring graph for use as the Tier-2 cache. Returns null when the * file is absent or unparseable (a fresh build, or a corrupt file we'll replace). + * A file over V8's string cap is walked line by line ({@link readGraphFromBuffer}) + * instead of read as one string; `opts.maxStringLength` lets a test force that path. */ -export function readGraph(path: string): GraphV1 | null { +export function readGraph(path: string, opts: ReadGraphOptions = {}): GraphV1 | null { + let size: number; + try { size = statSync(path).size; } catch { return null; } + const cap = opts.maxStringLength ?? bufferConstants.MAX_STRING_LENGTH; + if (size > cap) { + try { return readGraphFromBuffer(readFileSync(path)); } catch { return null; } + } try { return JSON.parse(readFileSync(path, "utf8")) as GraphV1; } catch { @@ -33,34 +45,64 @@ export function readGraph(path: string): GraphV1 | null { } } +/** Parse the line-per-element shape `writeGraph` produces without ever holding + * the whole file as one string. Null when the buffer is not in that shape. */ +export function readGraphFromBuffer(buf: Buffer): GraphV1 | null { + const NODES_OPEN = ',"nodes":['; + const EDGES_OPEN = '],"edges":['; + const END = "]}"; + let meta: GraphV1["meta"] | null = null; + const nodes: NodeV1[] = []; + const edges: EdgeV1[] = []; + let target: NodeV1[] | EdgeV1[] | null = null; + let closed = false; + let bad = false; + forEachLine(buf, (line, i) => { + if (bad || closed) { if (line !== "") bad = true; return; } + try { + if (i === 0) { + if (!line.startsWith('{"meta":') || !line.endsWith(NODES_OPEN)) { bad = true; return; } + meta = JSON.parse(line.slice('{"meta":'.length, line.length - NODES_OPEN.length)) as GraphV1["meta"]; + target = nodes; + return; + } + if (line === EDGES_OPEN) { target = edges; return; } + if (line === END) { closed = true; return; } + if (target === null) { bad = true; return; } + const body = line.endsWith(",") ? line.slice(0, -1) : line; + (target as unknown[]).push(JSON.parse(body)); + } catch { + bad = true; + } + }); + if (bad || !closed || meta === null) return null; + return { meta, nodes, edges }; +} + export function writeGraph(graph: GraphV1, outDir: string): string { - const sorted: GraphV1 = { - ...graph, - nodes: [...graph.nodes].sort((a, b) => a.id.localeCompare(b.id)).map(stripBodyText), - edges: [...graph.edges].sort(edgeOrder), - }; + const nodes = [...graph.nodes].sort((a, b) => a.id.localeCompare(b.id)); + const edges = [...graph.edges].sort(edgeOrder); const path = wiringPath(outDir); - mkdirSync(dirname(path), { recursive: true }); - // Atomic write (temp + rename): the crux pass now checkpoints wiring.json - // periodically (#128), and a --deep run is exactly what gets killed mid-flush - // (SIGTERM/CI timeout/laptop sleep). A partial writeFileSync would leave a - // truncated, unparseable graph; rename swaps it in atomically on the same fs. - // - // pid in the temp name, and removed when the write fails — the same discipline - // `writeJsonAtomic` (util/state.ts) documents: a fixed name lets a concurrent - // build (a manual `graft build` racing the refresh child) write the same scratch - // file and hand the loser a corrupt graph, and a failed rename would otherwise - // leave a full-size orphan behind that nothing ever cleans up. - const tmp = `${path}.${process.pid}.tmp`; + // Streamed, one element per line: still valid JSON for every JSON.parse reader + // (viz/serve, the viewer, fixtures, old grafts), but a newline at each element + // boundary lets readGraphFromBuffer walk a file over V8's string cap without + // ever holding it as one string. Compact within a line: pretty-printing was + // ~30% of the bytes for a file only machines read (graft/ is gitignored by + // default), and on a 65k-file repo it was the difference between a wiring.json + // a query can load and one over the cap. + const w = openAtomic(path); try { - writeFileSync(tmp, JSON.stringify(sorted, null, 2) + "\n"); - renameSync(tmp, path); + const { nodes: _n, edges: _e, ...rest } = graph; + const head = JSON.stringify({ ...rest, nodes: [], edges: [] }); // ends with `,"nodes":[],"edges":[]}` + w.write(head.slice(0, head.length - '"nodes":[],"edges":[]}'.length)); + w.write('"nodes":['); + nodes.forEach((n, i) => w.write("\n" + JSON.stringify(stripBodyText(n)) + (i < nodes.length - 1 ? "," : ""))); + w.write('\n],"edges":['); + edges.forEach((e, i) => w.write("\n" + JSON.stringify(e) + (i < edges.length - 1 ? "," : ""))); + w.write("\n]}\n"); + w.commit(); } catch (e) { - try { - rmSync(tmp, { force: true }); - } catch { - /* nothing more we can do */ - } + w.abort(); throw e; } return path; diff --git a/src/util/json-stream.ts b/src/util/json-stream.ts new file mode 100644 index 000000000..c30f15a8e --- /dev/null +++ b/src/util/json-stream.ts @@ -0,0 +1,67 @@ +/** + * Streamed, atomic file writes and line iteration over a Buffer. + * + * Every graft artifact used to be one `JSON.stringify` and one `JSON.parse`. + * V8 caps a string at ~512 MiB (`buffer.constants.MAX_STRING_LENGTH`); a + * 65k-file repo's extract cache is ~1 GB and its wiring.json ~0.5 GB, so the + * cold build threw `Invalid string length` at the very end, and a graph that + * did get written threw `ERR_STRING_TOO_LONG` on read. The writers here take + * one element at a time; the reader decodes one line at a time. + */ +import { closeSync, mkdirSync, openSync, renameSync, rmSync, writeSync } from "node:fs"; +import { dirname } from "node:path"; + +export interface AtomicWriter { + write(chunk: string): void; + commit(): void; + abort(): void; +} + +/** Write to `..tmp`, then rename over `path` on commit(). The pid keeps + * two concurrent writers off each other's scratch file, the same discipline as + * `writeJsonAtomic` in util/state.ts. */ +export function openAtomic(path: string): AtomicWriter { + mkdirSync(dirname(path), { recursive: true }); + const tmp = `${path}.${process.pid}.tmp`; + let fd: number | null = openSync(tmp, "w"); + const cleanup = (): void => { + if (fd !== null) { try { closeSync(fd); } catch { /* already closed */ } fd = null; } + try { rmSync(tmp, { force: true }); } catch { /* nothing more to do */ } + }; + return { + write(chunk) { + if (fd === null) throw new Error("atomic writer is closed"); + // writeSync may write fewer bytes than asked (pipes, some filesystems): + // loop on the byte count, so a multi-megabyte element is never truncated. + const bytes = Buffer.from(chunk, "utf8"); + let off = 0; + while (off < bytes.length) off += writeSync(fd, bytes, off, bytes.length - off); + }, + commit() { + if (fd === null) throw new Error("atomic writer is closed"); + try { + closeSync(fd); + fd = null; + renameSync(tmp, path); + } catch (e) { + cleanup(); + throw e; + } + }, + abort() { cleanup(); }, + }; +} + +/** Call `fn` for each line of `buf`. A trailing line without "\n" is still a line; + * an empty buffer yields none. Each line is decoded on its own, so the Buffer may + * be larger than any string V8 can hold. */ +export function forEachLine(buf: Buffer, fn: (line: string, index: number) => void): void { + let start = 0; + let index = 0; + while (start < buf.length) { + let end = buf.indexOf(0x0a, start); + if (end === -1) end = buf.length; + fn(buf.toString("utf8", start, end), index++); + start = end + 1; + } +} diff --git a/test/graph-artifacts-stream.test.ts b/test/graph-artifacts-stream.test.ts new file mode 100644 index 000000000..046ac02e0 --- /dev/null +++ b/test/graph-artifacts-stream.test.ts @@ -0,0 +1,134 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { emptyExtractCache, extractCachePath, readExtractCache, writeExtractCache, type ExtractEntry } from "../src/graph/extract-cache.js"; + +const FILES: Record = { + "src/a.ts": { size: 3, mtimeMs: 1, hash: "h1", nodes: [], rawEdges: [] }, + "src/b.rs": { size: 4, mtimeMs: 2, hash: "", nodes: [], rawEdges: [], error: "src/b.rs: nope" }, + "z/\"quoted\"\n.c": { size: 5, mtimeMs: 3, hash: "h3", nodes: [], rawEdges: [{ source: "z", relation: "calls", target: "q" } as never] }, +}; + +test("extract cache round-trips through NDJSON, including quotes and newlines in a path", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + assert.ok(writeExtractCache(d, { ...emptyExtractCache(), files: FILES })); + assert.ok(extractCachePath(d)!.endsWith(".ndjson")); + const back = readExtractCache(d); + assert.deepEqual(back.files, FILES); + assert.equal(back.version, 2); +}); + +test("extract cache: one line per entry, header first", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + writeExtractCache(d, { ...emptyExtractCache(), files: FILES }); + const lines = readFileSync(extractCachePath(d)!, "utf8").split("\n"); + assert.equal(lines[lines.length - 1], "", "trailing newline"); + assert.equal(lines.length - 1, 1 + Object.keys(FILES).length); + assert.deepEqual(JSON.parse(lines[0]), { version: 2, extractor: emptyExtractCache().extractor }); +}); + +test("extract cache: a wrong version, a wrong stamp, or a corrupt line means an empty cache, never a throw", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + writeExtractCache(d, { ...emptyExtractCache(), files: FILES }); + const p = extractCachePath(d)!; + writeFileSync(p, readFileSync(p, "utf8").replace('"version":2', '"version":1')); + assert.deepEqual(readExtractCache(d).files, {}); + writeExtractCache(d, { ...emptyExtractCache(), files: FILES }); + writeFileSync(p, readFileSync(p, "utf8") + "{not json\n"); + assert.deepEqual(readExtractCache(d).files, {}, "a corrupt line discards the cache: a partial memo is worse than none"); +}); + +test("extract cache: an empty file set still writes a valid header the reader accepts", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + assert.ok(writeExtractCache(d, emptyExtractCache())); + assert.deepEqual(readExtractCache(d).files, {}); +}); + +import { readGraph, wiringPath, writeGraph } from "../src/graph/write.js"; +import { askIndexPath, readAskIndex, writeAskIndex } from "../src/ask/index-file.js"; +import type { GraphV1, NodeV1 } from "../src/graph/types.js"; + +function node(id: string, extra: Partial = {}): NodeV1 { + return { id, name: id, kind: "function", path: "a.ts", span: "L1-L2", signature: null, exported: true, origin: "ast", body_hash: "h", chars: 10, summary_state: "pending", summary: null, crux: null, ...extra }; +} +const GRAPH: GraphV1 = { + meta: { version: 1, nodeCount: 2, edgeCount: 1, languages: ["typescript"], scopes: [{ prefix: "", label: "", markers: [] }] }, + nodes: [node("b", { body_text: "dropped on disk" }), node("a")], + edges: [{ source: "b", relation: "calls", target: "a" } as never], +}; + +test("writeGraph streams a compact document equal to JSON.stringify of the sorted, body-stripped graph", () => { + const d = mkdtempSync(join(tmpdir(), "graft-wiring-")); + writeGraph(GRAPH, d); + const text = readFileSync(wiringPath(d), "utf8"); + const expected = JSON.stringify({ + ...GRAPH, + nodes: [...GRAPH.nodes].sort((x, y) => x.id.localeCompare(y.id)).map(({ body_text: _b, ...rest }) => rest), + edges: GRAPH.edges, + }) + "\n"; + assert.deepEqual(JSON.parse(text), JSON.parse(expected)); + assert.deepEqual(readGraph(wiringPath(d)), JSON.parse(expected)); +}); + +test("writeGraph with no nodes and no edges is still valid", () => { + const d = mkdtempSync(join(tmpdir(), "graft-wiring-")); + writeGraph({ ...GRAPH, nodes: [], edges: [] }, d); + assert.deepEqual(readGraph(wiringPath(d))!.nodes, []); +}); + +test("writeAskIndex streams a document equal to the one-shot JSON.stringify", () => { + const d = mkdtempSync(join(tmpdir(), "graft-askidx-")); + writeAskIndex(d, GRAPH); + const text = readFileSync(askIndexPath(d), "utf8"); + const back = readAskIndex(d)!; + assert.deepEqual(JSON.parse(text), back); + assert.equal(back.docCount, 2); +}); + +import { readGraphFromBuffer } from "../src/graph/write.js"; +import { readAskIndexFromBuffer } from "../src/ask/index-file.js"; + +/** JSON.stringify of the graph with a newline at every node/edge boundary. */ +function lineShaped(g: GraphV1): string { + const nodes = [...g.nodes].sort((x, y) => x.id.localeCompare(y.id)).map(({ body_text: _b, ...rest }) => rest); + const edges = [...g.edges].sort((a, b) => a.source.localeCompare(b.source) || a.relation.localeCompare(b.relation) || a.target.localeCompare(b.target)); + return `{"meta":${JSON.stringify(g.meta)},"nodes":[\n${nodes.map((n) => JSON.stringify(n)).join(",\n")}\n],"edges":[\n${edges.map((e) => JSON.stringify(e)).join(",\n")}\n]}\n`; +} + +test("writeGraph writes one element per line and the file is still valid JSON", () => { + const d = mkdtempSync(join(tmpdir(), "graft-wiring-lines-")); + writeGraph(GRAPH, d); + const text = readFileSync(wiringPath(d), "utf8"); + assert.equal(text, lineShaped(GRAPH)); + assert.deepEqual(JSON.parse(text), JSON.parse(lineShaped(GRAPH))); + assert.deepEqual(readGraph(wiringPath(d)), JSON.parse(text)); +}); + +test("readGraph streams a file over the cap and returns the same graph as JSON.parse", () => { + const d = mkdtempSync(join(tmpdir(), "graft-wiring-stream-")); + writeGraph(GRAPH, d); + const text = readFileSync(wiringPath(d), "utf8"); + assert.deepEqual(readGraph(wiringPath(d), { maxStringLength: 16 }), JSON.parse(text), "forced streaming path"); + assert.deepEqual(readGraphFromBuffer(readFileSync(wiringPath(d))), JSON.parse(text)); + const empty = { ...GRAPH, nodes: [], edges: [] }; + writeGraph(empty, d); + assert.deepEqual(readGraph(wiringPath(d), { maxStringLength: 16 }), JSON.parse(readFileSync(wiringPath(d), "utf8"))); +}); + +test("readGraph streaming returns null on a file that is not line-shaped", () => { + assert.equal(readGraphFromBuffer(Buffer.from('{"meta":{},"nodes":[{"id":"a"}],"edges":[]}\n')), null, "one-line JSON is not the streamed shape"); + assert.equal(readGraphFromBuffer(Buffer.from("garbage\n")), null); +}); + +test("writeAskIndex writes one doc per line, still valid JSON, and streams back identically", () => { + const d = mkdtempSync(join(tmpdir(), "graft-askidx-lines-")); + writeAskIndex(d, GRAPH); + const text = readFileSync(askIndexPath(d), "utf8"); + const parsed = JSON.parse(text); + assert.equal(text.split("\n").length - 1, 1 + parsed.docs.length + 1, "header line, one line per doc, closing line"); + assert.deepEqual(readAskIndex(d), parsed); + assert.deepEqual(readAskIndex(d, { maxStringLength: 16 }), parsed, "forced streaming path"); + assert.deepEqual(readAskIndexFromBuffer(readFileSync(askIndexPath(d))), parsed); +}); diff --git a/test/graph-incremental.test.ts b/test/graph-incremental.test.ts index 57e5fc224..0a814d648 100644 --- a/test/graph-incremental.test.ts +++ b/test/graph-incremental.test.ts @@ -11,7 +11,7 @@ import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, readdirSyn import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { buildGraph } from "../src/graph/build.js"; -import { extractCachePath, extractorStamp, readExtractCache, stampDir } from "../src/graph/extract-cache.js"; +import { extractCachePath, extractorStamp, readExtractCache, rewriteExtractCacheForTest, stampDir, writeExtractCache } from "../src/graph/extract-cache.js"; import { fingerprintPath, isClean, probeDrift, readFingerprint } from "../src/graph/fingerprint.js"; import { readAskIndex } from "../src/ask/index-file.js"; import { readGraph, wiringPath } from "../src/graph/write.js"; @@ -356,11 +356,12 @@ test("the stamp moves for any module in the extractor directory, not just one", test("a memo written by a different extractor is dropped, not replayed", async () => { const d = repo(); await buildGraph(d); - const path = extractCachePath(outOf(d)); - const cache = JSON.parse(readFileSync(path, "utf8")); + const cache = readExtractCache(outOf(d)); assert.ok(Object.keys(cache.files).length > 0); - writeFileSync(path, JSON.stringify({ ...cache, extractor: "deadbeefdeadbeef" })); + // The header carries the extractor identity; rewriting it with a foreign stamp + // is a memo produced by a different extractor, keyed under our own filename. + writeExtractCache(outOf(d), { ...cache, extractor: "deadbeefdeadbeef" }); assert.deepEqual(readExtractCache(outOf(d)).files, {}, "entries from an unknown extractor are worthless"); // So the next build re-parses everything rather than trusting them. @@ -456,14 +457,11 @@ test("a build repairs an edit that leaves size and mtime untouched", async () => // pre-edit hash and nodes. That is precisely the state a 1s-granularity mount hands // you for free, and it avoids `utimesSync`, whose float-seconds round-trip can't // reproduce an mtimeMs exactly. - const memoPath = extractCachePath(outOf(d))!; - const memo = JSON.parse(readFileSync(memoPath, "utf8")) as { - files: Record; - }; const now = statSync(file); - memo.files["src/math.ts"].size = now.size; - memo.files["src/math.ts"].mtimeMs = now.mtimeMs; - writeFileSync(memoPath, JSON.stringify(memo)); + rewriteExtractCacheForTest(outOf(d), (files) => { + files["src/math.ts"].size = now.size; + files["src/math.ts"].mtimeMs = now.mtimeMs; + }); const r = await buildGraph(d); assert.equal(r.parsed, 1, "the file was re-parsed despite the identical stat"); diff --git a/test/graph-refresh.test.ts b/test/graph-refresh.test.ts index 60f86623c..911c6cdba 100644 --- a/test/graph-refresh.test.ts +++ b/test/graph-refresh.test.ts @@ -11,7 +11,7 @@ import { spawn } from "node:child_process"; import { dirname, join } from "node:path"; import { buildGraph } from "../src/graph/build.js"; import { ensureFreshChildren, ensureFreshGraph, refreshNote } from "../src/graph/refresh.js"; -import { extractCachePath } from "../src/graph/extract-cache.js"; +import { extractCachePath, rewriteExtractCacheForTest } from "../src/graph/extract-cache.js"; import { fingerprintPath, isClean, probeDrift } from "../src/graph/fingerprint.js"; import { readGraph, wiringPath } from "../src/graph/write.js"; import { acquireLock, readStats, releaseLock, writeStats, emptyStats } from "../src/util/state.js"; @@ -228,9 +228,9 @@ test("GRAFT_REFRESH=hash: drift the probe reports is drift the rebuild repairs", const staleHash = fp.files["src/math.ts"][2]; fp.files["src/math.ts"] = [now.size, now.mtimeMs, staleHash]; writeFileSync(fingerprintPath(outOf(d)), JSON.stringify(fp)); - const memo = JSON.parse(readFileSync(extractCachePath(outOf(d)), "utf8")); - Object.assign(memo.files["src/math.ts"], { size: now.size, mtimeMs: now.mtimeMs }); - writeFileSync(extractCachePath(outOf(d)), JSON.stringify(memo)); + rewriteExtractCacheForTest(outOf(d), (files) => { + Object.assign(files["src/math.ts"], { size: now.size, mtimeMs: now.mtimeMs }); + }); // Nobody notices without the flag — that's the documented trade-off, and the // whole reason the flag exists. diff --git a/test/json-stream.test.ts b/test/json-stream.test.ts new file mode 100644 index 000000000..3f700a211 --- /dev/null +++ b/test/json-stream.test.ts @@ -0,0 +1,49 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, readdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { forEachLine, openAtomic } from "../src/util/json-stream.js"; + +test("openAtomic: nothing is visible at the target until commit, and the temp file is gone after", () => { + const d = mkdtempSync(join(tmpdir(), "graft-atomic-")); + const p = join(d, "sub", "out.json"); + const w = openAtomic(p); + w.write("{\"a\":"); + assert.ok(!existsSync(p), "target absent before commit"); + w.write("1}\n"); + w.commit(); + assert.equal(readFileSync(p, "utf8"), "{\"a\":1}\n"); + assert.deepEqual(readdirSync(join(d, "sub")), ["out.json"], "no temp file left behind"); +}); + +test("openAtomic: abort leaves no file", () => { + const d = mkdtempSync(join(tmpdir(), "graft-atomic-")); + const p = join(d, "out.json"); + const w = openAtomic(p); + w.write("partial"); + w.abort(); + assert.deepEqual(readdirSync(d), []); +}); + +test("openAtomic: a multi-megabyte chunk is written whole (short-write loop)", () => { + const d = mkdtempSync(join(tmpdir(), "graft-atomic-")); + const p = join(d, "big.txt"); + const chunk = "x".repeat(8 * 1024 * 1024) + "é\n"; // multibyte tail catches byte/char confusion + const w = openAtomic(p); + w.write(chunk); + w.commit(); + assert.equal(readFileSync(p, "utf8"), chunk); +}); + +test("forEachLine: splits on newline, handles a missing trailing newline, and decodes UTF-8 per line", () => { + const seen: [string, number][] = []; + forEachLine(Buffer.from("a\n{\"k\":\"é\"}\n\nlast"), (l, i) => seen.push([l, i])); + assert.deepEqual(seen, [["a", 0], ["{\"k\":\"é\"}", 1], ["", 2], ["last", 3]]); +}); + +test("forEachLine: an empty buffer yields nothing", () => { + let n = 0; + forEachLine(Buffer.alloc(0), () => n++); + assert.equal(n, 0); +}); From 516059ffa86ad3bb840aba4f26f1294c75101ebe Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:34:31 -0600 Subject: [PATCH 6/8] perf(build): read sources lazily for the meaning pass; lift extractOne buildGraph kept every file's source text for the whole build, about 1 GB of strings on a 65k-file repo, for a pass a Tier-1 build never runs. The meaning pass now reads a file when it summarizes it, and only if its bytes still hash to what was parsed. The per-file read/hash/extract step moves into extractOne so the in-thread loop and a parser child run the same code. --- src/graph/build.ts | 145 +++++++++++++++------------------- src/graph/enrich.ts | 25 +++++- src/graph/extract-one.ts | 64 +++++++++++++++ test/graph-parse-pool.test.ts | 95 ++++++++++++++++++++++ 4 files changed, 247 insertions(+), 82 deletions(-) create mode 100644 src/graph/extract-one.ts create mode 100644 test/graph-parse-pool.test.ts diff --git a/src/graph/build.ts b/src/graph/build.ts index cf316d793..126fd2228 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -17,9 +17,9 @@ import { readFileSync } from "node:fs"; import { basename, dirname, resolve } from "node:path"; import { walkDir } from "../ingest/fs.js"; import { contextDirFor, ensureGitignored, ensureSearchable } from "../context/node-file.js"; -import { extractFile, languageLabelOf, languageOf, type RawEdge } from "./extract.js"; -import { extractGeneric, genericLangOf, warmGenericGrammars } from "./generic.js"; -import { containerLangOf, extractContainer, warmContainerGrammars } from "./container.js"; +import { languageLabelOf, type RawEdge } from "./extract.js"; +import { genericLangOf, warmGenericGrammars } from "./generic.js"; +import { containerLangOf, warmContainerGrammars } from "./container.js"; import { contentHash } from "../util/id.js"; import { relPosix } from "../util/paths.js"; import { readSourceFile } from "../util/source.js"; @@ -33,8 +33,10 @@ import { import { writeFingerprint } from "./fingerprint.js"; import { seedGraph, type SeedResult } from "./seed.js"; import { filterByOnlyDirs, listSourceStats } from "./source-files.js"; +import { extractOne, type ExtractOneResult } from "./extract-one.js"; +import type { SourceStat } from "./source-files.js"; import { resolveEdges, type GoModule } from "./resolve.js"; -import { enrichGraph, type EnrichStats } from "./enrich.js"; +import { enrichGraph, type EnrichStats, type SourceLookup } from "./enrich.js"; import { readGraph, writeGraph, wiringPath } from "./write.js"; import { writeCards, writeIndex, writeCovers, type CardStats } from "./cards.js"; import { writeAskIndex } from "../ask/index-file.js"; @@ -168,7 +170,6 @@ export async function buildGraph( const nodes: NodeV1[] = []; const rawEdges: RawEdge[] = []; - const sources = new Map(); /** Display labels, not grammars — `.mjs` is parsed as typescript but reported as * javascript, or the banner claims a repo's JavaScript went unindexed. */ const langs = new Set(); @@ -202,84 +203,47 @@ export async function buildGraph( new Set(files.map((f) => containerLangOf(f.abs)?.name).filter((n): n is string => !!n)), ); - files.forEach((f, i) => { + const labelOf = (abs: string): string => languageLabelOf(abs) ?? containerLangOf(abs)?.name ?? genericLangOf(abs)?.name ?? "unknown"; + const applyResult = (f: SourceStat, r: ExtractOneResult): void => { const rel = f.rel; - opts.onProgress?.({ phase: "parse", index: i, total: files.length, file: rel }); - // Depth tier (hand-written, native grammar) if a language claims the file; - // otherwise the breadth tier (generic tags.scm over a WASM grammar). - const lang = languageOf(f.abs); - // A container is neither tier: its wrapper grammar only locates the embedded - // block, which then goes to the depth-tier extractor. Checked before the - // breadth tier so a future grammar claiming .vue can't shadow it. - const container = lang ? null : containerLangOf(f.abs); - const generic = lang || container ? null : genericLangOf(f.abs); - const label = languageLabelOf(f.abs) ?? container?.name ?? generic?.name ?? "unknown"; - const cached = priorExtract.files[rel]; - - // Every file is read and hashed, every build — only the *parse* is memoized. - // The tempting optimization is to trust the probe's `(size, mtimeMs)` and skip - // the read too, but then `graft build` inherits the probe's blind spot: on a - // filesystem with coarse mtime granularity, a same-length edit inside the same - // second is invisible, so `graft check` reports drift (it always re-hashes) and - // the `graft build` it tells you to run refuses to repair it — forever. A stat - // may decide whether a *query* bothers rebuilding; it may not decide what the - // rebuild itself looks at. Reading is ~0.05ms/file against the ~4.6ms parse - // this still skips. - let source: string | null; - try { - source = readSourceFile(f.abs); - } catch (err) { - const message = `${rel}: ${err instanceof Error ? err.message : String(err)}`; - errors.push(message); - // Record it anyway (with the stat we do have) so the freshness probe's - // fast path doesn't report this file as new on every single query. - entries[rel] = { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [], error: message }; - return; - } - if (source === null) { - // Unsupported encoding (UTF-16BE) — a skip, never an error: recorded with - // an empty entry so the freshness probe doesn't treat it as new every run. - entries[rel] = { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [] }; - return; - } - - const hash = contentHash(source); - // A cached FAILURE is never replayed. Most parse failures are not properties - // of the file: a WASM grammar that aborted because the heap was exhausted by - // the files before it fails again only if the heap is exhausted again. Replaying - // the error would pin it to the file until its bytes changed; on a 65k-file - // repo that was 52k files stuck failing after a single abort. Re-parsing a - // genuinely broken file costs one parse per build, which is nothing. (The - // pre-query refresh never rebuilds solely to retry an errored file: - // fingerprint.ts keeps the entry's real hash on its stat fast path. A rebuild - // triggered by other drift re-parses it like any build.) - if (cached && hash === cached.hash && !cached.error) { - entries[rel] = { ...cached, size: f.size, mtimeMs: f.mtimeMs }; - sources.set(rel, source); - reused++; - nodes.push(...cached.nodes); - rawEdges.push(...cached.rawEdges); - langs.add(label); - return; + switch (r.kind) { + case "reused": { + const cached = priorExtract.files[rel]!; // reused is only reported against a hash we supplied + entries[rel] = { ...cached, size: f.size, mtimeMs: f.mtimeMs }; + reused++; + nodes.push(...cached.nodes); + rawEdges.push(...cached.rawEdges); + langs.add(labelOf(f.abs)); + return; + } + case "skipped": + entries[rel] = r.entry; + if (r.entry.error) errors.push(r.entry.error); + return; + case "parsed": + parsed++; + entries[rel] = r.entry; + nodes.push(...r.entry.nodes); + rawEdges.push(...r.entry.rawEdges); + langs.add(r.label); + return; + case "error": + parsed++; + entries[rel] = r.entry; + errors.push(r.message); + return; } + }; + // A cached FAILURE is never offered for reuse (PR 1): its hash is withheld so + // extractOne parses the file again. + const cachedHashOf = (rel: string): string | null => { + const c = priorExtract.files[rel]; + return c && !c.error ? c.hash : null; + }; - parsed++; - try { - const { nodes: fileNodes, rawEdges: fileEdges } = lang - ? extractFile(rel, source, lang) - : container - ? extractContainer(rel, source, container) - : extractGeneric(rel, source, generic!.name); - nodes.push(...fileNodes); - rawEdges.push(...fileEdges); - sources.set(rel, source); - langs.add(label); - entries[rel] = { size: f.size, mtimeMs: f.mtimeMs, hash, nodes: fileNodes, rawEdges: fileEdges }; - } catch (err) { - const message = `${rel}: parse failed — ${err instanceof Error ? err.message : String(err)}`; - errors.push(message); - entries[rel] = { size: f.size, mtimeMs: f.mtimeMs, hash, nodes: [], rawEdges: [], error: message }; - } + files.forEach((f, i) => { + opts.onProgress?.({ phase: "parse", index: i, total: files.length, file: f.rel }); + applyResult(f, extractOne(f, cachedHashOf(f.rel))); }); // Persist the memo BEFORE enrichment, because `enrichGraph` mutates these very @@ -320,6 +284,27 @@ export async function buildGraph( // Read BEFORE the first checkpoint can overwrite wiring.json. const prior = readGraph(wiringPath(outDir)); const priorById = new Map((prior?.nodes ?? []).map((n) => [n.id, n])); + // The meaning pass reads a file only when it summarizes it, and only if the + // bytes still hash to what was parsed. Holding every source for the whole build + // was ~1 GB of strings on a 65k-file repo, for a pass a Tier-1 build never runs. + const absOf = new Map(files.map((f) => [f.rel, f.abs] as const)); + const sources: SourceLookup = { + has: (rel) => { + const e = entries[rel]; + return e !== undefined && e.hash !== "" && !e.error; + }, + get: (rel) => { + const abs = absOf.get(rel); + const e = entries[rel]; + if (!abs || !e) return undefined; + try { + const text = readSourceFile(abs); + return text !== null && contentHash(text) === e.hash ? text : undefined; + } catch { + return undefined; + } + }, + }; const meaning = await enrichGraph(nodes, priorById, sources, { summarizer: opts.summarizer, concurrency: opts.concurrency, diff --git a/src/graph/enrich.ts b/src/graph/enrich.ts index f18a3b2ce..c4dfbd368 100644 --- a/src/graph/enrich.ts +++ b/src/graph/enrich.ts @@ -71,10 +71,18 @@ export interface EnrichStats { fatal?: string; } +/** Where the meaning pass gets a file's text. `Map` satisfies it, + * which is what tests pass; `buildGraph` passes a lazy, hash-checked reader so a + * 65k-file build does not hold every source for a pass that touches a handful. */ +export interface SourceLookup { + has(path: string): boolean; + get(path: string): string | undefined; +} + export async function enrichGraph( nodes: NodeV1[], prior: Map, - sources: Map, + sources: SourceLookup, opts: EnrichOptions = {}, ): Promise { const stats: EnrichStats = { @@ -144,7 +152,20 @@ export async function enrichGraph( await mapWithConcurrency(files, limit, async (path) => { const fileNodes = byFile.get(path)!; - const source = sources.get(path)!; + const source = sources.get(path); + if (source === undefined) { + // The file changed or vanished between parse and summary: a crux sliced + // against different bytes would point at the wrong lines. Leave the nodes + // pending; the next build re-hashes and settles them. This runs before the + // gate.stopped check and, unlike the fatal-abort branch below, is not a + // provider failure, so it does not call gate.skip(). + for (const node of fileNodes) { + if (node.summary_state === "stale") stats.stale++; + else stats.pending++; + } + opts.onProgress?.({ index: done++, total: files.length, node: path }); + return; + } const lineCount = source.split("\n").length; // Once the pass is fatal, the remaining files are not attempted: every call diff --git a/src/graph/extract-one.ts b/src/graph/extract-one.ts new file mode 100644 index 000000000..f15a9d152 --- /dev/null +++ b/src/graph/extract-one.ts @@ -0,0 +1,64 @@ +/** + * The per-file half of `buildGraph`'s parse loop: read, hash, pick a tier, + * extract. Pure apart from the read, and it never touches the cache or the graph: + * the caller decides what to do with each outcome. Kept separate so the same + * code runs in-thread for a small refresh and inside a forked parser child for + * a large cold build (`parse-worker.ts`), and the two cannot drift. + */ +import { extractFile, languageLabelOf, languageOf } from "./extract.js"; +import { extractGeneric, genericLangOf } from "./generic.js"; +import { containerLangOf, extractContainer } from "./container.js"; +import { contentHash } from "../util/id.js"; +import { readSourceFile } from "../util/source.js"; +import type { ExtractEntry } from "./extract-cache.js"; +import type { SourceStat } from "./source-files.js"; + +export type ExtractOneResult = + | { kind: "reused"; hash: string } + | { kind: "skipped"; entry: ExtractEntry } + | { kind: "parsed"; entry: ExtractEntry; label: string } + | { kind: "error"; entry: ExtractEntry; message: string }; + +/** Grammars must already be warmed (`warmGenericGrammars`, `warmContainerGrammars`). */ +export function extractOne(f: SourceStat, cachedHash: string | null): ExtractOneResult { + const rel = f.rel; + // Depth tier (hand-written, native grammar) if a language claims the file; + // otherwise the breadth tier (generic tags.scm over a WASM grammar). A container + // is neither: checked before the breadth tier so a grammar claiming .vue can't + // shadow it. + const lang = languageOf(f.abs); + const container = lang ? null : containerLangOf(f.abs); + const generic = lang || container ? null : genericLangOf(f.abs); + const label = languageLabelOf(f.abs) ?? container?.name ?? generic?.name ?? "unknown"; + + // Every file is read and hashed, every build; only the parse is memoized. + // Trusting (size, mtime) here would make a same-length edit inside one mtime + // tick invisible to `graft build`, which `graft check` (always hashing) would + // then report as drift the build refuses to repair. + let source: string | null; + try { + source = readSourceFile(f.abs); + } catch (err) { + const message = `${rel}: ${err instanceof Error ? err.message : String(err)}`; + return { kind: "skipped", entry: { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [], error: message } }; + } + if (source === null) { + // Unsupported encoding (UTF-16BE): a skip, never an error. + return { kind: "skipped", entry: { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [] } }; + } + + const hash = contentHash(source); + if (cachedHash !== null && hash === cachedHash) return { kind: "reused", hash }; + + try { + const { nodes, rawEdges } = lang + ? extractFile(rel, source, lang) + : container + ? extractContainer(rel, source, container) + : extractGeneric(rel, source, generic!.name); + return { kind: "parsed", label, entry: { size: f.size, mtimeMs: f.mtimeMs, hash, nodes, rawEdges } }; + } catch (err) { + const message = `${rel}: parse failed — ${err instanceof Error ? err.message : String(err)}`; + return { kind: "error", message, entry: { size: f.size, mtimeMs: f.mtimeMs, hash, nodes: [], rawEdges: [], error: message } }; + } +} diff --git a/test/graph-parse-pool.test.ts b/test/graph-parse-pool.test.ts new file mode 100644 index 000000000..d0a82c2e2 --- /dev/null +++ b/test/graph-parse-pool.test.ts @@ -0,0 +1,95 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { extractOne } from "../src/graph/extract-one.js"; +import { warmGenericGrammars } from "../src/graph/generic.js"; +import { listSourceStats } from "../src/graph/source-files.js"; +import { contentHash } from "../src/util/id.js"; + +const RS = "pub fn f() {}\npub fn g() { f() }\n"; + +function repo(): string { + const d = mkdtempSync(join(tmpdir(), "graft-pool-")); + mkdirSync(join(d, "src"), { recursive: true }); + writeFileSync(join(d, "src", "math.ts"), "export function add(a: number, b: number): number { return a + b; }\n"); + writeFileSync(join(d, "src", "lib.rs"), RS); + return d; +} + +test("extractOne: a fresh file is parsed and its entry carries the tier label", async () => { + const d = repo(); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const rs = files.find((f) => f.rel === "src/lib.rs")!; + const ts = files.find((f) => f.rel === "src/math.ts")!; + const r1 = extractOne(rs, null); + assert.equal(r1.kind, "parsed"); + if (r1.kind !== "parsed") return; + assert.equal(r1.label, "rust"); + assert.ok(r1.entry.nodes.some((n) => n.id === "src/lib.rs#f")); + assert.equal(r1.entry.hash, contentHash(RS)); + const r2 = extractOne(ts, null); + assert.equal(r2.kind, "parsed"); + if (r2.kind !== "parsed") return; + assert.equal(r2.label, "typescript"); +}); + +test("extractOne: matching bytes report reused without parsing", () => { + const d = repo(); + const rs = listSourceStats(d, join(d, "graft")).find((f) => f.rel === "src/lib.rs")!; + assert.deepEqual(extractOne(rs, contentHash(RS)), { kind: "reused", hash: contentHash(RS) }); +}); + +test("extractOne: an unreadable file is skipped with an empty-hash entry that names the file", () => { + const r = extractOne({ abs: join(tmpdir(), "does-not-exist-graft.rs"), rel: "nope.rs", size: 0, mtimeMs: 0 }, null); + assert.equal(r.kind, "skipped"); + if (r.kind !== "skipped") return; + assert.equal(r.entry.hash, ""); + assert.match(r.entry.error ?? "", /^nope\.rs: /); +}); + +import { buildGraph } from "../src/graph/build.js"; +import { readGraph, wiringPath } from "../src/graph/write.js"; +import { contextDirFor } from "../src/context/node-file.js"; +import type { CruxSummarizer, FileCruxInput, NodeCrux } from "../src/ai/crux.js"; + +/** Same shape as RecordingCrux in test/graph-enrich-checkpoint.test.ts. */ +class TrivialCrux implements CruxSummarizer { + calls: string[] = []; + async describeFile(input: FileCruxInput): Promise { + this.calls.push(input.path); + return input.nodes.map((n) => ({ id: n.id, summary: `crux ${n.id}`, crux_start: 0, crux_end: 0 })); + } +} + +test("a --deep build summarizes from disk without buildGraph holding every source", async () => { + const d = repo(); + const crux = new TrivialCrux(); + const r = await buildGraph(d, { reuse: false, summarizer: crux }); + assert.equal(r.errors.length, 0, r.errors.join("; ")); + assert.ok(crux.calls.includes("src/lib.rs")); + const g = readGraph(wiringPath(contextDirFor(d)))!; + assert.equal(g.nodes.find((n) => n.id === "src/lib.rs#f")!.summary_state, "ready"); +}); + +test("a file edited between parse and summary is left pending, not summarized against the wrong lines", async () => { + const d = repo(); + let edited = false; + const crux: CruxSummarizer = { + async describeFile(input) { + // Simulate an agent editing another file while this one is summarized. + if (!edited) { edited = true; writeFileSync(join(d, "src", "math.ts"), "export function add(a: number, b: number): number {\n return a + b + 0;\n}\n"); } + return input.nodes.map((n) => ({ id: n.id, summary: `crux ${n.id}`, crux_start: 0, crux_end: 0 })); + }, + }; + const r = await buildGraph(d, { reuse: false, summarizer: crux, concurrency: 1 }); + assert.equal(r.errors.length, 0, r.errors.join("; ")); + const g = readGraph(wiringPath(contextDirFor(d)))!; + const states = new Set(g.nodes.filter((n) => n.path === "src/math.ts" || n.path === "src/lib.rs").map((n) => n.summary_state)); + // Whichever file was summarized first is ready; the edited one may be pending. + // What must never happen is a "ready" summary computed against changed bytes. + assert.ok(states.has("ready")); + assert.ok(r.meaning.pending + r.meaning.computed >= 1); +}); From 69525d2ea22286346834cb3150b73053b2931213 Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:34:31 -0600 Subject: [PATCH 7/8] feat(build): graft build --workers, an opt-in forked parser pool Large cold builds can parse in child processes; results are folded in file order so the graph is byte-identical to a single-process build. Off by default and read only by the build command, so the pre-query refresh, the MCP server and the App never fork. A dead child's job is retried once; a job that kills two children is parked and becomes an error unless the pool never succeeded at all, in which case the parent parses the rest itself. --- .env.example | 1 + README.md | 2 + src/cli.ts | 9 ++ src/engine.ts | 4 + src/graph/build.ts | 38 +++++- src/graph/parse-pool.ts | 235 ++++++++++++++++++++++++++++++++++ src/graph/parse-worker.ts | 39 ++++++ test/graph-parse-pool.test.ts | 132 ++++++++++++++++++- 8 files changed, 455 insertions(+), 5 deletions(-) create mode 100644 src/graph/parse-pool.ts create mode 100644 src/graph/parse-worker.ts diff --git a/.env.example b/.env.example index 76a3c31a2..e1c584af0 100644 --- a/.env.example +++ b/.env.example @@ -54,6 +54,7 @@ # --- Build side effects (opt-out) --------------------------------------------- # GRAFT_NO_GITIGNORE=1 — skip writing graft/ into .gitignore (e.g. you already ignore it globally) # GRAFT_NO_IGNORE=1 — skip writing .ignore (ripgrep re-admit); only if you manage search yourself +# GRAFT_PARSE_WORKERS=auto — `graft build` parses in child processes (auto = cores-1, memory-capped), or a number. Off by default; the pre-query refresh never forks. # --- Init (Claude Code) ------------------------------------------------------- # Skip writing statusLine / subagentStatusLine during `graft init` (same as diff --git a/README.md b/README.md index 709a27680..8ea574900 100644 --- a/README.md +++ b/README.md @@ -352,6 +352,7 @@ graft build [dir] # build graft/ from the code at [dir]: wiri graft build --deep # add the LLM layer: concept nodes + per-symbol summary/crux (cached) graft build --extensions .ts .py # only include these code extensions graft build --no-reuse # re-parse every file instead of replaying unchanged ones from cache +graft build --workers auto # large repo? parse in parallel child processes (opt-in; never used by the pre-query refresh) graft build --follow-submodules # include initialized submodules; persist the choice for builds + MCP refresh graft build --no-follow-submodules # exclude submodules again and persist that choice (the default) graft build --follow-nested-repos # include nested git clones the index doesn't track; persist the choice @@ -389,6 +390,7 @@ graft check --json # print the drift report as JSON # --no-refresh # answer from the graph exactly as it is on disk # GRAFT_NO_REFRESH=1 # same, for every command # GRAFT_REFRESH=hash # hash every file instead of trusting size+mtime +# GRAFT_PARSE_WORKERS=auto # graft build only: parse in child processes (or a number); default in-process graft trail push [dir] # read this repo's history into its Trail (the first run signs up in the browser) graft trail pull [dir] # write the changes accepted in Trail into CLAUDE.md, AGENTS.md and the other context files diff --git a/src/cli.ts b/src/cli.ts index e7656d563..f188bb10d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -365,6 +365,7 @@ program .option("--deep", "run the LLM pass: concept nodes (graft/*.md) + per-symbol summary/crux") .option("-e, --extensions ", 'code extensions to include (e.g. ".ts" ".py"); an extension with no parser is ignored with a warning that lists the supported set') .option("-j, --concurrency ", "files summarized in parallel during --deep (default 5)") + .option("--workers ", "parse in child processes for a large cold build (or 'auto'); default in-process. GRAFT_PARSE_WORKERS sets the default") .option("--no-reuse", "re-parse every file instead of replaying unchanged ones from the extraction cache") .option("--lsp", "add compiler-grade call edges via a language server if one is installed (opt-in, slower; e.g. rust-analyzer, clangd)") .option("--allow-partial", "with --deep: exit 0 even when some files' summaries failed (default: a degraded meaning tier exits 1)") @@ -408,6 +409,7 @@ program deep?: boolean; extensions?: string[]; concurrency?: string; + workers?: string; reuse?: boolean; lsp?: boolean; allowPartial?: boolean; @@ -428,6 +430,11 @@ program console.error(`✗ --concurrency must be a number, got "${opts.concurrency}"`); process.exit(1); } + // Opt-in only. The env var is read HERE, by the one command a human runs, + // never by buildGraph: the pre-query refresh must not fork under a query. + const workersArg = (opts.workers as string | undefined) ?? process.env.GRAFT_PARSE_WORKERS; + const parseWorkers: number | "auto" | undefined = + workersArg === undefined ? undefined : workersArg === "auto" ? "auto" : Number.parseInt(workersArg, 10) || 0; warnUnsupportedExtensions(opts.extensions); // Persisted BEFORE the build itself runs, so this invocation's walks (and // every later no-flag build / hooks refresh) see it identically — the @@ -549,6 +556,7 @@ program const g = await engine.graph(dir, { llm: deep, concurrency, + parseWorkers, reuse: opts.reuse, lsp: opts.lsp, onlyDirs, @@ -563,6 +571,7 @@ program process.stderr.write("\n"); console.log(`✓ wiring: ${g.nodes} nodes (${fmt(g.byKind)}), ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); console.log(` parsed: ${g.parsed} of ${g.files} files (${g.reused} replayed from cache)`); + if (g.parseWorkers > 0) console.log(` workers: ${g.parseWorkers} parser processes`); // Worth one line: this build started from a graph the user never built *here*. if (g.seededFrom) console.log(` seeded: copied a starting graph from ${g.seededFrom} (git worktree)`); if (deep) { diff --git a/src/engine.ts b/src/engine.ts index 5ec744721..757b6c56f 100644 --- a/src/engine.ts +++ b/src/engine.ts @@ -49,6 +49,9 @@ export interface GraphRunOptions { lsp?: boolean; /** Repo-relative directory prefixes to limit the build to (`--only-dir`). */ onlyDirs?: string[]; + /** Parser child processes for a large cold build (`graft build --workers`); + * `"auto"` sizes the pool. Absent/0 parses in-thread. */ + parseWorkers?: number | "auto"; onProgress?: GraphBuildOptions["onProgress"]; } @@ -93,6 +96,7 @@ export class Graft { contextDir: this.cfg.contextDir, summarizer: opts.llm ? this.cruxSummarizer() : undefined, concurrency: opts.concurrency, + parseWorkers: opts.parseWorkers, reuse: opts.reuse, lsp: opts.lsp, onlyDirs: opts.onlyDirs, diff --git a/src/graph/build.ts b/src/graph/build.ts index 126fd2228..2d56e6c41 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -34,6 +34,7 @@ import { writeFingerprint } from "./fingerprint.js"; import { seedGraph, type SeedResult } from "./seed.js"; import { filterByOnlyDirs, listSourceStats } from "./source-files.js"; import { extractOne, type ExtractOneResult } from "./extract-one.js"; +import { poolSize, runParsePool } from "./parse-pool.js"; import type { SourceStat } from "./source-files.js"; import { resolveEdges, type GoModule } from "./resolve.js"; import { enrichGraph, type EnrichStats, type SourceLookup } from "./enrich.js"; @@ -95,6 +96,12 @@ export interface GraphBuildOptions { * set, only files under these prefixes are indexed; the list is recorded in the * fingerprint so the freshness probe enumerates the same set. */ onlyDirs?: string[]; + /** Parser child processes for the parse loop (`graft build --workers`). Absent + * or 0: parse in-thread. `"auto"` sizes the pool from cores, free memory and + * the file count (see {@link poolSize}). `buildGraph` never decides this itself + * and never reads `process.env`: the pre-query refresh, the MCP server and the + * App must never fork under a query. */ + parseWorkers?: number | "auto"; onProgress?: (info: { phase: "parse" | "enrich"; index: number; @@ -113,6 +120,8 @@ export interface GraphBuildResult { parsed: number; /** Files replayed from the extraction cache. */ reused: number; + /** Parser children forked (0 = parsed in-thread). */ + parseWorkers: number; /** The parent checkout this build copied a starting graph from, when it was run in * a git worktree that had none of its own. See `./seed.ts`. */ seededFrom?: string; @@ -241,10 +250,30 @@ export async function buildGraph( return c && !c.error ? c.hash : null; }; - files.forEach((f, i) => { - opts.onProgress?.({ phase: "parse", index: i, total: files.length, file: f.rel }); - applyResult(f, extractOne(f, cachedHashOf(f.rel))); - }); + // `cli.ts` does not know the file count, so `"auto"` is resolved here — the one + // place that already knows it. `buildGraph` never reads `process.env`: a number + // (or `"auto"`) had to be asked for explicitly by the `graft build` command. + const workers = opts.parseWorkers === "auto" + ? poolSize(files.length, { GRAFT_PARSE_WORKERS: "auto" }) + : Math.max(0, Math.floor(opts.parseWorkers ?? 0)); + if (workers > 0 && files.length > 0) { + let done = 0; + const results = await runParsePool(files, cachedHashOf, { + generic: new Set(files.map((f) => genericLangOf(f.abs)?.name).filter((n): n is string => !!n)), + container: new Set(files.map((f) => containerLangOf(f.abs)?.name).filter((n): n is string => !!n)), + }, { + workers, + onResult: (_index, file) => opts.onProgress?.({ phase: "parse", index: done++, total: files.length, file }), + }); + // Fold in FILE order, never completion order: node and edge order is what + // makes a pooled build byte-identical to an in-thread one. + files.forEach((f, i) => applyResult(f, results[i])); + } else { + files.forEach((f, i) => { + opts.onProgress?.({ phase: "parse", index: i, total: files.length, file: f.rel }); + applyResult(f, extractOne(f, cachedHashOf(f.rel))); + }); + } // Persist the memo BEFORE enrichment, because `enrichGraph` mutates these very // node objects (summary/crux/summary_state) and the cache must only ever hold @@ -388,6 +417,7 @@ export async function buildGraph( files: files.length, parsed, reused, + parseWorkers: workers, seededFrom: seed.from, nodes: nodes.length, edges: edges.length, diff --git a/src/graph/parse-pool.ts b/src/graph/parse-pool.ts new file mode 100644 index 000000000..d14cc60b6 --- /dev/null +++ b/src/graph/parse-pool.ts @@ -0,0 +1,235 @@ +/** + * A pool of forked parser children for large cold builds. Opt-in only: + * `buildGraph` never forks unless the `graft build` command asked for it + * (`--workers`, or `GRAFT_PARSE_WORKERS`), so the pre-query refresh, the MCP + * server and the App's review worker parse in-thread whatever the edit size. + * + * Failure modes degrade, never hang and never reject: a dead child's job is + * retried once, then parked. A parked job's fate is decided only once the pool + * settles, by whether the pool ever completed a job in a child at all: + * - if some job succeeded, a file that killed two children is a poisoned file, + * and its parked job becomes an `error` result — never retried in the parent, + * where the same crash would take down the whole build; + * - if no job ever succeeded, the crashes are environmental (fork, loader, + * permissions), so the parked jobs are parsed in-thread with the rest. + * A pool that cannot keep children alive hands the remaining queue back to the + * parent, which has warmed grammars. + */ +import { fork, type ChildProcess, type ForkOptions } from "node:child_process"; +import { existsSync } from "node:fs"; +import { availableParallelism, freemem } from "node:os"; +import { fileURLToPath } from "node:url"; +import { extractOne, type ExtractOneResult } from "./extract-one.js"; +import type { SourceStat } from "./source-files.js"; +import type { FromChild, ToChild } from "./parse-worker.js"; + +/** Under this many files, forking, warming grammars (~300 ms per child) and IPC + * cost more than they save. */ +export const PARSE_POOL_MIN_FILES = 200; +const FILES_PER_CHILD = 200; +/** Working set of one child with nine native grammars, WASM and a parse in flight. */ +const BYTES_PER_CHILD = 300 * 1024 * 1024; +const MAX_ATTEMPTS = 2; + +export interface PoolOptions { + workers: number; + entry?: string; + onResult?: (index: number, file: string) => void; + jobTimeoutMs?: number; + warmTimeoutMs?: number; +} + +/** `dist/graph/parse-worker.js`, or the `.ts` next to it when run from a checkout + * under tsx (fork inherits `--import tsx` through execArgv). */ +export function parseWorkerEntry(): string { + const js = fileURLToPath(new URL("./parse-worker.js", import.meta.url)); + if (existsSync(js)) return js; + const ts = js.replace(/\.js$/, ".ts"); + return existsSync(ts) ? ts : js; +} + +/** Children to fork for `files` files. 0 means in-thread, and 0 is the answer + * unless `GRAFT_PARSE_WORKERS` says otherwise: `N` forces N, `auto` picks + * `min(cores - 1, freemem / 300 MB, files / 200)`, anything else is in-thread. */ +export function poolSize( + files: number, + env: NodeJS.ProcessEnv = process.env, + cores = availableParallelism(), + freeBytes = freemem(), +): number { + const v = (env.GRAFT_PARSE_WORKERS ?? "").trim(); + if (v === "") return 0; + if (v !== "auto") { + const n = Number.parseInt(v, 10); + return Number.isFinite(n) && n >= 0 && String(n) === v ? n : 0; + } + if (files < PARSE_POOL_MIN_FILES) return 0; + const byCores = cores - 1; + const byMemory = Math.floor(freeBytes / BYTES_PER_CHILD); + const byWork = Math.ceil(files / FILES_PER_CHILD); + const n = Math.min(byCores, byMemory, byWork); + return n >= 2 ? n : 0; +} + +interface Job { index: number; attempts: number; exit?: string } + +export function runParsePool( + files: SourceStat[], + cachedHashOf: (rel: string) => string | null, + langs: { generic: Iterable; container: Iterable }, + opts: PoolOptions, +): Promise { + const entry = opts.entry ?? parseWorkerEntry(); + const jobTimeoutMs = opts.jobTimeoutMs ?? 120_000; + const warmTimeoutMs = opts.warmTimeoutMs ?? 60_000; + const results: ExtractOneResult[] = new Array(files.length); + const queue: Job[] = files.map((_, index) => ({ index, attempts: 0 })); + const init: ToChild = { type: "init", generic: [...langs.generic], container: [...langs.container] }; + let outstanding = files.length; + let succeeded = 0; + let inFlight = 0; + let respawns = 0; + const maxRespawns = opts.workers * 2; + const children = new Set(); + // Jobs that killed MAX_ATTEMPTS children: held here until the pool settles, when + // `succeeded` decides error (poisoned file) vs in-thread parse (environmental). + const parked: Job[] = []; + let settled = false; + + return new Promise((resolvePool) => { + const killAll = (): void => { + for (const c of children) { try { c.kill(); } catch { /* already gone */ } } + children.clear(); + }; + const finish = (): void => { + if (settled) return; + settled = true; + killAll(); + resolvePool(results); + }; + /** Drain the parked jobs. `inThread` (i.e. `succeeded === 0`) means the crashes + * were environmental, so parse them like the rest of the queue; otherwise they + * are the brief's twice-crashed `error` result. */ + const settleParked = (inThread: boolean): void => { + if (settled) return; + for (const job of parked.splice(0)) { + const f = files[job.index]; + if (inThread) { + results[job.index] = extractOne(f, cachedHashOf(f.rel)); + } else { + const message = `${f.rel}: parse failed — parser process exited (${job.exit ?? "unknown"}) on this file twice`; + results[job.index] = { kind: "error", message, entry: { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [], error: message } }; + } + opts.onResult?.(job.index, f.rel); + outstanding--; + } + if (outstanding === 0) finish(); + }; + /** The pool cannot keep children alive: parse what is left here, in order, + * then settle the parked jobs (in-thread, since none ever succeeded). */ + const fallbackInThread = (): void => { + if (settled) return; + killAll(); + for (const job of queue.splice(0)) { + results[job.index] = extractOne(files[job.index], cachedHashOf(files[job.index].rel)); + opts.onResult?.(job.index, files[job.index].rel); + outstanding--; + } + settleParked(succeeded === 0); + if (outstanding === 0) finish(); + }; + /** No queued work and no child holds a job: the pool has drained. Settle any + * parked jobs (poisoned files if the pool ever worked, environmental if not), + * or finish if there is nothing left. */ + const drainIfIdle = (): void => { + if (queue.length !== 0 || inFlight !== 0) return; + if (parked.length > 0) settleParked(succeeded === 0); + else if (outstanding === 0) finish(); + }; + const post = (child: ChildProcess, msg: ToChild): boolean => { + if (!child.connected) return false; + try { child.send(msg); return true; } catch { return false; } + }; + + const spawn = (): void => { + // `windowsHide` is a real, effective fork option (forwarded to spawn) but + // @types/node 26 lists it on CommonOptions, which ForkOptions does not + // extend — hence the intersection rather than dropping the flag or `any`. + const forkOpts: ForkOptions & { windowsHide?: boolean } = { + stdio: ["ignore", "ignore", "inherit", "ipc"], + serialization: "advanced", + windowsHide: true, + }; + const child = fork(entry, [], forkOpts); + children.add(child); + let current: Job | null = null; + let ready = false; + let timer: NodeJS.Timeout | null = setTimeout(() => child.kill(), warmTimeoutMs); + timer.unref(); + const clearTimer = (): void => { if (timer) { clearTimeout(timer); timer = null; } }; + + const next = (): void => { + const job = queue.shift(); + if (!job) { + current = null; + post(child, { type: "stop" }); + drainIfIdle(); + return; + } + job.attempts++; + current = job; + inFlight++; + timer = setTimeout(() => child.kill(), jobTimeoutMs); + timer.unref(); + if (!post(child, { type: "job", seq: job.index, f: files[job.index], cachedHash: cachedHashOf(files[job.index].rel) })) { + // Dead channel: the exit handler re-queues `current` and decrements inFlight. + clearTimer(); + } + }; + + child.on("message", (m: FromChild) => { + if (settled) return; + if (m.type === "ready") { ready = true; clearTimer(); next(); return; } + if (m.type !== "done" || current === null || m.seq !== current.index) return; + clearTimer(); + succeeded++; + results[m.seq] = m.result; + opts.onResult?.(m.seq, files[m.seq].rel); + current = null; + inFlight--; + if (--outstanding === 0) finish(); + else next(); + }); + + const onGone = (code: number | null, signal: NodeJS.Signals | null): void => { + clearTimer(); + children.delete(child); + if (settled) return; + if (current !== null) { + const job = current; + current = null; + inFlight--; + if (job.attempts < MAX_ATTEMPTS) { + queue.unshift(job); + } else { + // Twice-crashed: park it. Whether it is an error or parsed in-thread is + // decided at settle time by `succeeded`, not here. + job.exit = signal ?? `code ${code}`; + parked.push(job); + } + } + if (queue.length === 0) { drainIfIdle(); return; } + // Work remains. Replace the child within reason; past that, the pool is + // not working on this machine and the parent finishes the job itself. + if (respawns++ < maxRespawns) spawn(); + else if (children.size === 0) fallbackInThread(); + void ready; + }; + child.once("exit", onGone); + child.once("error", () => onGone(null, null)); // spawn failure: no exit follows + if (!post(child, init)) onGone(null, null); + }; + + for (let i = 0; i < Math.max(1, opts.workers); i++) spawn(); + }); +} diff --git a/src/graph/parse-worker.ts b/src/graph/parse-worker.ts new file mode 100644 index 000000000..ce1fe4bbb --- /dev/null +++ b/src/graph/parse-worker.ts @@ -0,0 +1,39 @@ +/** + * A parser child. `parse-pool.ts` forks N of these for a large cold build + * (`graft build --workers`): each warms its own grammars (native addons and WASM + * alike, in a fresh process), then answers `job` messages with the very + * `extractOne` the in-thread loop uses. A child process rather than a worker + * thread for the reasons `app/review-process.ts` spells out: the nine native + * grammars load exactly as they do in production, a wedged parse is a process + * to kill, and a crash (or a WASM abort, which poisons its Emscripten runtime + * for good) costs one file instead of the build. + */ +import { extractOne, type ExtractOneResult } from "./extract-one.js"; +import { warmGenericGrammars } from "./generic.js"; +import { warmContainerGrammars } from "./container.js"; +import type { SourceStat } from "./source-files.js"; + +export type ToChild = + | { type: "init"; generic: string[]; container: string[] } + | { type: "job"; seq: number; f: SourceStat; cachedHash: string | null } + | { type: "stop" }; +export type FromChild = { type: "ready" } | { type: "done"; seq: number; result: ExtractOneResult }; + +function send(msg: FromChild): void { + try { process.send?.(msg); } catch { /* parent gone; disconnect handler exits */ } +} + +if (process.send) { + process.on("message", (m: ToChild) => { + if (m.type === "init") { + void Promise.all([warmGenericGrammars(m.generic), warmContainerGrammars(m.container)]) + .then(() => send({ type: "ready" }), () => process.exit(3)); + } else if (m.type === "job") { + send({ type: "done", seq: m.seq, result: extractOne(m.f, m.cachedHash) }); + } else if (m.type === "stop") { + process.exit(0); + } + }); + // The parent going away (killed, crashed) closes the channel: do not linger. + process.on("disconnect", () => process.exit(0)); +} diff --git a/test/graph-parse-pool.test.ts b/test/graph-parse-pool.test.ts index d0a82c2e2..f2655209b 100644 --- a/test/graph-parse-pool.test.ts +++ b/test/graph-parse-pool.test.ts @@ -1,6 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { extractOne } from "../src/graph/extract-one.js"; @@ -93,3 +93,133 @@ test("a file edited between parse and summary is left pending, not summarized ag assert.ok(states.has("ready")); assert.ok(r.meaning.pending + r.meaning.computed >= 1); }); + +import { existsSync } from "node:fs"; +import { pathToFileURL } from "node:url"; +import { PARSE_POOL_MIN_FILES, parseWorkerEntry, poolSize, runParsePool } from "../src/graph/parse-pool.js"; + +test("poolSize: 0 unless GRAFT_PARSE_WORKERS is set or the caller asks; then bounded by cores, memory and work", () => { + assert.equal(poolSize(100_000, {}, 8, 64e9), 0, "never automatic"); + assert.equal(poolSize(100_000, { GRAFT_PARSE_WORKERS: "0" }, 8, 64e9), 0); + assert.equal(poolSize(100_000, { GRAFT_PARSE_WORKERS: "3" }, 8, 64e9), 3); + assert.equal(poolSize(100_000, { GRAFT_PARSE_WORKERS: "auto" }, 8, 64e9), 7, "auto = cores - 1"); + assert.equal(poolSize(100_000, { GRAFT_PARSE_WORKERS: "auto" }, 64, 1e9), 3, "memory-capped at ~300 MB per child"); + assert.equal(poolSize(50, { GRAFT_PARSE_WORKERS: "auto" }, 64, 64e9), 0, "under the minimum, in-thread"); + assert.equal(poolSize(PARSE_POOL_MIN_FILES * 2, { GRAFT_PARSE_WORKERS: "auto" }, 64, 64e9), 2, "work-capped"); + assert.equal(poolSize(100_000, { GRAFT_PARSE_WORKERS: "garbage" }, 8, 64e9), 0, "unparseable = in-thread"); +}); + +test("parseWorkerEntry points at a file that exists", () => { + assert.ok(existsSync(parseWorkerEntry()), parseWorkerEntry()); +}); + +test("runParsePool returns the same results, in file order, as extractOne in-thread", async () => { + const d = repo(); + for (let i = 0; i < 30; i++) writeFileSync(join(d, "src", `m${i}.rs`), `pub fn m${i}() { f() }\n`); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const inThread = files.map((f) => extractOne(f, null)); + const pooled = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2 }); + assert.deepEqual(pooled, inThread); +}); + +test("runParsePool honours cachedHash and reports reused", async () => { + const d = repo(); + const files = listSourceStats(d, join(d, "graft")); + const rs = files.find((f) => f.rel === "src/lib.rs")!; + const pooled = await runParsePool(files, (rel) => (rel === rs.rel ? contentHash(RS) : null), { generic: ["rust"], container: [] }, { workers: 2 }); + assert.deepEqual(pooled[files.indexOf(rs)], { kind: "reused", hash: contentHash(RS) }); +}); + +/** A child entry that behaves like parse-worker.ts except where `mode` says otherwise. */ +function fakeWorker(dir: string, mode: "crash-on-boom" | "never-ready" | "always-crash"): string { + const entry = join(dir, `worker-${mode}.mjs`); + const src = pathToFileURL(join(process.cwd(), "src", "graph")).href; + writeFileSync(entry, [ + `import { extractOne } from ${JSON.stringify(src + "/extract-one.ts")};`, + `import { warmGenericGrammars } from ${JSON.stringify(src + "/generic.ts")};`, + `const mode = ${JSON.stringify(mode)};`, + "process.on('message', async (m) => {", + " if (m.type === 'init') { if (mode === 'never-ready') return; await warmGenericGrammars(m.generic); process.send({ type: 'ready' }); }", + " else if (m.type === 'job') {", + " if (mode === 'always-crash' || (mode === 'crash-on-boom' && m.f.rel.endsWith('boom.rs'))) process.exit(9);", + " process.send({ type: 'done', seq: m.seq, result: extractOne(m.f, m.cachedHash) });", + " } else if (m.type === 'stop') process.exit(0);", + "});", + "process.on('disconnect', () => process.exit(0));", + ].join("\n")); + return entry; +} + +test("runParsePool: a child that dies mid-job costs that file two attempts, then an error; the rest still parse", async () => { + const d = repo(); + writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "crash-on-boom") }); + const boom = results[files.findIndex((f) => f.rel === "src/boom.rs")]; + assert.equal(boom.kind, "error"); + if (boom.kind !== "error") return; + assert.match(boom.message, /src\/boom\.rs: parse failed — parser process exited/); + assert.equal(results[files.findIndex((f) => f.rel === "src/lib.rs")].kind, "parsed"); +}); + +test("runParsePool: children that never become ready are killed at the warm-up timeout and the parent parses in-thread", async () => { + const d = repo(); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const t0 = Date.now(); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "never-ready"), warmTimeoutMs: 1500 }); + assert.ok(Date.now() - t0 < 10_000, "did not hang"); + assert.deepEqual(results, files.map((f) => extractOne(f, null)), "in-thread fallback produced the full result set"); +}); + +test("runParsePool: children that always crash fall back to in-thread parsing instead of rejecting", async () => { + const d = repo(); + for (let i = 0; i < 10; i++) writeFileSync(join(d, "src", `c${i}.rs`), `pub fn c${i}() {}\n`); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "always-crash") }); + assert.equal(results.length, files.length); + assert.ok(results.every((r) => r.kind === "parsed"), "every file was parsed in-thread after the pool gave up"); +}); + +test("runParsePool: a file that kills every child with no other work is treated as environmental and parsed in-thread", async () => { + // boom.rs is the ONLY file, so no job ever completes in a child (succeeded === 0). + // The crashes are therefore taken to be environmental (fork/loader/permissions), + // not this file poisoning the parser, so the parked job is parsed in-thread. + const d = mkdtempSync(join(tmpdir(), "graft-pool-solo-")); + mkdirSync(join(d, "src"), { recursive: true }); + writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + assert.equal(files.length, 1); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "crash-on-boom") }); + assert.equal(results.length, 1); + assert.equal(results[0].kind, "parsed"); +}); + +test("buildGraph with parseWorkers writes byte-identical wiring.json to the in-thread build", async () => { + const d = repo(); + for (let i = 0; i < 40; i++) writeFileSync(join(d, "src", `p${i}.rs`), `pub fn p${i}() { f() }\n`); + const inThread = await buildGraph(d, { reuse: false }); + assert.equal(inThread.parseWorkers, 0); + const a = readFileSync(wiringPath(contextDirFor(d)), "utf8"); + const pooled = await buildGraph(d, { reuse: false, parseWorkers: 2 }); + assert.equal(pooled.parseWorkers, 2); + assert.equal(pooled.parsed, inThread.parsed); + assert.equal(readFileSync(wiringPath(contextDirFor(d)), "utf8"), a); + assert.equal((await buildGraph(d, { reuse: false, parseWorkers: "auto" })).parseWorkers, 0, "auto on 42 files stays in-thread"); +}); + +test("buildGraph never forks unless asked, whatever the environment says", async () => { + const d = repo(); + const prev = process.env.GRAFT_PARSE_WORKERS; + process.env.GRAFT_PARSE_WORKERS = "4"; + try { + const r = await buildGraph(d, { reuse: false }); + assert.equal(r.parseWorkers, 0, "the env var is read by the CLI, not by buildGraph"); + } finally { + if (prev === undefined) delete process.env.GRAFT_PARSE_WORKERS; else process.env.GRAFT_PARSE_WORKERS = prev; + } +}); From b1d2ff80a60dd645e79a819836a2f04a37b76265 Mon Sep 17 00:00:00 2001 From: Kane Bruce <54819806+csm-kb@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:34:31 -0600 Subject: [PATCH 8/8] fix(build): harden the parser pool and streaming readers; keep the background sync in-process; changelog --- .env.example | 5 +- CHANGELOG.md | 23 +++ README.md | 2 + src/ask/index-file.ts | 18 ++- src/claude/stats.ts | 12 +- src/claude/sync-run.ts | 8 +- src/cli.ts | 17 +- src/graph/build.ts | 10 +- src/graph/enrich.ts | 31 ++-- src/graph/extract-cache.ts | 29 ++-- src/graph/fingerprint.ts | 2 +- src/graph/parse-pool.ts | 154 +++++++++++++++--- src/graph/parse-worker.ts | 11 +- src/graph/seed.ts | 3 + src/graph/write.ts | 37 ++++- src/util/json-stream.ts | 56 ++++++- test/claude-hooks.test.ts | 25 +++ test/claude-stats.test.ts | 20 ++- test/graph-artifacts-stream.test.ts | 94 ++++++++++- test/graph-build-workers-cli.test.ts | 95 +++++++++++ test/graph-enrich-failures.test.ts | 12 ++ test/graph-incremental.test.ts | 8 +- test/graph-load.test.ts | 22 ++- test/graph-parse-pool.test.ts | 228 +++++++++++++++++++++++++-- test/graph-seed.test.ts | 19 +++ test/json-stream.test.ts | 37 ++++- 26 files changed, 860 insertions(+), 118 deletions(-) create mode 100644 test/graph-build-workers-cli.test.ts diff --git a/.env.example b/.env.example index e1c584af0..ecbe02b8c 100644 --- a/.env.example +++ b/.env.example @@ -54,7 +54,10 @@ # --- Build side effects (opt-out) --------------------------------------------- # GRAFT_NO_GITIGNORE=1 — skip writing graft/ into .gitignore (e.g. you already ignore it globally) # GRAFT_NO_IGNORE=1 — skip writing .ignore (ripgrep re-admit); only if you manage search yourself -# GRAFT_PARSE_WORKERS=auto — `graft build` parses in child processes (auto = cores-1, memory-capped), or a number. Off by default; the pre-query refresh never forks. +# GRAFT_PARSE_WORKERS=auto — `graft build` parses in child processes, or a number (capped at the core count and the file count). +# auto = min(cores - 1, free memory / 300 MB, files / 200); in-process under 200 files or when that comes to fewer than 2. +# Off by default; the pre-query refresh, background sync and workspace builds never fork. +# Very large repos (tens of thousands of files) also need a bigger V8 heap: NODE_OPTIONS=--max-old-space-size=16384 graft build. # --- Init (Claude Code) ------------------------------------------------------- # Skip writing statusLine / subagentStatusLine during `graft init` (same as diff --git a/CHANGELOG.md b/CHANGELOG.md index d51a45cff..098684724 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ ## Unreleased +### Changed + +- **Very large repos build and load.** The extract cache, wiring.json and the + ask index were each one `JSON.stringify` and one `JSON.parse`, which V8 caps + at 512 MB; a C source base of ~65,000 files overshoots that on all three. + The extract cache is now newline-delimited JSON, and wiring.json and the ask + index are written one element per line (still plain JSON for every reader) + and read back line by line when they are over the cap. `graft viz` still + reads wiring.json as one string and is limited to graphs under 512 MB. +- **`graft build` holds far less memory on big repos.** Source text is no + longer kept for every file across the build; the meaning pass reads a file + when it summarizes it, and only if its bytes still hash to what was parsed. + A repo this size still needs a larger V8 heap than Node's default + (`NODE_OPTIONS=--max-old-space-size=16384` completed the 65k-file build); + moving `body_text` out of the retained node set is the next step. +- **`graft build --workers `** parses a large cold build in child + processes and merges their output in file order, so the graph is + byte-identical to a single-process build, except that a file whose parse + exceeds the 120 s per-file timeout is recorded as an error. Opt-in; the + refresh that runs before a query never forks. `GRAFT_PARSE_WORKERS` sets + the default; `auto` is min(cores - 1, free memory / 300 MB, files / 200), + and stays in-process under 200 files or below 2 workers. + ### Fixed - **`graft build` no longer aborts partway through very large repos.** The diff --git a/README.md b/README.md index 8ea574900..dea0d3833 100644 --- a/README.md +++ b/README.md @@ -353,6 +353,7 @@ graft build --deep # add the LLM layer: concept nodes + per-sy graft build --extensions .ts .py # only include these code extensions graft build --no-reuse # re-parse every file instead of replaying unchanged ones from cache graft build --workers auto # large repo? parse in parallel child processes (opt-in; never used by the pre-query refresh) +# very large repos (tens of thousands of files) need a bigger V8 heap: NODE_OPTIONS=--max-old-space-size=16384 graft build graft build --follow-submodules # include initialized submodules; persist the choice for builds + MCP refresh graft build --no-follow-submodules # exclude submodules again and persist that choice (the default) graft build --follow-nested-repos # include nested git clones the index doesn't track; persist the choice @@ -391,6 +392,7 @@ graft check --json # print the drift report as JSON # GRAFT_NO_REFRESH=1 # same, for every command # GRAFT_REFRESH=hash # hash every file instead of trusting size+mtime # GRAFT_PARSE_WORKERS=auto # graft build only: parse in child processes (or a number); default in-process +# # auto = min(cores-1, free memory / 300 MB, files / 200); in-process under 200 files or below 2 workers graft trail push [dir] # read this repo's history into its Trail (the first run signs up in the browser) graft trail pull [dir] # write the changes accepted in Trail into CLAUDE.md, AGENTS.md and the other context files diff --git a/src/ask/index-file.ts b/src/ask/index-file.ts index df3bcb425..aafffcbdb 100644 --- a/src/ask/index-file.ts +++ b/src/ask/index-file.ts @@ -23,7 +23,7 @@ import { constants as bufferConstants } from "node:buffer"; import { readFileSync, statSync } from "node:fs"; import { join } from "node:path"; import type { GraphV1 } from "../graph/types.js"; -import { forEachLine, openAtomic } from "../util/json-stream.js"; +import { forEachFileLine, forEachLine, openAtomic, type LineWalker } from "../util/json-stream.js"; import { CACHE_DIR } from "../context/node-file.js"; /** Words too common/short to carry query intent — dropped before scoring. */ @@ -122,10 +122,15 @@ export function writeAskIndex(outDir: string, graph: GraphV1): string { // Streamed one doc per line: still valid JSON for JSON.parse readers, but the // newline at each doc boundary lets readAskIndexFromBuffer walk a file over // V8's string cap without ever holding it as one string. On a 65k-file repo - // the one-shot string was ~360 MB, within the cap but with no headroom. Key + // the one-shot string measured ~590 MB, over the ~512 MB cap. Key // order matches AskIndex (version, avgBodyLen, df, docCount, docs). const w = openAtomic(outPath); try { + // The header is one line, and the whole `df` array lives on it — the streaming + // reader walks a line at a time, so this single line must itself fit under + // buffer.constants.MAX_STRING_LENGTH (~512 MB). df is the corpus vocabulary + // (one entry per distinct token, not per node), so even at 65k files it is a + // few MB — far off the ceiling. Only the per-line docs below scale with nodes. w.write(`{"version":1,"avgBodyLen":${JSON.stringify(avgBodyLen)},"df":${JSON.stringify(pairs(df))},"docCount":${nodes.length},"docs":[`); docs.forEach((doc, i) => w.write("\n" + JSON.stringify(doc) + (i < docs.length - 1 ? "," : ""))); w.write("\n]}\n"); @@ -170,7 +175,8 @@ export function readAskIndex(outDir: string, opts: ReadAskIndexOptions = {}): As try { size = statSync(path).size; } catch { return null; } const cap = opts.maxStringLength ?? bufferConstants.MAX_STRING_LENGTH; if (size > cap) { - try { return readAskIndexFromBuffer(readFileSync(path)); } catch { return null; } + // Chunked: past 2 GiB the file is too large for one Buffer, let alone a string. + try { return readAskIndexFromLines((fn) => forEachFileLine(path, fn)); } catch { return null; } } let raw: unknown; try { @@ -185,13 +191,17 @@ export function readAskIndex(outDir: string, opts: ReadAskIndexOptions = {}): As * the whole file as one string. Null when the buffer is not in that shape or * fails {@link validateAskIndex}. */ export function readAskIndexFromBuffer(buf: Buffer): AskIndex | null { + return readAskIndexFromLines((fn) => forEachLine(buf, fn)); +} + +function readAskIndexFromLines(eachLine: LineWalker): AskIndex | null { const DOCS_OPEN = ',"docs":['; const END = "]}"; let head: Record | null = null; const docs: AskIndexDoc[] = []; let closed = false; let bad = false; - forEachLine(buf, (line, i) => { + eachLine((line, i) => { if (bad || closed) { if (line !== "") bad = true; return; } try { if (i === 0) { diff --git a/src/claude/stats.ts b/src/claude/stats.ts index 92f9d7d86..b8b4f7a90 100644 --- a/src/claude/stats.ts +++ b/src/claude/stats.ts @@ -1,13 +1,13 @@ -import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; import type { GraphV1 } from '../graph/types.js'; +import { readGraph, wiringPath, type ReadGraphOptions } from '../graph/write.js'; import type { Stats } from './state.js'; import { resolveContextDir } from '../util/state.js'; -export function readWiring(projectDir: string): GraphV1 | null { - try { - return JSON.parse(readFileSync(join(resolveContextDir(projectDir), '.graph', 'wiring.json'), 'utf8')) as GraphV1; - } catch { return null; } +/** The project's wiring graph, or null when it is missing or unreadable. Through + * `readGraph`, so a graph over V8's string cap still reads (the statusline, the + * hooks and the background sync all start here); `opts` lets a test force that. */ +export function readWiring(projectDir: string, opts: ReadGraphOptions = {}): GraphV1 | null { + return readGraph(wiringPath(resolveContextDir(projectDir)), opts); } export function computeStats( diff --git a/src/claude/sync-run.ts b/src/claude/sync-run.ts index e07acb8eb..dadb57a32 100644 --- a/src/claude/sync-run.ts +++ b/src/claude/sync-run.ts @@ -9,9 +9,11 @@ function realBuild(dir: string): void { // GRAFT_TEST_CLI is the same seam hooks.ts's graftJson uses, so a test can // point this at a stub and inspect the exact argv it was invoked with. const cliPath = process.env.GRAFT_TEST_CLI ?? graftCliPath(); - const args = [cliPath, 'build', '.']; - // Mirrors `withContextDirArg` in hooks.ts: a no-op unless GRAFT_DIR is set, so an - // unconfigured repo's rebuild sees byte-identical argv to before this existed. + // `--workers 0`: a background sync never forks a parser pool. The flag outranks + // GRAFT_PARSE_WORKERS, which the child would otherwise inherit from this env or + // load from the repo's own .env through dotenv. + const args = [cliPath, 'build', '.', '--workers', '0']; + // Mirrors `withContextDirArg` in hooks.ts: a no-op unless GRAFT_DIR is set. if (process.env.GRAFT_DIR) args.push('--dir', resolveContextDir(dir)); execFileSync(process.execPath, args, { cwd: dir, stdio: 'ignore', timeout: 120000 }); } diff --git a/src/cli.ts b/src/cli.ts index f188bb10d..0f0b7d5ac 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -432,9 +432,15 @@ program } // Opt-in only. The env var is read HERE, by the one command a human runs, // never by buildGraph: the pre-query refresh must not fork under a query. - const workersArg = (opts.workers as string | undefined) ?? process.env.GRAFT_PARSE_WORKERS; + // An empty GRAFT_PARSE_WORKERS= line in a .env reads as unset, not as invalid. + const workersArg = (opts.workers as string | undefined) ?? (process.env.GRAFT_PARSE_WORKERS?.trim() || undefined); + if (workersArg !== undefined && workersArg !== "auto" && !/^\d+$/.test(workersArg)) { + const source = opts.workers !== undefined ? "--workers" : "GRAFT_PARSE_WORKERS"; + console.error(`✗ ${source} must be a non-negative integer or "auto", got "${workersArg}"`); + process.exit(1); + } const parseWorkers: number | "auto" | undefined = - workersArg === undefined ? undefined : workersArg === "auto" ? "auto" : Number.parseInt(workersArg, 10) || 0; + workersArg === undefined ? undefined : workersArg === "auto" ? "auto" : Number(workersArg); warnUnsupportedExtensions(opts.extensions); // Persisted BEFORE the build itself runs, so this invocation's walks (and // every later no-flag build / hooks refresh) see it identically — the @@ -514,6 +520,10 @@ program const buildRoot = resolve(dir); const buildGlobalDir = program.opts().dir; if (isWorkspaceBuildRoot(buildRoot, buildGlobalDir)) { + // Children build in-thread; `0` asks for exactly that, so only a pool request is ignored. + if (parseWorkers !== undefined && parseWorkers !== 0) { + console.error(`⚠ --workers / GRAFT_PARSE_WORKERS is ignored for workspace builds; each repo parses in-process`); + } await runWorkspaceBuild(buildRoot, { deep: !!deep, extensions: opts.extensions, @@ -571,7 +581,8 @@ program process.stderr.write("\n"); console.log(`✓ wiring: ${g.nodes} nodes (${fmt(g.byKind)}), ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); console.log(` parsed: ${g.parsed} of ${g.files} files (${g.reused} replayed from cache)`); - if (g.parseWorkers > 0) console.log(` workers: ${g.parseWorkers} parser processes`); + if (g.parseWorkers > 0 && g.poolFallback) console.log(` workers: ${g.parseWorkers} parser processes, until the pool could not keep them alive; the rest parsed in-process`); + else if (g.parseWorkers > 0) console.log(` workers: ${g.parseWorkers} parser processes`); // Worth one line: this build started from a graph the user never built *here*. if (g.seededFrom) console.log(` seeded: copied a starting graph from ${g.seededFrom} (git worktree)`); if (deep) { diff --git a/src/graph/build.ts b/src/graph/build.ts index 2d56e6c41..761ebccda 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -14,6 +14,7 @@ * one — the invariant `test/graph-incremental.test.ts` pins down. */ import { readFileSync } from "node:fs"; +import { availableParallelism } from "node:os"; import { basename, dirname, resolve } from "node:path"; import { walkDir } from "../ingest/fs.js"; import { contextDirFor, ensureGitignored, ensureSearchable } from "../context/node-file.js"; @@ -122,6 +123,8 @@ export interface GraphBuildResult { reused: number; /** Parser children forked (0 = parsed in-thread). */ parseWorkers: number; + /** The pool could not do the parsing and the parent parsed files in-thread. */ + poolFallback: boolean; /** The parent checkout this build copied a starting graph from, when it was run in * a git worktree that had none of its own. See `./seed.ts`. */ seededFrom?: string; @@ -253,9 +256,12 @@ export async function buildGraph( // `cli.ts` does not know the file count, so `"auto"` is resolved here — the one // place that already knows it. `buildGraph` never reads `process.env`: a number // (or `"auto"`) had to be asked for explicitly by the `graft build` command. + // An explicit count is honoured up to the cores and one child per file: more + // children than either only costs warm-ups (`auto` has its own, tighter, bounds). const workers = opts.parseWorkers === "auto" ? poolSize(files.length, { GRAFT_PARSE_WORKERS: "auto" }) - : Math.max(0, Math.floor(opts.parseWorkers ?? 0)); + : Math.min(Math.max(0, Math.floor(opts.parseWorkers ?? 0)), availableParallelism(), files.length); + let poolFallback = false; if (workers > 0 && files.length > 0) { let done = 0; const results = await runParsePool(files, cachedHashOf, { @@ -264,6 +270,7 @@ export async function buildGraph( }, { workers, onResult: (_index, file) => opts.onProgress?.({ phase: "parse", index: done++, total: files.length, file }), + onFallback: () => { poolFallback = true; }, }); // Fold in FILE order, never completion order: node and edge order is what // makes a pooled build byte-identical to an in-thread one. @@ -418,6 +425,7 @@ export async function buildGraph( parsed, reused, parseWorkers: workers, + poolFallback, seededFrom: seed.from, nodes: nodes.length, edges: edges.length, diff --git a/src/graph/enrich.ts b/src/graph/enrich.ts index c4dfbd368..614d9acba 100644 --- a/src/graph/enrich.ts +++ b/src/graph/enrich.ts @@ -152,25 +152,11 @@ export async function enrichGraph( await mapWithConcurrency(files, limit, async (path) => { const fileNodes = byFile.get(path)!; - const source = sources.get(path); - if (source === undefined) { - // The file changed or vanished between parse and summary: a crux sliced - // against different bytes would point at the wrong lines. Leave the nodes - // pending; the next build re-hashes and settles them. This runs before the - // gate.stopped check and, unlike the fatal-abort branch below, is not a - // provider failure, so it does not call gate.skip(). - for (const node of fileNodes) { - if (node.summary_state === "stale") stats.stale++; - else stats.pending++; - } - opts.onProgress?.({ index: done++, total: files.length, node: path }); - return; - } - const lineCount = source.split("\n").length; // Once the pass is fatal, the remaining files are not attempted: every call // would fail the same way, and on a metered gateway each one still costs a // request. They are counted so the caller can report what was left undone. + // Checked before `sources.get`, which reads and hashes the file from disk. if (gate.stopped) { gate.skip(); for (const node of fileNodes) { @@ -184,6 +170,21 @@ export async function enrichGraph( return; } + const source = sources.get(path); + if (source === undefined) { + // The file changed or vanished between parse and summary: a crux sliced + // against different bytes would point at the wrong lines. Leave the nodes + // pending; the next build re-hashes and settles them. Unlike the fatal-abort + // branch above, this is not a provider failure, so it does not call gate.skip(). + for (const node of fileNodes) { + if (node.summary_state === "stale") stats.stale++; + else stats.pending++; + } + opts.onProgress?.({ index: done++, total: files.length, node: path }); + return; + } + const lineCount = source.split("\n").length; + const refs: NodeRef[] = fileNodes.map((n) => { const [startLine, endLine] = spanLines(n.span, lineCount); return { id: n.id, kind: n.kind, signature: n.signature, startLine, endLine }; diff --git a/src/graph/extract-cache.ts b/src/graph/extract-cache.ts index aec139d5d..45e152edc 100644 --- a/src/graph/extract-cache.ts +++ b/src/graph/extract-cache.ts @@ -28,7 +28,7 @@ import { createHash } from "node:crypto"; import { fileURLToPath } from "node:url"; import { dirname, extname, join } from "node:path"; import { CACHE_DIR } from "../context/node-file.js"; -import { forEachLine, openAtomic, type AtomicWriter } from "../util/json-stream.js"; +import { forEachFileLine, openAtomic, type AtomicWriter } from "../util/json-stream.js"; import type { RawEdge } from "./extract.js"; import type { NodeV1 } from "./types.js"; @@ -199,26 +199,31 @@ export function readExtractCache(outDir: string): ExtractCache { const path = extractCachePath(outDir); const stamp = extractorStamp(); if (path === null || stamp === null) return emptyExtractCache(); - let buf: Buffer; - try { buf = readFileSync(path); } catch { return emptyExtractCache(); } - // Decoded a line at a time: the file may be larger than any string V8 holds. - const files: Record = {}; + // Read in chunks and decoded a line at a time: the file may be larger than any + // string V8 holds, and than the 2 GiB one Buffer can. + // No prototype: `rel` comes from disk, and `__proto__` must stay a plain key. + const files: Record = Object.create(null) as Record; let ok = true; + let header = false; try { - forEachLine(buf, (line, i) => { - if (!ok || line === "") return; - const parsed = JSON.parse(line) as { version?: number; extractor?: string; rel?: string; e?: ExtractEntry }; + forEachFileLine(path, (line, i) => { + if (!ok) return; if (i === 0) { - if (parsed.version !== CACHE_VERSION || parsed.extractor !== stamp) ok = false; + // A blank or missing header is no header: nothing after it is trusted. + const head = (line === "" ? null : JSON.parse(line)) as { version?: number; extractor?: string } | null; + header = head !== null && head.version === CACHE_VERSION && head.extractor === stamp; + if (!header) ok = false; return; } + if (line === "") return; + const parsed = JSON.parse(line) as { rel?: string; e?: ExtractEntry }; if (typeof parsed.rel !== "string" || !parsed.e || typeof parsed.e !== "object") { ok = false; return; } files[parsed.rel] = parsed.e; }); } catch { - ok = false; // a corrupt line: the whole memo is suspect + ok = false; // unreadable, or a corrupt line: the whole memo is suspect } - return ok ? { version: CACHE_VERSION, extractor: stamp, files } : emptyExtractCache(); + return ok && header ? { version: CACHE_VERSION, extractor: stamp, files } : emptyExtractCache(); } /** Best-effort write — a full graph is already on disk by the time this runs, so an @@ -228,7 +233,7 @@ export function readExtractCache(outDir: string): ExtractCache { * * NDJSON: a header line, then one line per entry. Each element is stringified on its * own, so the writer never builds a string larger than one entry, and the reader — via - * {@link forEachLine} — never decodes more than one line at a time. The atomic rename + * {@link forEachFileLine} — never decodes more than one line at a time. The atomic rename * means a half-written file is never visible under `path`, so it can never be read as a * valid (but truncated) cache. */ export function writeExtractCache(outDir: string, cache: ExtractCache): boolean { diff --git a/src/graph/fingerprint.ts b/src/graph/fingerprint.ts index f8fb7785a..b52a373a0 100644 --- a/src/graph/fingerprint.ts +++ b/src/graph/fingerprint.ts @@ -43,7 +43,7 @@ type Print = [number, number, string]; export interface Fingerprint { version: number; /** The extractor that produced the graph these prints describe — the same stamp - * `extract.json` carries. Without it the two sidecars can disagree about whether + * `extract..ndjson` carries. Without it the two sidecars can disagree about whether * the graph is current: an extractor change correctly drops every memo entry, * yet the prints still match the tree byte-for-byte, so the probe would report * clean and queries would keep answering from nodes the old extractor built. */ diff --git a/src/graph/parse-pool.ts b/src/graph/parse-pool.ts index d14cc60b6..fba4f376d 100644 --- a/src/graph/parse-pool.ts +++ b/src/graph/parse-pool.ts @@ -13,13 +13,24 @@ * - if no job ever succeeded, the crashes are environmental (fork, loader, * permissions), so the parked jobs are parsed in-thread with the rest. * A pool that cannot keep children alive hands the remaining queue back to the - * parent, which has warmed grammars. + * parent, which has warmed grammars; a queued job that already killed a child + * follows the parked jobs' rule. A child that dies before it ever signals + * `ready` is a warm-up failure, not a parse failure — in a broken environment + * (fork, loader, permissions) each such wave costs a full warm-up timeout with + * nothing to show — so it is charged two respawn credits instead of one, and the + * pool falls back to in-thread after half as many warm-up waves. A child whose + * WASM runtime was poisoned marks its result `retire`; the pool stops it and + * replaces it free of charge for the first `workers * 4` retirements, then for + * one respawn credit each. A child that exits because the pool told it to stop + * costs nothing. */ import { fork, type ChildProcess, type ForkOptions } from "node:child_process"; import { existsSync } from "node:fs"; import { availableParallelism, freemem } from "node:os"; import { fileURLToPath } from "node:url"; import { extractOne, type ExtractOneResult } from "./extract-one.js"; +import { contentHash } from "../util/id.js"; +import { readSourceFile } from "../util/source.js"; import type { SourceStat } from "./source-files.js"; import type { FromChild, ToChild } from "./parse-worker.js"; @@ -35,6 +46,9 @@ export interface PoolOptions { workers: number; entry?: string; onResult?: (index: number, file: string) => void; + /** Called once if the parent ends up parsing files itself (the pool could not + * keep children alive, or every crash looked environmental). */ + onFallback?: () => void; jobTimeoutMs?: number; warmTimeoutMs?: number; } @@ -71,6 +85,8 @@ export function poolSize( return n >= 2 ? n : 0; } +/** `exit` is how the job's last child died, in words ("exited (code 9)", + * "timed out after 120 s"); set on every death. */ interface Job { index: number; attempts: number; exit?: string } export function runParsePool( @@ -90,11 +106,17 @@ export function runParsePool( let inFlight = 0; let respawns = 0; const maxRespawns = opts.workers * 2; + // Retirements (a poisoned WASM runtime) are replaced free up to this many; past + // it each costs a respawn credit, so a grammar that aborts on a common construct + // ends in the in-thread fallback instead of a fork per file. + const freeRetirements = Math.max(1, opts.workers) * 4; + let retirements = 0; const children = new Set(); // Jobs that killed MAX_ATTEMPTS children: held here until the pool settles, when // `succeeded` decides error (poisoned file) vs in-thread parse (environmental). const parked: Job[] = []; let settled = false; + let fellBack = false; return new Promise((resolvePool) => { const killAll = (): void => { @@ -107,31 +129,52 @@ export function runParsePool( killAll(); resolvePool(results); }; + /** The parent parses a file itself; the caller hears about it once. */ + const parseHere = (job: Job): void => { + if (!fellBack) { fellBack = true; opts.onFallback?.(); } + const f = files[job.index]; + results[job.index] = extractOne(f, cachedHashOf(f.rel)); + }; + /** The `error` result for a job that killed a child and is not parsed here. + * Its entry keeps the file's real hash (read the way `extractOne` reads it; + * "" if unreadable), so the fingerprint keeps it on the stat fast path and the + * pre-query refresh does not re-parse it in-process. */ + const crashed = (job: Job, why: string): ExtractOneResult => { + const f = files[job.index]; + let hash = ""; + try { + const source = readSourceFile(f.abs); + if (source !== null) hash = contentHash(source); + } catch { /* unreadable: "" */ } + const message = `${f.rel}: parse failed — parser process ${job.exit ?? "exited (unknown)"} ${why}`; + return { kind: "error", message, entry: { size: f.size, mtimeMs: f.mtimeMs, hash, nodes: [], rawEdges: [], error: message } }; + }; /** Drain the parked jobs. `inThread` (i.e. `succeeded === 0`) means the crashes * were environmental, so parse them like the rest of the queue; otherwise they * are the brief's twice-crashed `error` result. */ const settleParked = (inThread: boolean): void => { if (settled) return; for (const job of parked.splice(0)) { - const f = files[job.index]; - if (inThread) { - results[job.index] = extractOne(f, cachedHashOf(f.rel)); - } else { - const message = `${f.rel}: parse failed — parser process exited (${job.exit ?? "unknown"}) on this file twice`; - results[job.index] = { kind: "error", message, entry: { size: f.size, mtimeMs: f.mtimeMs, hash: "", nodes: [], rawEdges: [], error: message } }; - } - opts.onResult?.(job.index, f.rel); + if (inThread) parseHere(job); + else results[job.index] = crashed(job, "on this file twice"); + opts.onResult?.(job.index, files[job.index].rel); outstanding--; } if (outstanding === 0) finish(); }; /** The pool cannot keep children alive: parse what is left here, in order, - * then settle the parked jobs (in-thread, since none ever succeeded). */ + * then settle the parked jobs. A queued job that already killed a child is + * held to the parked jobs' rule: once any job has completed in a child the + * crash is the file's, and parsing it here could take down the build. */ const fallbackInThread = (): void => { if (settled) return; killAll(); for (const job of queue.splice(0)) { - results[job.index] = extractOne(files[job.index], cachedHashOf(files[job.index].rel)); + if (job.exit !== undefined && succeeded > 0) { + results[job.index] = crashed(job, "on this file; not retried in-process after the pool gave up, where the same crash would end the build"); + } else { + parseHere(job); + } opts.onResult?.(job.index, files[job.index].rel); outstanding--; } @@ -146,12 +189,21 @@ export function runParsePool( if (parked.length > 0) settleParked(succeeded === 0); else if (outstanding === 0) finish(); }; + /** A child is gone and work remains: replace it within reason, charging + * `credits` against the budget. Past that the pool is not working on this + * machine, and once no child is left the parent finishes the job itself. */ + const replace = (credits: number): void => { + respawns += credits; + if (respawns <= maxRespawns) spawn(); + else if (children.size === 0) fallbackInThread(); + }; const post = (child: ChildProcess, msg: ToChild): boolean => { if (!child.connected) return false; try { child.send(msg); return true; } catch { return false; } }; const spawn = (): void => { + if (settled) return; // `windowsHide` is a real, effective fork option (forwarded to spawn) but // @types/node 26 lists it on CommonOptions, which ForkOptions does not // extend — hence the intersection rather than dropping the flag or `any`. @@ -160,10 +212,29 @@ export function runParsePool( serialization: "advanced", windowsHide: true, }; - const child = fork(entry, [], forkOpts); + let child: ChildProcess; + try { + child = fork(entry, [], forkOpts); + } catch { + // fork() can throw synchronously (an invalid path, some spawn errnos): + // a child that died before `ready`, holding no job. + if (queue.length === 0) drainIfIdle(); + else replace(2); + return; + } children.add(child); let current: Job | null = null; let ready = false; + // `onGone` is wired to both `exit` and `error` and is also called directly + // when the initial `post(init)` fails; the second firing for the same child + // must be a no-op, or a slot would be freed twice. + let gone = false; + // An exit the pool asked for: after `stop` with nothing left to give it, or + // after a `retire` (its WASM runtime is poisoned). It held no job and is not + // a failure. + let stopping = false; + let retiring = false; + let timedOut = false; let timer: NodeJS.Timeout | null = setTimeout(() => child.kill(), warmTimeoutMs); timer.unref(); const clearTimer = (): void => { if (timer) { clearTimeout(timer); timer = null; } }; @@ -172,6 +243,7 @@ export function runParsePool( const job = queue.shift(); if (!job) { current = null; + stopping = true; post(child, { type: "stop" }); drainIfIdle(); return; @@ -179,7 +251,7 @@ export function runParsePool( job.attempts++; current = job; inFlight++; - timer = setTimeout(() => child.kill(), jobTimeoutMs); + timer = setTimeout(() => { timedOut = true; child.kill(); }, jobTimeoutMs); timer.unref(); if (!post(child, { type: "job", seq: job.index, f: files[job.index], cachedHash: cachedHashOf(files[job.index].rel) })) { // Dead channel: the exit handler re-queues `current` and decrements inFlight. @@ -188,8 +260,16 @@ export function runParsePool( }; child.on("message", (m: FromChild) => { - if (settled) return; - if (m.type === "ready") { ready = true; clearTimer(); next(); return; } + // A message read after this child's exit or error must not hand a dead + // child a job; a repeated `ready` must not hand a busy one a second. + if (settled || gone) return; + if (m.type === "ready") { + if (ready) return; + ready = true; + clearTimer(); + next(); + return; + } if (m.type !== "done" || current === null || m.seq !== current.index) return; clearTimer(); succeeded++; @@ -197,39 +277,65 @@ export function runParsePool( opts.onResult?.(m.seq, files[m.seq].rel); current = null; inFlight--; + if (m.retire) { + // Its WASM runtime is poisoned: stop it (it waits for this rather than + // exiting by itself, so its `exit` can never overtake this `done`), and + // replace it in onGone. + retiring = true; + post(child, { type: "stop" }); + } if (--outstanding === 0) finish(); - else next(); + else if (!retiring) next(); }); const onGone = (code: number | null, signal: NodeJS.Signals | null): void => { + if (gone) return; + gone = true; clearTimer(); children.delete(child); + // After an `error` the process may still be running; it no longer counts. + try { child.kill(); } catch { /* already gone */ } if (settled) return; if (current !== null) { const job = current; current = null; inFlight--; + job.exit = timedOut ? `timed out after ${jobTimeoutMs / 1000} s` : `exited (${signal ?? `code ${code}`})`; if (job.attempts < MAX_ATTEMPTS) { queue.unshift(job); } else { // Twice-crashed: park it. Whether it is an error or parsed in-thread is // decided at settle time by `succeeded`, not here. - job.exit = signal ?? `code ${code}`; parked.push(job); } + } else if (retiring || stopping) { + if (retiring) retirements++; + if (queue.length === 0) drainIfIdle(); + // A retired child did its job: replace it outright, up to `freeRetirements`. + else if (retiring) { + if (retirements > freeRetirements) replace(1); + else spawn(); + } + // Another child's crash re-queued work after this one was told to stop: + // that crash already paid for a replacement, so step in, free of charge, + // only if no child is left to take the work. + else if (children.size === 0) replace(0); + return; } if (queue.length === 0) { drainIfIdle(); return; } - // Work remains. Replace the child within reason; past that, the pool is - // not working on this machine and the parent finishes the job itself. - if (respawns++ < maxRespawns) spawn(); - else if (children.size === 0) fallbackInThread(); - void ready; + // Work remains. A death before this child ever signalled `ready` is a + // warm-up failure, so it costs a second respawn credit: a pathological + // environment burns its budget in half as many warm-up waves before + // falling back to in-thread. + replace(ready ? 1 : 2); }; child.once("exit", onGone); - child.once("error", () => onGone(null, null)); // spawn failure: no exit follows + // `on`, not `once`: a second `error` (a failed kill or send) must still find + // a listener, or it is thrown in the parent. + child.on("error", () => onGone(null, null)); if (!post(child, init)) onGone(null, null); }; - for (let i = 0; i < Math.max(1, opts.workers); i++) spawn(); + for (let i = 0; i < Math.max(1, opts.workers) && !settled; i++) spawn(); }); } diff --git a/src/graph/parse-worker.ts b/src/graph/parse-worker.ts index ce1fe4bbb..ea11f7d7c 100644 --- a/src/graph/parse-worker.ts +++ b/src/graph/parse-worker.ts @@ -9,7 +9,7 @@ * for good) costs one file instead of the build. */ import { extractOne, type ExtractOneResult } from "./extract-one.js"; -import { warmGenericGrammars } from "./generic.js"; +import { warmGenericGrammars, wasmRuntimePoisoned } from "./generic.js"; import { warmContainerGrammars } from "./container.js"; import type { SourceStat } from "./source-files.js"; @@ -17,7 +17,11 @@ export type ToChild = | { type: "init"; generic: string[]; container: string[] } | { type: "job"; seq: number; f: SourceStat; cachedHash: string | null } | { type: "stop" }; -export type FromChild = { type: "ready" } | { type: "done"; seq: number; result: ExtractOneResult }; +/** `retire`: this child's WASM runtime is poisoned, so every later WASM parse in + * it would fail. The pool answers with `stop` and replaces it. The child never + * exits on its own: IPC is ordered, so the pool handles this `done` before the + * `exit` its `stop` causes, and cannot mistake the retirement for a crash. */ +export type FromChild = { type: "ready" } | { type: "done"; seq: number; result: ExtractOneResult; retire?: true }; function send(msg: FromChild): void { try { process.send?.(msg); } catch { /* parent gone; disconnect handler exits */ } @@ -29,7 +33,8 @@ if (process.send) { void Promise.all([warmGenericGrammars(m.generic), warmContainerGrammars(m.container)]) .then(() => send({ type: "ready" }), () => process.exit(3)); } else if (m.type === "job") { - send({ type: "done", seq: m.seq, result: extractOne(m.f, m.cachedHash) }); + const result = extractOne(m.f, m.cachedHash); + send(wasmRuntimePoisoned() === null ? { type: "done", seq: m.seq, result } : { type: "done", seq: m.seq, result, retire: true }); } else if (m.type === "stop") { process.exit(0); } diff --git a/src/graph/seed.ts b/src/graph/seed.ts index f134782ac..8d3538d47 100644 --- a/src/graph/seed.ts +++ b/src/graph/seed.ts @@ -141,6 +141,9 @@ function seedable(rel: string): boolean { if (rel === "" || rel === GRAPH_DIR || rel === CACHE_DIR) return true; if (!rel.startsWith(`${CACHE_DIR}/`)) return false; // incl. the graph: `installGraph` places it const name = rel.slice(CACHE_DIR.length + 1); + // `..tmp`: another writer's scratch file (openAtomic), possibly + // orphaned by a killed build: partial, and hundreds of MB on a large repo. + if (name.endsWith(".tmp")) return false; return ( name === ASK_INDEX_FILE || name.startsWith(`${EXTRACT_CACHE_PREFIX}.`) || diff --git a/src/graph/write.ts b/src/graph/write.ts index 5f4c27f57..37d89b36f 100644 --- a/src/graph/write.ts +++ b/src/graph/write.ts @@ -11,7 +11,7 @@ import { constants as bufferConstants } from "node:buffer"; import { readFileSync, statSync } from "node:fs"; import { join } from "node:path"; -import { forEachLine, openAtomic } from "../util/json-stream.js"; +import { forEachFileLine, forEachLine, openAtomic, type LineWalker } from "../util/json-stream.js"; import type { EdgeV1, GraphV1, NodeV1 } from "./types.js"; /** Hidden subdir under the context dir that holds machine-only graph artifacts. */ @@ -36,7 +36,8 @@ export function readGraph(path: string, opts: ReadGraphOptions = {}): GraphV1 | try { size = statSync(path).size; } catch { return null; } const cap = opts.maxStringLength ?? bufferConstants.MAX_STRING_LENGTH; if (size > cap) { - try { return readGraphFromBuffer(readFileSync(path)); } catch { return null; } + // Chunked: past 2 GiB the file is too large for one Buffer, let alone a string. + try { return readGraphFromLines((fn) => forEachFileLine(path, fn)); } catch { return null; } } try { return JSON.parse(readFileSync(path, "utf8")) as GraphV1; @@ -48,21 +49,30 @@ export function readGraph(path: string, opts: ReadGraphOptions = {}): GraphV1 | /** Parse the line-per-element shape `writeGraph` produces without ever holding * the whole file as one string. Null when the buffer is not in that shape. */ export function readGraphFromBuffer(buf: Buffer): GraphV1 | null { + return readGraphFromLines((fn) => forEachLine(buf, fn)); +} + +function readGraphFromLines(eachLine: LineWalker): GraphV1 | null { const NODES_OPEN = ',"nodes":['; const EDGES_OPEN = '],"edges":['; const END = "]}"; - let meta: GraphV1["meta"] | null = null; + let head: Record | null = null; const nodes: NodeV1[] = []; const edges: EdgeV1[] = []; let target: NodeV1[] | EdgeV1[] | null = null; let closed = false; let bad = false; - forEachLine(buf, (line, i) => { + eachLine((line, i) => { if (bad || closed) { if (line !== "") bad = true; return; } try { if (i === 0) { + // Parse the whole header object (as readAskIndexFromBuffer does) rather than + // slicing `meta` out by fixed offsets: strip the trailing `,"nodes":[`, close + // the object, and JSON.parse it. This tolerates any top-level key that precedes + // `nodes` — not just `meta` — so a future header field round-trips instead of + // failing a slice that assumed `meta` was the only key before `nodes`. if (!line.startsWith('{"meta":') || !line.endsWith(NODES_OPEN)) { bad = true; return; } - meta = JSON.parse(line.slice('{"meta":'.length, line.length - NODES_OPEN.length)) as GraphV1["meta"]; + head = JSON.parse(line.slice(0, -NODES_OPEN.length) + "}") as Record; target = nodes; return; } @@ -75,11 +85,24 @@ export function readGraphFromBuffer(buf: Buffer): GraphV1 | null { bad = true; } }); - if (bad || !closed || meta === null) return null; - return { meta, nodes, edges }; + if (bad || !closed || head === null) return null; + const { meta, ...rest } = head as Record; + if (meta === undefined || meta === null) return null; + // Spread any other top-level header keys (there are none today) alongside meta so + // the reader stays forward-compatible with whatever writeGraph writes next. + return { ...rest, meta, nodes, edges } as GraphV1; } export function writeGraph(graph: GraphV1, outDir: string): string { + // The `head` slice below assumes the serialized object is exactly + // `{...meta...,"nodes":[],"edges":[]}` — meta the only key before `nodes`. A + // future top-level key would land between them and silently corrupt that slice, + // so refuse it here (naming it) rather than write a broken file. + for (const key of Object.keys(graph)) { + if (key !== "meta" && key !== "nodes" && key !== "edges") { + throw new Error(`writeGraph: unexpected top-level graph key ${JSON.stringify(key)} (only meta, nodes, edges are serialized)`); + } + } const nodes = [...graph.nodes].sort((a, b) => a.id.localeCompare(b.id)); const edges = [...graph.edges].sort(edgeOrder); const path = wiringPath(outDir); diff --git a/src/util/json-stream.ts b/src/util/json-stream.ts index c30f15a8e..d01f766a5 100644 --- a/src/util/json-stream.ts +++ b/src/util/json-stream.ts @@ -8,7 +8,7 @@ * did get written threw `ERR_STRING_TOO_LONG` on read. The writers here take * one element at a time; the reader decodes one line at a time. */ -import { closeSync, mkdirSync, openSync, renameSync, rmSync, writeSync } from "node:fs"; +import { closeSync, mkdirSync, openSync, readSync, renameSync, rmSync, writeSync } from "node:fs"; import { dirname } from "node:path"; export interface AtomicWriter { @@ -52,6 +52,16 @@ export function openAtomic(path: string): AtomicWriter { }; } +/** Decode `buf[start, end)` as one line, dropping a trailing "\r" (a CRLF file). */ +function decodeLine(buf: Buffer, start: number, end: number): string { + if (end > start && buf[end - 1] === 0x0d) end--; + return buf.toString("utf8", start, end); +} + +/** Feeds `fn` each line of some source, in order: {@link forEachLine} over a + * Buffer, {@link forEachFileLine} over a file. */ +export type LineWalker = (fn: (line: string, index: number) => void) => void; + /** Call `fn` for each line of `buf`. A trailing line without "\n" is still a line; * an empty buffer yields none. Each line is decoded on its own, so the Buffer may * be larger than any string V8 can hold. */ @@ -61,7 +71,49 @@ export function forEachLine(buf: Buffer, fn: (line: string, index: number) => vo while (start < buf.length) { let end = buf.indexOf(0x0a, start); if (end === -1) end = buf.length; - fn(buf.toString("utf8", start, end), index++); + fn(decodeLine(buf, start, end), index++); start = end + 1; } } + +const LINE_CHUNK_BYTES = 32 * 1024 * 1024; + +/** {@link forEachLine} over a file, read `chunkBytes` at a time: the file is never + * one Buffer either, so it may exceed Node's 2 GiB `readFileSync` limit + * (`ERR_FS_FILE_TOO_LARGE`). A line split across chunks is carried over and + * joined before it is decoded, so a multibyte character is never cut. */ +export function forEachFileLine( + path: string, + fn: (line: string, index: number) => void, + chunkBytes = LINE_CHUNK_BYTES, +): void { + const fd = openSync(path, "r"); + try { + const chunk = Buffer.allocUnsafe(chunkBytes); + let carry: Buffer[] = []; // the current line's bytes from earlier chunks + let index = 0; + const emit = (tail: Buffer): void => { + const line = carry.length === 0 ? tail : Buffer.concat([...carry, tail]); + carry = []; + fn(decodeLine(line, 0, line.length), index++); + }; + for (;;) { + const n = readSync(fd, chunk, 0, chunkBytes, null); + if (n === 0) break; + let start = 0; + for (;;) { + const nl = chunk.indexOf(0x0a, start); + if (nl === -1 || nl >= n) { + // `chunk` is reused by the next read: keep a copy of the partial line. + if (start < n) carry.push(Buffer.from(chunk.subarray(start, n))); + break; + } + emit(chunk.subarray(start, nl)); + start = nl + 1; + } + } + if (carry.length > 0) emit(Buffer.alloc(0)); + } finally { + closeSync(fd); + } +} diff --git a/test/claude-hooks.test.ts b/test/claude-hooks.test.ts index 91d240747..e0520014e 100644 --- a/test/claude-hooks.test.ts +++ b/test/claude-hooks.test.ts @@ -123,6 +123,31 @@ test("runSync's default build passes --dir to graft build when GRAFT_ } }); +test("runSync's default build pins --workers 0 so a background sync never forks, whatever GRAFT_PARSE_WORKERS says", () => { + const d = mkdtempSync(join(tmpdir(), 'graft-sync-workers-')); + mkdirSync(join(d, 'graft', '.graph'), { recursive: true }); + const argsFile = join(d, 'args-seen.json'); + const stub = join(d, 'build-stub.cjs'); + writeFileSync( + stub, + `const fs = require('fs');\n` + + `fs.writeFileSync(${JSON.stringify(argsFile)}, JSON.stringify(process.argv.slice(2)));\n` + + `fs.writeFileSync(${JSON.stringify(join(d, 'graft', '.graph', 'wiring.json'))}, JSON.stringify({ meta: { nodeCount: 0, edgeCount: 0, languages: [] }, nodes: [], edges: [] }));\n`, + ); + const prev = process.env.GRAFT_PARSE_WORKERS; + process.env.GRAFT_TEST_CLI = stub; + process.env.GRAFT_PARSE_WORKERS = 'auto'; + try { + runSync(d); + const argsSeen: string[] = JSON.parse(readFileSync(argsFile, 'utf8')); + // The CLI flag outranks the env var (and a repo .env the child's dotenv loads). + assert.deepEqual(argsSeen, ['build', '.', '--workers', '0']); + } finally { + delete process.env.GRAFT_TEST_CLI; + if (prev === undefined) delete process.env.GRAFT_PARSE_WORKERS; else process.env.GRAFT_PARSE_WORKERS = prev; + } +}); + test('runSync clears syncing even if build throws (money-safe failure)', () => { const d = mkdtempSync(join(tmpdir(), 'graft-sync-')); writeStats(d, { ...emptyStats(), dirty: true, syncing: true }); diff --git a/test/claude-stats.test.ts b/test/claude-stats.test.ts index 6b444a0b5..14b4882cf 100644 --- a/test/claude-stats.test.ts +++ b/test/claude-stats.test.ts @@ -1,9 +1,10 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { computeStats, readWiring } from '../src/claude/stats.js'; +import { wiringPath, writeGraph } from '../src/graph/write.js'; const wiring = { meta: { version: 1, nodeCount: 3, edgeCount: 2, languages: ['typescript'] }, @@ -42,3 +43,20 @@ test('readWiring reads from a GRAFT_DIR-relocated context dir instead of { + const d = mkdtempSync(join(tmpdir(), 'graft-stats-big-')); + const out = join(d, 'graft'); + const graph = { + meta: { version: 1, nodeCount: 1, edgeCount: 0, languages: ['typescript'] }, + nodes: [{ id: 'a', name: 'a', kind: 'function', path: 'a.ts', span: 'L1-L1', summary_state: 'ready' }], + edges: [], + } as any; + writeGraph(graph, out); // line-shaped, as every build writes it + assert.deepEqual(readWiring(d, { maxStringLength: 16 }), JSON.parse(readFileSync(wiringPath(out), 'utf8'))); + // A one-line file over the cap is past what JSON.parse could take in one string: + // only the line-shaped form is readable there, which proves which path ran. + writeFileSync(wiringPath(out), JSON.stringify(graph)); + assert.equal(readWiring(d, { maxStringLength: 16 }), null); + assert.equal(readWiring(join(d, 'missing')), null, 'no graph is still null'); +}); diff --git a/test/graph-artifacts-stream.test.ts b/test/graph-artifacts-stream.test.ts index 046ac02e0..f6d64a207 100644 --- a/test/graph-artifacts-stream.test.ts +++ b/test/graph-artifacts-stream.test.ts @@ -16,10 +16,40 @@ test("extract cache round-trips through NDJSON, including quotes and newlines in assert.ok(writeExtractCache(d, { ...emptyExtractCache(), files: FILES })); assert.ok(extractCachePath(d)!.endsWith(".ndjson")); const back = readExtractCache(d); - assert.deepEqual(back.files, FILES); + assert.deepEqual({ ...back.files }, FILES); // spread: the reader's map has a null prototype assert.equal(back.version, 2); }); +test("extract cache: a blank first line fails the header check, so the entries after it are not trusted", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + writeExtractCache(d, { ...emptyExtractCache(), files: FILES }); + const p = extractCachePath(d)!; + const lines = readFileSync(p, "utf8").split("\n"); + writeFileSync(p, ["", ...lines.slice(1)].join("\n")); + assert.deepEqual(readExtractCache(d).files, {}); +}); + +test("extract cache: a rel of __proto__ is an ordinary key, not the prototype setter", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + writeExtractCache(d, emptyExtractCache()); + const p = extractCachePath(d)!; + const e: ExtractEntry = { size: 1, mtimeMs: 1, hash: "hp", nodes: [], rawEdges: [] }; + writeFileSync(p, readFileSync(p, "utf8") + JSON.stringify({ rel: "__proto__", e }) + "\n"); + const files = readExtractCache(d).files; + assert.ok(Object.prototype.hasOwnProperty.call(files, "__proto__"), "stored as its own key"); + assert.deepEqual({ ...files["__proto__"] }, e); + assert.equal(Object.getPrototypeOf(files), null, "no prototype for a crafted rel to reach"); + assert.equal(files["toString"], undefined, "an inherited name is not a cache hit"); +}); + +test("extract cache: a CRLF file reads back the same", () => { + const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); + writeExtractCache(d, { ...emptyExtractCache(), files: { "src/a.ts": FILES["src/a.ts"] } }); + const p = extractCachePath(d)!; + writeFileSync(p, readFileSync(p, "utf8").replace(/\n/g, "\r\n")); + assert.deepEqual({ ...readExtractCache(d).files }, { "src/a.ts": FILES["src/a.ts"] }); +}); + test("extract cache: one line per entry, header first", () => { const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); writeExtractCache(d, { ...emptyExtractCache(), files: FILES }); @@ -43,11 +73,11 @@ test("extract cache: a wrong version, a wrong stamp, or a corrupt line means an test("extract cache: an empty file set still writes a valid header the reader accepts", () => { const d = mkdtempSync(join(tmpdir(), "graft-ndjson-")); assert.ok(writeExtractCache(d, emptyExtractCache())); - assert.deepEqual(readExtractCache(d).files, {}); + assert.deepEqual({ ...readExtractCache(d).files }, {}); }); import { readGraph, wiringPath, writeGraph } from "../src/graph/write.js"; -import { askIndexPath, readAskIndex, writeAskIndex } from "../src/ask/index-file.js"; +import { askIndexPath, readAskIndex, writeAskIndex, type AskIndex } from "../src/ask/index-file.js"; import type { GraphV1, NodeV1 } from "../src/graph/types.js"; function node(id: string, extra: Partial = {}): NodeV1 { @@ -78,13 +108,43 @@ test("writeGraph with no nodes and no edges is still valid", () => { assert.deepEqual(readGraph(wiringPath(d))!.nodes, []); }); -test("writeAskIndex streams a document equal to the one-shot JSON.stringify", () => { +/** The AskIndex GRAPH must produce, written out by hand: one-letter names and the + * "a" of "a.ts" are dropped by tokenize, as is "on" (a stop word). */ +const GRAPH_ASK_INDEX: AskIndex = { + version: 1, + avgBodyLen: 1, + df: [["ts", 2], ["dropped", 1], ["disk", 1]], + docCount: 2, + docs: [ + { id: "a", name: [], path: [["ts", 1]], body: [] }, + { id: "b", name: [], path: [["ts", 1]], body: [["dropped", 1], ["disk", 1]] }, + ], +}; + +/** JSON.stringify of an AskIndex with a newline before every doc and the close. */ +function askLineShaped(idx: AskIndex): string { + const { docs, ...head } = idx; + return `${JSON.stringify(head).slice(0, -1)},"docs":[${docs.map((doc) => "\n" + JSON.stringify(doc)).join(",")}\n]}\n`; +} + +test("writeAskIndex writes exactly the line-shaped JSON.stringify of the index it should build", () => { const d = mkdtempSync(join(tmpdir(), "graft-askidx-")); writeAskIndex(d, GRAPH); const text = readFileSync(askIndexPath(d), "utf8"); - const back = readAskIndex(d)!; - assert.deepEqual(JSON.parse(text), back); - assert.equal(back.docCount, 2); + assert.equal(text, askLineShaped(GRAPH_ASK_INDEX)); + assert.deepEqual(JSON.parse(text), JSON.parse(JSON.stringify(GRAPH_ASK_INDEX)), "and it is that object as JSON"); + assert.deepEqual(readAskIndex(d), GRAPH_ASK_INDEX); +}); + +test("readGraph and readAskIndex stream a CRLF file over the cap", () => { + const d = mkdtempSync(join(tmpdir(), "graft-crlf-")); + writeGraph(GRAPH, d); + writeAskIndex(d, GRAPH); + const graphText = readFileSync(wiringPath(d), "utf8"); + writeFileSync(wiringPath(d), graphText.replace(/\n/g, "\r\n")); + writeFileSync(askIndexPath(d), readFileSync(askIndexPath(d), "utf8").replace(/\n/g, "\r\n")); + assert.deepEqual(readGraph(wiringPath(d), { maxStringLength: 16 }), JSON.parse(graphText)); + assert.deepEqual(readAskIndex(d, { maxStringLength: 16 }), GRAPH_ASK_INDEX); }); import { readGraphFromBuffer } from "../src/graph/write.js"; @@ -122,6 +182,26 @@ test("readGraph streaming returns null on a file that is not line-shaped", () => assert.equal(readGraphFromBuffer(Buffer.from("garbage\n")), null); }); +test("readGraphFromBuffer tolerates a top-level header key other than meta before nodes", () => { + // A hand-written header carrying an extra key ahead of "nodes": the reader must + // parse the whole header object rather than slice `meta` out by fixed offsets, so + // the extra field survives on the result instead of failing the parse. + const meta = JSON.stringify(GRAPH.meta); + const buf = Buffer.from(`{"meta":${meta},"extra":1,"nodes":[\n],"edges":[\n]}\n`); + const g = readGraphFromBuffer(buf); + assert.ok(g, "parsed the hand-written header"); + assert.deepEqual(g!.meta, GRAPH.meta); + assert.equal((g as unknown as { extra: number }).extra, 1, "the extra key survives on the result"); + assert.deepEqual(g!.nodes, []); + assert.deepEqual(g!.edges, []); +}); + +test("writeGraph refuses a graph carrying a top-level key other than meta/nodes/edges", () => { + const d = mkdtempSync(join(tmpdir(), "graft-wiring-guard-")); + const rogue = { ...GRAPH, extra: 1 } as unknown as GraphV1; + assert.throws(() => writeGraph(rogue, d), /extra/, "the error names the offending key so it can't silently corrupt the head slice"); +}); + test("writeAskIndex writes one doc per line, still valid JSON, and streams back identically", () => { const d = mkdtempSync(join(tmpdir(), "graft-askidx-lines-")); writeAskIndex(d, GRAPH); diff --git a/test/graph-build-workers-cli.test.ts b/test/graph-build-workers-cli.test.ts new file mode 100644 index 000000000..0a5ebaf37 --- /dev/null +++ b/test/graph-build-workers-cli.test.ts @@ -0,0 +1,95 @@ +/** + * `graft build --workers` / `GRAFT_PARSE_WORKERS` through the real CLI: a value + * that is neither `auto` nor a non-negative integer is rejected up front (as + * `--concurrency` is), and a workspace build, which does not use the pool, says + * so instead of silently ignoring the request. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, existsSync, rmSync } from "node:fs"; +import { availableParallelism, tmpdir } from "node:os"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; +import { buildGraph } from "../src/graph/build.js"; + +function repo(): string { + const d = mkdtempSync(join(tmpdir(), "graft-workers-cli-")); + writeFileSync(join(d, "main.ts"), "export function main(): number {\n return 2;\n}\n"); + return d; +} + +function runCliCapture(args: string[], env: NodeJS.ProcessEnv = {}): { stdout: string; stderr: string; status: number } { + const base = { ...process.env }; + delete base.GRAFT_PARSE_WORKERS; + // spawnSync, not execFileSync: a successful run's stderr carries the warnings. + const r = spawnSync(process.execPath, ["--import", "tsx", "src/cli.ts", ...args], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + env: { ...base, ...env }, + }); + return { stdout: r.stdout ?? "", stderr: r.stderr ?? "", status: r.status ?? 1 }; +} + +for (const bad of ["abc", "-2", "4abc"]) { + test(`--workers rejects "${bad}" and exits 1 before building`, () => { + const d = repo(); + try { + const r = runCliCapture(["build", d, "--workers", bad]); + assert.equal(r.status, 1, `expected exit 1, got ${r.status}\nstdout: ${r.stdout}\nstderr: ${r.stderr}`); + assert.ok(r.stderr.includes(`✗ --workers must be a non-negative integer or "auto", got "${bad}"`), r.stderr); + assert.equal(existsSync(join(d, "graft")), false, "nothing was built"); + } finally { + rmSync(d, { recursive: true, force: true }); + } + }); +} + +test("GRAFT_PARSE_WORKERS with an invalid value is rejected the same way", () => { + const d = repo(); + try { + const r = runCliCapture(["build", d], { GRAFT_PARSE_WORKERS: "4abc" }); + assert.equal(r.status, 1, `expected exit 1, got ${r.status}\nstdout: ${r.stdout}\nstderr: ${r.stderr}`); + assert.ok(r.stderr.includes(`✗ GRAFT_PARSE_WORKERS must be a non-negative integer or "auto", got "4abc"`), r.stderr); + } finally { + rmSync(d, { recursive: true, force: true }); + } +}); + +test("--workers 0 and --workers auto are accepted", () => { + const d = repo(); + try { + for (const ok of ["0", "auto"]) { + const r = runCliCapture(["build", d, "--workers", ok]); + assert.equal(r.status, 0, `--workers ${ok}: ${r.stderr}`); + } + } finally { + rmSync(d, { recursive: true, force: true }); + } +}); + +test("a workspace build warns once that --workers is ignored", () => { + const parent = mkdtempSync(join(tmpdir(), "graft-workers-ws-")); + try { + for (const child of ["repoA", "repoB"]) { + mkdirSync(join(parent, child, ".git"), { recursive: true }); + writeFileSync(join(parent, child, "main.ts"), "export function main(): number {\n return 2;\n}\n"); + } + const r = runCliCapture(["build", parent, "--workers", "2"]); + assert.equal(r.status, 0, r.stderr); + const warnings = r.stderr.split("\n").filter((l) => l.includes("--workers") && l.includes("workspace")); + assert.equal(warnings.length, 1, r.stderr); + } finally { + rmSync(parent, { recursive: true, force: true }); + } +}); + +test("buildGraph clamps an explicit worker count to the cores and the file count", async () => { + const d = repo(); + try { + const r = await buildGraph(d, { reuse: false, parseWorkers: 500 }); + assert.equal(r.files, 1); + assert.equal(r.parseWorkers, Math.min(1, availableParallelism()), "one file never forks more than one child"); + } finally { + rmSync(d, { recursive: true, force: true }); + } +}); diff --git a/test/graph-enrich-failures.test.ts b/test/graph-enrich-failures.test.ts index 378433331..a767e99d2 100644 --- a/test/graph-enrich-failures.test.ts +++ b/test/graph-enrich-failures.test.ts @@ -120,3 +120,15 @@ test("#127: a clean pass reports no failure at all — the flags are not noise", assert.equal(stats.skippedFiles, 0); assert.equal(stats.computed, 4); }); + +test("a stopped pass does not read (and hash) the sources of the files it skips", async () => { + const { nodes, sources } = fixture(10); + const reads: string[] = []; + const lookup = { has: (p: string) => sources.has(p), get: (p: string) => { reads.push(p); return sources.get(p); } }; + const stats = await enrichGraph(nodes, new Map(), lookup, { + summarizer: new FailingCrux("429 Request exceeds your current quota, please check your plan"), + concurrency: 1, + }); + assert.equal(stats.skippedFiles, 9); + assert.deepEqual(reads, ["f0.ts"], "only the file that was actually attempted"); +}); diff --git a/test/graph-incremental.test.ts b/test/graph-incremental.test.ts index 0a814d648..fe5da160e 100644 --- a/test/graph-incremental.test.ts +++ b/test/graph-incremental.test.ts @@ -412,8 +412,14 @@ test("with no extractor identity, nothing is memoized — and 'unknown' is never // failure sentinel AND a value written into the files, so every later run compared // equal to it and extractor-change invalidation silently stopped working forever. for (const f of readdirSync(join(outOf(d), ".cache"))) { - if (!f.endsWith(".json")) continue; + if (!f.endsWith(".json") && !f.endsWith(".ndjson")) continue; assert.ok(!f.includes("unknown"), `${f} is filed under the failure sentinel`); + if (f.endsWith(".ndjson")) { + // The extract memo is NDJSON — its extractor identity is on the header line, not + // parseable as whole-file JSON — so read it back through readExtractCache. + assert.notEqual(readExtractCache(outOf(d)).extractor, "unknown", `${f} stores the sentinel`); + continue; + } const body = JSON.parse(readFileSync(join(outOf(d), ".cache", f), "utf8")) as { extractor?: string }; if (body.extractor !== undefined) assert.notEqual(body.extractor, "unknown", `${f} stores the sentinel`); } diff --git a/test/graph-load.test.ts b/test/graph-load.test.ts index 8b84e86eb..d17a942d6 100644 --- a/test/graph-load.test.ts +++ b/test/graph-load.test.ts @@ -41,6 +41,12 @@ function fixtureDir(): string { return mkdtempSync(join(tmpdir(), "graft-load-")); } +/** A well-formed GraphV1 for these fixtures: the version lives under `meta`, never + * as a stray top-level key (which `writeGraph` now rejects). */ +function graphOf(nodes: NodeV1[]): GraphV1 { + return { meta: { version: 1, nodeCount: nodes.length, edgeCount: 0, languages: [] }, nodes, edges: [] }; +} + /** Force a distinct (mtimeMs, size) from the previous write, even on * filesystems with coarse mtime resolution. */ function bump(path: string): void { @@ -51,7 +57,7 @@ function bump(path: string): void { test("loadGraphCached: two loads of an unchanged file return the same parsed object", () => { const dir = fixtureDir(); - writeGraph({ version: 1, nodes: [node("a")], edges: [] } as GraphV1, dir); + writeGraph(graphOf([node("a")]), dir); __resetParseCounts(); const g1 = loadGraphCached(dir); @@ -63,7 +69,7 @@ test("loadGraphCached: two loads of an unchanged file return the same parsed obj test("loadGraphCached: rewriting the wiring file invalidates the cache", () => { const dir = fixtureDir(); - writeGraph({ version: 1, nodes: [node("a")], edges: [] } as GraphV1, dir); + writeGraph(graphOf([node("a")]), dir); __resetParseCounts(); const g1 = loadGraphCached(dir); @@ -72,7 +78,7 @@ test("loadGraphCached: rewriting the wiring file invalidates the cache", () => { // Rewrite with a changed node, then force a distinct mtime so the cache // can't coast on a filesystem with coarse mtime granularity. - writeGraph({ version: 1, nodes: [node("b")], edges: [] } as GraphV1, dir); + writeGraph(graphOf([node("b")]), dir); bump(wiringPath(dir)); const g2 = loadGraphCached(dir); @@ -92,7 +98,7 @@ test("loadGraphCached: missing file returns null and a later-created file is pic const missAgain = loadGraphCached(dir); assert.equal(missAgain, null, "missing file must not be negatively cached forever"); - writeGraph({ version: 1, nodes: [node("late")], edges: [] } as GraphV1, dir); + writeGraph(graphOf([node("late")]), dir); const found = loadGraphCached(dir); assert.equal(found?.nodes[0]?.id, "late"); assert.equal(__parseCount.graph, 1); @@ -100,7 +106,7 @@ test("loadGraphCached: missing file returns null and a later-created file is pic test("loadAskIndexCached: caches and invalidates the same way as the graph loader", () => { const dir = fixtureDir(); - const graph = { version: 1, nodes: [node("a"), node("b")], edges: [] } as GraphV1; + const graph = graphOf([node("a"), node("b")]); writeAskIndex(dir, graph); __resetParseCounts(); @@ -110,7 +116,7 @@ test("loadAskIndexCached: caches and invalidates the same way as the graph loade assert.strictEqual(i1, i2); assert.equal(__parseCount.askIndex, 1); - writeAskIndex(dir, { version: 1, nodes: [node("a")], edges: [] } as GraphV1); + writeAskIndex(dir, graphOf([node("a")])); bump(askIndexPath(dir)); const i3 = loadAskIndexCached(dir); @@ -128,7 +134,7 @@ test("callTool: graft_trace_calls on the same dir twice doesn't reparse the grap mkdirSync(join(dir, "src"), { recursive: true }); writeFileSync(join(dir, "src", "math.ts"), "export function add() { return 1; }\n"); const graph: GraphV1 = { - version: 1, + meta: { version: 1, nodeCount: 2, edgeCount: 0, languages: ["typescript"] }, // A real `buildGraph` always emits a `kind: "file"` node per source file; // include one here too so `graft_trace_calls` with depth (a `resolveSymbol` + // `edgeWalk` walk, the old `graft impact`) can resolve the filename-shaped @@ -138,7 +144,7 @@ test("callTool: graft_trace_calls on the same dir twice doesn't reparse the grap { ...node("src/math.ts"), kind: "file", path: "src/math.ts", name: "math.ts", signature: null }, ], edges: [], - } as GraphV1; + }; // graft_trace_calls reads through `contextDirFor(root)`, i.e. `/graft` // by default — write the graph there directly rather than round-tripping // through a real `graft build`. diff --git a/test/graph-parse-pool.test.ts b/test/graph-parse-pool.test.ts index f2655209b..6c056d8b6 100644 --- a/test/graph-parse-pool.test.ts +++ b/test/graph-parse-pool.test.ts @@ -1,7 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { mkdtempSync, mkdirSync, writeFileSync, readFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { availableParallelism, tmpdir } from "node:os"; import { join } from "node:path"; import { extractOne } from "../src/graph/extract-one.js"; import { warmGenericGrammars } from "../src/graph/generic.js"; @@ -76,10 +76,16 @@ test("a --deep build summarizes from disk without buildGraph holding every sourc test("a file edited between parse and summary is left pending, not summarized against the wrong lines", async () => { const d = repo(); + // With concurrency 1 the enrich pass visits files in dirty-node order, which is + // `listSourceStats` order: src/lib.rs first, then src/math.ts. On the FIRST + // describeFile call (src/lib.rs) we edit the file summarized SECOND (src/math.ts) + // so its on-disk bytes no longer hash to what was parsed. buildGraph's + // SourceLookup.get re-hashes before handing over the source and returns undefined + // on a mismatch, so src/math.ts is left pending — never summarized against changed + // bytes. Drop that hash check and math.ts goes "ready" here, which is the bug. let edited = false; const crux: CruxSummarizer = { async describeFile(input) { - // Simulate an agent editing another file while this one is summarized. if (!edited) { edited = true; writeFileSync(join(d, "src", "math.ts"), "export function add(a: number, b: number): number {\n return a + b + 0;\n}\n"); } return input.nodes.map((n) => ({ id: n.id, summary: `crux ${n.id}`, crux_start: 0, crux_end: 0 })); }, @@ -87,11 +93,16 @@ test("a file edited between parse and summary is left pending, not summarized ag const r = await buildGraph(d, { reuse: false, summarizer: crux, concurrency: 1 }); assert.equal(r.errors.length, 0, r.errors.join("; ")); const g = readGraph(wiringPath(contextDirFor(d)))!; - const states = new Set(g.nodes.filter((n) => n.path === "src/math.ts" || n.path === "src/lib.rs").map((n) => n.summary_state)); - // Whichever file was summarized first is ready; the edited one may be pending. - // What must never happen is a "ready" summary computed against changed bytes. - assert.ok(states.has("ready")); - assert.ok(r.meaning.pending + r.meaning.computed >= 1); + const first = g.nodes.filter((n) => n.path === "src/lib.rs"); + const second = g.nodes.filter((n) => n.path === "src/math.ts"); + assert.ok(first.length > 0, "src/lib.rs contributed nodes"); + assert.ok(second.length > 0, "src/math.ts contributed nodes"); + // The first file was summarized against the exact bytes it was parsed from. + for (const n of first) assert.equal(n.summary_state, "ready", `${n.id} should be ready`); + // The second file changed on disk between parse and summary; every one of its + // nodes must be left pending — never a "ready" summary computed against the + // wrong lines. This is what fails if the hash check in SourceLookup.get is gone. + for (const n of second) assert.notEqual(n.summary_state, "ready", `${n.id} must not be ready against changed bytes`); }); import { existsSync } from "node:fs"; @@ -131,26 +142,200 @@ test("runParsePool honours cachedHash and reports reused", async () => { assert.deepEqual(pooled[files.indexOf(rs)], { kind: "reused", hash: contentHash(RS) }); }); -/** A child entry that behaves like parse-worker.ts except where `mode` says otherwise. */ -function fakeWorker(dir: string, mode: "crash-on-boom" | "never-ready" | "always-crash"): string { +/** A child entry that behaves like parse-worker.ts except where `mode` says otherwise. + * `slow-stop` logs each child's start to `spawned.log`, warms slowly, crashes 500 ms + * into a boom job, and lingers 1200 ms after `stop` before its clean exit. + * `retire-waits` marks every result `retire` and then, like parse-worker.ts, exits + * only when the pool sends `stop`. */ +function fakeWorker(dir: string, mode: "crash-on-boom" | "never-ready" | "always-crash" | "hang-on-boom" | "slow-stop" | "retire-waits"): string { const entry = join(dir, `worker-${mode}.mjs`); const src = pathToFileURL(join(process.cwd(), "src", "graph")).href; writeFileSync(entry, [ + `import { appendFileSync } from "node:fs";`, `import { extractOne } from ${JSON.stringify(src + "/extract-one.ts")};`, `import { warmGenericGrammars } from ${JSON.stringify(src + "/generic.ts")};`, `const mode = ${JSON.stringify(mode)};`, + `if (mode === 'slow-stop') appendFileSync(${JSON.stringify(join(dir, "spawned.log"))}, process.pid + "\\n");`, + "const sleep = (ms) => new Promise((r) => setTimeout(r, ms));", "process.on('message', async (m) => {", - " if (m.type === 'init') { if (mode === 'never-ready') return; await warmGenericGrammars(m.generic); process.send({ type: 'ready' }); }", - " else if (m.type === 'job') {", - " if (mode === 'always-crash' || (mode === 'crash-on-boom' && m.f.rel.endsWith('boom.rs'))) process.exit(9);", - " process.send({ type: 'done', seq: m.seq, result: extractOne(m.f, m.cachedHash) });", - " } else if (m.type === 'stop') process.exit(0);", + " if (m.type === 'init') {", + " if (mode === 'never-ready') return;", + " await warmGenericGrammars(m.generic);", + " if (mode === 'slow-stop') await sleep(1500);", + " process.send({ type: 'ready' });", + " } else if (m.type === 'job') {", + " const boom = m.f.rel.endsWith('boom.rs');", + " if (mode === 'always-crash' || (mode === 'crash-on-boom' && boom)) process.exit(9);", + " if (mode === 'hang-on-boom' && boom) return;", + " if (mode === 'slow-stop' && boom) { setTimeout(() => process.exit(9), 500); return; }", + " const done = { type: 'done', seq: m.seq, result: extractOne(m.f, m.cachedHash) };", + " if (mode === 'retire-waits') done.retire = true;", + " process.send(done);", + " } else if (m.type === 'stop') {", + " if (mode === 'slow-stop') setTimeout(() => process.exit(0), 1200); else process.exit(0);", + " }", "});", "process.on('disconnect', () => process.exit(0));", ].join("\n")); return entry; } +/** The real parse-worker.ts, with a listener registered ahead of its own that + * swaps the rust grammar for one that aborts like a real Emscripten abort when a + * `*boom.rs` job arrives — so the child's WASM runtime is poisoned for real. */ +function poisoningWorker(dir: string): string { + const src = pathToFileURL(join(process.cwd(), "src", "graph")).href; + const swap = join(dir, "swap-on-boom.mjs"); + writeFileSync(swap, [ + `import { swapGrammarForTest } from ${JSON.stringify(src + "/generic.ts")};`, + "process.on('message', (m) => {", + " if (m.type !== 'job' || !m.f.rel.endsWith('boom.rs')) return;", + " const real = swapGrammarForTest('rust', null);", + " swapGrammarForTest('rust', { language: real.language, query: real.query,", + " parser: { parse() { throw new WebAssembly.RuntimeError('Aborted(). test abort'); }, reset() {}, delete() {} } });", + "});", + ].join("\n")); + const entry = join(dir, "worker-poisoning.mjs"); + writeFileSync(entry, [ + `import ${JSON.stringify(pathToFileURL(swap).href)};`, + `import ${JSON.stringify(src + "/parse-worker.ts")};`, + ].join("\n")); + return entry; +} + +const byRel = (files: ReturnType) => files.sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); + +test("runParsePool: a child whose WASM runtime is poisoned is stopped after its result and replaced free of charge", async () => { + const d = repo(); + for (const n of [1, 2, 3]) writeFileSync(join(d, "src", `a${n}boom.rs`), `pub fn b${n}() {}\n`); + for (let i = 0; i < 4; i++) writeFileSync(join(d, "src", `z${i}.rs`), `pub fn z${i}() {}\n`); + await warmGenericGrammars(["rust"]); + const files = byRel(listSourceStats(d, join(d, "graft"))); + let fellBack = false; + // One worker, three poisoning files: three retirements against a respawn budget of 2. + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { + workers: 1, entry: poisoningWorker(d), onFallback: () => { fellBack = true; }, + }); + files.forEach((f, i) => { + const r = results[i]; + if (f.rel.endsWith("boom.rs")) { + assert.equal(r.kind, "error", f.rel); + if (r.kind === "error") assert.match(r.message, /Aborted\(\)/, "the child's own result, not a retry"); + } else { + assert.equal(r.kind, "parsed", `${f.rel}: ${r.kind === "error" ? r.message : r.kind}`); + } + }); + assert.equal(fellBack, false, "a retirement is not a crash: the pool never fell back to in-thread"); +}); + +test("runParsePool: a retiring child is told to stop; it never has to race its result with its own exit", { timeout: 60_000 }, async () => { + const d = repo(); + for (let i = 0; i < 3; i++) writeFileSync(join(d, "src", `r${i}.rs`), `pub fn r${i}() {}\n`); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + let fellBack = false; + // The fake exits only on `stop`. A pool that waits for a retiring child to exit + // by itself hangs here (and the test times out). + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { + workers: 1, entry: fakeWorker(d, "retire-waits"), onFallback: () => { fellBack = true; }, + }); + assert.deepEqual(results, files.map((f) => extractOne(f, null)), "every result came from a child, once"); + assert.equal(fellBack, false); +}); + +test("runParsePool: past workers * 4 retirements each one costs a respawn credit, so an always-poisoning grammar falls back", { timeout: 120_000 }, async () => { + const d = mkdtempSync(join(tmpdir(), "graft-pool-retire-")); + mkdirSync(join(d, "src"), { recursive: true }); + // Every file poisons the child that parses it. + for (let i = 0; i < 12; i++) writeFileSync(join(d, "src", `f${String(i).padStart(2, "0")}boom.rs`), `pub fn f${i}() {}\n`); + await warmGenericGrammars(["rust"]); + const files = byRel(listSourceStats(d, join(d, "graft"))); + let fellBack = false; + // One worker: 4 free retirements, then one credit each against a budget of 2, + // so the 7th retirement exhausts it and the parent parses the remaining 5. + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { + workers: 1, entry: poisoningWorker(d), onFallback: () => { fellBack = true; }, + }); + assert.equal(fellBack, true, "the pool gave up instead of forking a child per file"); + assert.deepEqual(results.map((r) => r.kind), [...Array(7).fill("error"), ...Array(5).fill("parsed")]); +}); + +test("runParsePool: a parked file keeps its real content hash", async () => { + const d = repo(); + writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "crash-on-boom") }); + const boom = results[files.findIndex((f) => f.rel === "src/boom.rs")]; + assert.equal(boom.kind, "error"); + if (boom.kind !== "error") return; + assert.equal(boom.entry.hash, contentHash("pub fn boom() {}\n")); +}); + +test("runParsePool: once the pool has worked, the in-thread fallback never parses a job that killed a child", async () => { + const d = repo(); + writeFileSync(join(d, "src", "a.rs"), "pub fn a() {}\n"); + writeFileSync(join(d, "src", "b1boom.rs"), "pub fn b1() {}\n"); + writeFileSync(join(d, "src", "b2boom.rs"), "pub fn b2() {}\n"); + await warmGenericGrammars(["rust"]); + const files = byRel(listSourceStats(d, join(d, "graft"))); + assert.equal(files[0].rel, "src/a.rs"); + let fellBack = false; + // One worker (budget 2): a.rs succeeds, b1boom.rs kills two children and is + // parked, b2boom.rs kills the third and the budget is gone: the rest of the + // queue goes in-thread, but b2boom.rs must not be parsed in the parent. + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { + workers: 1, entry: fakeWorker(d, "crash-on-boom"), onFallback: () => { fellBack = true; }, + }); + const b2 = results[files.findIndex((f) => f.rel === "src/b2boom.rs")]; + assert.equal(b2.kind, "error"); + if (b2.kind !== "error") return; + assert.match(b2.message, /^src\/b2boom\.rs: parse failed — parser process exited \(code 9\) on this file; not retried in-process/); + assert.equal(b2.entry.hash, contentHash("pub fn b2() {}\n")); + assert.equal(results[files.findIndex((f) => f.rel === "src/lib.rs")].kind, "parsed", "the untouched rest still parses in-thread"); + assert.equal(fellBack, true, "the pool reported its fallback"); +}); + +test("runParsePool: a child's clean exit after stop neither burns a respawn credit nor forks a replacement", async () => { + const d = repo(); + const only = ["src/boom.rs", "src/lib.rs"]; + writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); + await warmGenericGrammars(["rust"]); + const files = byRel(listSourceStats(d, join(d, "graft"))).filter((f) => only.includes(f.rel)); + // Two children: A takes boom.rs, B takes lib.rs, finishes, is told to stop and + // lingers 1200 ms. A crashes 500 ms in; its retry waits for a fresh child (1.5 s + // warm-up), so B's clean exit lands while boom.rs is back in the queue — 700 ms + // after A's crash, and at least 800 ms before the replacement is ready. + await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "slow-stop") }); + const spawned = readFileSync(join(d, "spawned.log"), "utf8").trim().split("\n"); + assert.equal(spawned.length, 3, "A, B and A's one replacement — B's stop exit forks nothing"); +}); + +test("runParsePool: a job killed at the timeout says it timed out", async () => { + const d = repo(); + writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "hang-on-boom"), jobTimeoutMs: 500 }); + const boom = results[files.findIndex((f) => f.rel === "src/boom.rs")]; + assert.equal(boom.kind, "error"); + if (boom.kind !== "error") return; + assert.match(boom.message, /^src\/boom\.rs: parse failed — parser process timed out after 0\.5 s on this file twice$/); +}); + +test("runParsePool: a fork() that throws is a dead child, not a rejected build", async () => { + const d = repo(); + await warmGenericGrammars(["rust"]); + const files = listSourceStats(d, join(d, "graft")); + let fellBack = false; + // A NUL in the path makes fork() throw synchronously (ERR_INVALID_ARG_VALUE). + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { + workers: 2, entry: join(d, "no\u0000such.mjs"), onFallback: () => { fellBack = true; }, + }); + assert.deepEqual(results, files.map((f) => extractOne(f, null))); + assert.equal(fellBack, true); +}); + test("runParsePool: a child that dies mid-job costs that file two attempts, then an error; the rest still parse", async () => { const d = repo(); writeFileSync(join(d, "src", "boom.rs"), "pub fn boom() {}\n"); @@ -170,7 +355,12 @@ test("runParsePool: children that never become ready are killed at the warm-up t const files = listSourceStats(d, join(d, "graft")); const t0 = Date.now(); const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "never-ready"), warmTimeoutMs: 1500 }); - assert.ok(Date.now() - t0 < 10_000, "did not hang"); + // A death before "ready" costs two respawn credits, so a pool of 2 (budget + // workers*2 = 4) burns out in two 1500ms warm-up waves (~3s), not the four the + // old one-credit rule would have run (~6s). Bound it below three waves so the + // faster give-up is enforced, not just "did not hang". + const elapsed = Date.now() - t0; + assert.ok(elapsed < 4000, `fell back after two warm-up waves, not more (was ${elapsed}ms)`); assert.deepEqual(results, files.map((f) => extractOne(f, null)), "in-thread fallback produced the full result set"); }); @@ -179,9 +369,11 @@ test("runParsePool: children that always crash fall back to in-thread parsing in for (let i = 0; i < 10; i++) writeFileSync(join(d, "src", `c${i}.rs`), `pub fn c${i}() {}\n`); await warmGenericGrammars(["rust"]); const files = listSourceStats(d, join(d, "graft")); - const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "always-crash") }); + let fellBack = false; + const results = await runParsePool(files, () => null, { generic: ["rust"], container: [] }, { workers: 2, entry: fakeWorker(d, "always-crash"), onFallback: () => { fellBack = true; } }); assert.equal(results.length, files.length); assert.ok(results.every((r) => r.kind === "parsed"), "every file was parsed in-thread after the pool gave up"); + assert.equal(fellBack, true, "the pool reported its fallback, so the CLI does not claim N parser processes"); }); test("runParsePool: a file that kills every child with no other work is treated as environmental and parsed in-thread", async () => { @@ -206,7 +398,9 @@ test("buildGraph with parseWorkers writes byte-identical wiring.json to the in-t assert.equal(inThread.parseWorkers, 0); const a = readFileSync(wiringPath(contextDirFor(d)), "utf8"); const pooled = await buildGraph(d, { reuse: false, parseWorkers: 2 }); - assert.equal(pooled.parseWorkers, 2); + assert.equal(pooled.parseWorkers, Math.min(2, availableParallelism()), "clamped to the core count on a small runner"); + assert.equal(pooled.poolFallback, false, "the children did the parsing"); + assert.equal(inThread.poolFallback, false); assert.equal(pooled.parsed, inThread.parsed); assert.equal(readFileSync(wiringPath(contextDirFor(d)), "utf8"), a); assert.equal((await buildGraph(d, { reuse: false, parseWorkers: "auto" })).parseWorkers, 0, "auto on 42 files stays in-thread"); diff --git a/test/graph-seed.test.ts b/test/graph-seed.test.ts index f0214eb9d..51184b8c8 100644 --- a/test/graph-seed.test.ts +++ b/test/graph-seed.test.ts @@ -216,6 +216,25 @@ test("a git worktree starts with no graph, and one query gives it a correct one" rmSync(wt, { recursive: true, force: true }); }); +test("a seed never copies another process's half-written *.tmp sidecar", async () => { + const main = gitRepo(); + await buildGraph(main); + const cache = join(outOf(main), ".cache"); + const sidecars = readdirSync(cache).filter((f) => f.startsWith("extract.") || f.startsWith("fingerprint.")); + assert.ok(sidecars.length >= 2, sidecars.join(", ")); + // What openAtomic leaves behind when a writer dies before its rename. + for (const f of sidecars) writeFileSync(join(cache, `${f}.4242.tmp`), "partial"); + + const wt = addWorktree(main, "tmp-orphans"); + assert.equal(seedGraph(wt).seeded, true); + const copied = readdirSync(join(outOf(wt), ".cache")); + assert.deepEqual(copied.filter((f) => f.endsWith(".tmp")), [], "no temp file travels"); + for (const f of sidecars) assert.ok(copied.includes(f), `${f} still travels`); + + rmSync(main, { recursive: true, force: true }); + rmSync(wt, { recursive: true, force: true }); +}); + test("a worktree nested inside the repo is seeded too, and isn't double-indexed", async () => { // Claude Code's own worktree feature puts them at `/.claude/worktrees/` // rather than off in a temp dir. Same `.git`-file shape, but nested — so the parent's diff --git a/test/json-stream.test.ts b/test/json-stream.test.ts index 3f700a211..98c81d68a 100644 --- a/test/json-stream.test.ts +++ b/test/json-stream.test.ts @@ -1,9 +1,9 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { existsSync, mkdtempSync, readFileSync, readdirSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { forEachLine, openAtomic } from "../src/util/json-stream.js"; +import { forEachFileLine, forEachLine, openAtomic } from "../src/util/json-stream.js"; test("openAtomic: nothing is visible at the target until commit, and the temp file is gone after", () => { const d = mkdtempSync(join(tmpdir(), "graft-atomic-")); @@ -47,3 +47,36 @@ test("forEachLine: an empty buffer yields nothing", () => { forEachLine(Buffer.alloc(0), () => n++); assert.equal(n, 0); }); + +test("forEachLine: a CRLF line ending is stripped like LF", () => { + const seen: string[] = []; + forEachLine(Buffer.from("a\r\nb\r\n"), (l) => seen.push(l)); + assert.deepEqual(seen, ["a", "b"]); +}); + +test("forEachFileLine: lines straddling every chunk boundary come out whole, multibyte UTF-8 and CRLF included", () => { + const d = mkdtempSync(join(tmpdir(), "graft-lines-")); + const p = join(d, "lines.ndjson"); + const text = "a\n{\"k\":\"é\"}\r\n\nlonger line than any chunk é€😀\r\nlast"; + writeFileSync(p, text); + const expected: [string, number][] = [["a", 0], ["{\"k\":\"é\"}", 1], ["", 2], ["longer line than any chunk é€😀", 3], ["last", 4]]; + for (const chunk of [1, 2, 3, 5, 7, 64 * 1024]) { + const seen: [string, number][] = []; + forEachFileLine(p, (l, i) => seen.push([l, i]), chunk); + assert.deepEqual(seen, expected, `chunk size ${chunk}`); + } +}); + +test("forEachFileLine: an empty file yields nothing; a trailing newline adds no line", () => { + const d = mkdtempSync(join(tmpdir(), "graft-lines-")); + const empty = join(d, "empty"); + writeFileSync(empty, ""); + let n = 0; + forEachFileLine(empty, () => n++, 4); + assert.equal(n, 0); + const one = join(d, "one"); + writeFileSync(one, "x\n"); + const seen: string[] = []; + forEachFileLine(one, (l) => seen.push(l), 1); + assert.deepEqual(seen, ["x"]); +});