Skip to content

Fix/wasm parser lifetime - #340

Open
csm-kb wants to merge 3 commits into
trailhq:mainfrom
csm-kb:fix/wasm-parser-lifetime
Open

Fix/wasm parser lifetime#340
csm-kb wants to merge 3 commits into
trailhq:mainfrom
csm-kb:fix/wasm-parser-lifetime

Conversation

@csm-kb

@csm-kb csm-kb commented Sep 10, 2026

Copy link
Copy Markdown

Was working with graft on a large codebase (~65k files) to stress-test, noticed some failure doing so -> attempted a fix pass to move it towards functional. Feel free to leave inputs or modify/reject!

I have a subsequent PR that improves the graft build memory tolerance + performance using a streaming worker pool (~2.5x speedup on that codebase) that I can open afterwards.


Problem

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 subsequent parse failed with Aborted() after a few minutes.

Solution

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.

…ed 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.
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.
@trailhq-graft

trailhq-graft Bot commented Sep 10, 2026

Copy link
Copy Markdown

🌱 graft blast radius

2 areas changed → 7 areas can be affected. 20 dependent symbols, depth 2.
Tests: 1 area updated its tests.
Tag: @anirudhkumar-nanonets — 7 of 9 areas · @shhdwi — 6 of 9 areas · @Frankie-Xu — Context Building

flowchart TB
  A0(("Graph Workspace Orchestration<br/>7 symbols"))
  A1(("Pull Request Review<br/>6 symbols"))
  A2(("Engine Graph<br/>2 symbols"))
  A3(("Context Building<br/>2 symbols"))
  A4(("Viewer Build Script<br/>1 symbol"))
  AX(("2 smaller areas<br/>2 symbols"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3,A4 reached;
  classDef tail fill:#EEF2F3,stroke:#9AA4A9,stroke-width:1px,color:#3A4247;
  class AX tail;
Loading
Can be affected Symbols Nearest hop Reached from
Graph Workspace Orchestration 7 src/graph/check.ts:L58-L164 checkGraph — calls, depth 1 Graph Construction, test/generic-extract.test.ts
Pull Request Review 6 src/app/review-process.ts:L206-L211 settle — calls, depth 1 Graph Construction, test/generic-extract.test.ts
Engine Graph 2 src/engine.ts:L91-L101 graph — calls, depth 1 Graph Construction
Context Building 2 src/context/build.ts:L452-L467 mapWithConcurrency — calls, depth 1 test/generic-extract.test.ts
Viewer Build Script 1 scripts/build-viewer.mjs:L1-L45 build-viewer.mjs — calls, depth 2 Graph Construction
MCP Tools 1 src/mcp/tools.ts:L216-L244 callTool — calls, depth 2 Graph Construction
1 smaller area 1 Claude Sync Execution see below
Who knows this code — 4 people across 9 areas
Area Who knows it
Graph Construction · changed @anirudhkumar-nanonets — 17 commits, last 21d ago · @shhdwi — 11 commits, last 28d ago
CLI · changed @anirudhkumar-nanonets — 36 commits, last 8d ago · @shhdwi — 23 commits, last 28d ago
Graph Workspace Orchestration · affected @anirudhkumar-nanonets — 16 commits, last 14d ago · @shhdwi — 8 commits, last 29d ago
Pull Request Review · affected @anirudhkumar-nanonets — 5 commits, last 9d ago
Engine Graph · affected @anirudhkumar-nanonets — 16 commits, last 1mo ago
Context Building · affected @anirudhkumar-nanonets — 4 commits, last 21d ago · @Frankie-Xu — 2 commits, last 10d ago
Viewer Build Script · affected @shhdwi — 2 commits, last 29d ago
MCP Tools · affected @shhdwi — 14 commits, last 29d ago · @anirudhkumar-nanonets — 7 commits, last 8d ago
…1 further area

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 20 dependent symbols, grouped by area

Graph Workspace Orchestration — 7 symbols in 5 files

  • src/graph/check.ts:L58-L164 — checkGraph (calls, depth 1)
    90: await warmGenericGrammars(
  • src/graph/enrich.ts:L226-L241 — mapWithConcurrency (calls, depth 1)
    229: fn: (item: T, index: number) => Promise<R>,
  • src/graph/refresh.ts:L150-L227 — ensureFreshGraph (calls, depth 1)
    216: await buildGraph(dir, { contextDir: opts.contextDir, graphOnly: true, onlyDirs });
  • src/graph/enrich.ts:L74-L223 — enrichGraph (calls, depth 2)
    94: const was = prior.get(node.id);
  • src/graph/refresh.ts:L235-L261 — ensureFreshChildren (calls, depth 2)
  • src/graph/workspace-cli.ts:L49-L70 — buildChild (calls, depth 2)
  • src/graph/workspace.ts:L630-L655 — federateCheck (calls, depth 2)

Pull Request Review — 6 symbols in 4 files

  • src/app/review-process.ts:L206-L211 — settle (calls, depth 1)
    206: const settle = (fn: () => void): void => {
  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 1)
    55: await buildGraph(checkout.dir);
  • src/app/review-process.ts:L179-L183 — childReviewer (references, depth 2)
  • src/app/review-process.ts:L185-L265 — runInChild (calls, depth 2)
    206: const settle = (fn: () => void): void => {
  • src/app/review-worker.ts:L67-L87 — run (calls, depth 2)
  • src/app/server.ts:L31-L37 — AppSeams (references, depth 2)

Engine Graph — 2 symbols in 1 file

  • src/engine.ts:L91-L101 — graph (calls, depth 1)
    91: graph(dir: string, opts: GraphRunOptions = {}): Promise<GraphBuildResult> {
  • src/engine.ts:L82-L84 — checkGraph (calls, depth 2)

Context Building — 2 symbols in 1 file

  • src/context/build.ts:L452-L467 — mapWithConcurrency (calls, depth 1)
    455: fn: (item: T, index: number) => Promise<R>,
  • src/context/build.ts:L146-L374 — buildContext (calls, depth 2)
    258: else delete cache.synth[key];

Viewer Build Script — 1 symbol in 1 file

  • scripts/build-viewer.mjs:L1-L45 — build-viewer.mjs (calls, depth 2)
    3: * assets). Runs as part of `npm run build`; the bundle ships in the package

MCP Tools — 1 symbol in 1 file

  • src/mcp/tools.ts:L216-L244 — callTool (calls, depth 2)

Claude Sync Execution — 1 symbol in 1 file

  • src/claude/sync-run.ts:L19-L33 — runSync (calls, depth 2)
Test signal per changed area — 1 ✓ · 1 –

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Graph Construction — 9 of 12 reached · 2 test files changed here: test/container-extract.test.ts, test/generic-extract.test.ts
    • not reached: notePoison, poisonedError, mkDef
  • CLI — no function, method or class changed here
35 test suites also reference this code

47 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/claude-hooks.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-node24-probe.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • test/graph-r-phase3.test.ts
  • test/graph-r-phase4.test.ts
  • test/graph-r-phase5.test.ts
  • test/graph-r.test.ts
  • …15 more

⚠️ 1 changed file not in the graph (CHANGELOG.md) — no parser claims the extension, or the index predates the file.

graft blast · origin/main...HEAD · depth 2 · 7 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant