Skip to content

feat(graph): name JS/TS namespace-member functions; add --exclude-dir - #306

Open
henryhans31415 wants to merge 2 commits into
trailhq:mainfrom
henryhans31415:feat/js-namespace-members
Open

feat(graph): name JS/TS namespace-member functions; add --exclude-dir#306
henryhans31415 wants to merge 2 commits into
trailhq:mainfrom
henryhans31415:feat/js-namespace-members

Conversation

@henryhans31415

Copy link
Copy Markdown

Problem

A codebase written in the one-namespace style — MN.runNode = (g, n) => {…}, the pre-ESM idiom, and likewise exports.foo = … and Foo.prototype.bar = function — gets no nodes for those functions. describe() names a TS/JS function only from function_declaration/method_definition or a variable_declarator with a function value, so an assignment_expression with a function on the right mints nothing.

Measured on a 200-file app with 1,311 such definitions: graft build found 581 named symbols in it, graft callers runNode answered no symbol "runNode" in the graph, and skeleton listed only the inner helpers. That is a confidently wrong map — every one of those functions read as "nothing depends on this".

Change

  • extract.ts / bindings.tsNS.foo = (…) => …, NS.foo = function, exports.foo = … and Foo.prototype.bar = function mint a method node owned by the receiver path (NS, NS.sub, Foo; prototype is stripped so the method belongs to the class and this.x() inside it resolves against it). exports. / module.exports. members are marked exported. bindings.ts pushes the same scope segment (NS.foo) so a typed local inside the body still resolves; the helper lives in bindings.ts, which extract.ts already imports from, so no new cycle.
  • resolve.ts — an untyped member call now carries nsReceiver (TS/JS only; a plain identifier path, never this/super). It resolves only owner-qualified — ownerMethod.has("MN.runNode") — so this is not the bare-name fallback map clusters one entry per file instead of per directory #35 ruled out: x.foo() with no x.foo = … definition still drops (test included), and cross-file ambiguity still drops.
  • graft build --exclude-dir <path> — the complement of --only-dir. The motivating repo commits a generated copy of src/ (a serverless function cannot reach ../../src), and git ls-files always lists a tracked file, so without this every symbol appeared twice and every owner-qualified call dropped as ambiguous. Same normalization and the same home as --only-dir (the fingerprint, never .graft/config.json); honored by build, check, the hooks/refresh path and --deep.

Evidence

Same repo, before → after: named symbols in the app 581 → 2,518; resolved calls into the namespace 0 → 5,603; callers runNode / runCharge / sanitiseState went from "no symbol" to 11 / 10 / 1 callers, matching a plain grep.

Full suite green (1,227 pass, 0 fail). Eight new tests in test/graph-ts-namespace.test.ts (nodes, owners, same-file/cross-file confidence, prototype + this, bindings scope, the #35 regression guard, the shapes that must mint nothing) and test/graph-exclude-dir.test.ts (end-to-end through the CLI: fingerprint, check, the freshness probe, composition with --only-dir).

Not in this PR

Arrow functions as object-literal values ({ run: () => {…} }) are still unnamed — there is no natural qualified name for them. Happy to follow up if you would like a scheme for that.

🤖 Generated with Claude Code

`NS.foo = (…) => …` / `NS.foo = function` / `exports.foo` / `Foo.prototype.bar`
mint a method node owned by the receiver path, and an untyped `NS.foo()` call
resolves owner-qualified against it (never by bare name — trailhq#35 holds). Before,
such definitions had no node at all: `callers` said "no symbol", `skeleton`
omitted them, and the calls in their bodies attributed to the file.

`--exclude-dir <path>` is the complement of `--only-dir`, for a committed
generated copy of real source that Git's ignore rules cannot hide; it is
recorded in the fingerprint so check/refresh/--deep enumerate the same set.

Measured on a 200-file app written in the one-namespace style: named symbols
581 → 2,518; resolved calls into the namespace 0 → 5,603; the same three
functions that returned "no symbol" now list 11, 10 and 1 callers, matching a
plain grep.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@trailhq-graft

trailhq-graft Bot commented Sep 7, 2026

Copy link
Copy Markdown

🌱 graft blast radius

4 areas changed → 5 areas can be affected. 14 dependent symbols, depth 2.
Tests: no test reaches list; Context Building, CLI Engine have tests the diff did not touch; 1 area updated its tests.
Tag: @anirudhkumar-nanonets — 7 of 9 areas · @shhdwi — 6 of 9 areas · @Frankie-Xu — Context Building

flowchart TB
  A0(("Workspace Federation<br/>4 symbols"))
  A1(("MCP Server<br/>4 symbols"))
  A2(("Pull Request Review<br/>4 symbols"))
  A3(("Viewer Build<br/>1 symbol"))
  A4(("Context Validation<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3,A4 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Workspace Federation 4 src/graph/workspace-cli.ts:L49-L70 buildChild — calls, depth 1 Context Building, CLI Engine +1
MCP Server 4 src/mcp/tools.ts:L216-L244 callTool — calls, depth 1 Dependency Graph
Pull Request Review 4 src/app/review.ts:L45-L99 reviewPullRequest — calls, depth 1 Dependency Graph
Viewer Build 1 scripts/build-viewer.mjs:L1-L45 build-viewer.mjs — calls, depth 2 Dependency Graph
Context Validation 1 src/context/check.ts:L38-L111 checkContext — calls, depth 1 Context Building, Dependency Graph
Who knows this code — 5 people across 9 areas
Area Who knows it
Dependency Graph · changed @anirudhkumar-nanonets — 24 commits, last 11d ago · @shhdwi — 20 commits, last 25d ago
Context Building · changed @anirudhkumar-nanonets — 4 commits, last 18d ago · @Frankie-Xu — 2 commits, last 7d ago
CLI Engine · changed @anirudhkumar-nanonets — 38 commits, last 5d ago · @shhdwi — 24 commits, last 25d ago
list · changed @shhdwi — 3 commits, last 2mo ago · @bhavesh-gupta-investis — 1 commit, last 14d ago
Workspace Federation · affected @shhdwi — 6 commits, last 26d ago · @anirudhkumar-nanonets — 3 commits, last 27d ago
MCP Server · affected @shhdwi — 15 commits, last 26d ago · @anirudhkumar-nanonets — 11 commits, last 5d ago
Pull Request Review · affected @anirudhkumar-nanonets — 5 commits, last 6d ago
Viewer Build · affected @shhdwi — 2 commits, last 26d 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 14 dependent symbols, grouped by area

Workspace Federation — 4 symbols in 2 files

  • src/graph/workspace-cli.ts:L49-L70 — buildChild (calls, depth 1)
    66: if (opts.deep) await engine.init(childDir, { extensions: opts.extensions });
  • src/graph/workspace.ts:L630-L655 — federateCheck (calls, depth 1)
    638: const g = await checkGraph(join(root, child));
  • src/graph/workspace-cli.ts:L126-L130 — runWorkspaceCheck (calls, depth 2)
  • src/graph/workspace.ts:L709-L722 — splitWorkspace (calls, depth 2)
    719: clearParentGraft(root, override); // drop the mega-graph/.cache/cards…

MCP Server — 4 symbols in 2 files

  • src/mcp/tools.ts:L216-L244 — callTool (calls, depth 1)
    235: : await ensureFreshGraph(root, { contextDir: dirOverride });
  • src/mcp/server.ts:L71-L151 — startMcpServer (calls, depth 2)
    74: // refreshes rule files an older `graft init` wrote, and kicks off the cached
  • src/mcp/tools.ts:L247-L329 — callSingleTool (calls, depth 2)
    272: const g = await engine.checkGraph(root);
  • src/mcp/tools.ts:L153-L199 — callWorkspaceTool (calls, depth 2)

Pull Request Review — 4 symbols in 4 files

  • 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-worker.ts:L67-L87 — run (calls, depth 2)
  • src/app/server.ts:L31-L37 — AppSeams (references, depth 2)

Viewer Build — 1 symbol in 1 file

  • scripts/build-viewer.mjs:L1-L45 — build-viewer.mjs (calls, depth 2)
    30: // The generic (breadth) tier's tags.scm query files are runtime assets — tsc

Context Validation — 1 symbol in 1 file

  • src/context/check.ts:L38-L111 — checkContext (calls, depth 1)
    58: // Current code files on disk (same persisted directory/submodule choices
Test signal per changed area — 1 ✓ · 2 ⚠ · 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.

  • Dependency Graph — 6 of 19 reached · 3 test files changed here: test/graph-exclude-dir.test.ts, test/graph-graftignore.test.ts, test/graph-ts-namespace.test.ts
    • not reached: defName, tsMemberAssignmentTarget, walk, describe, tsReceiver, isNamespaceReceiver, tsIdentifierPath, writeFingerprint, …5 more
  • Context Building — 1 of 3 reached · 5 test files reach it, none changed here
    • not reached: resolveExcludeDirs, listContextFiles
  • CLI Engine — 1 of 2 reached · 1 test file reaches it, none changed here
    • not reached: graph
  • list — 0 of 2 reached · no test file reaches it
    • not reached: realBuild, list
42 test suites also reference this code

51 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/container-extract.test.ts
  • test/context-checkpoint.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-cross-language.test.ts
  • test/graph-follow-nested-repos.test.ts
  • test/graph-follow-submodules.test.ts
  • test/graph-go.test.ts
  • test/graph-include-dir.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-load.test.ts
  • test/graph-only-dir.test.ts
  • test/graph-php.test.ts
  • …22 more

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

graft blast · origin/main...HEAD · depth 2 · 18 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 7, 2026
…les links every caller to every definition; the Stop-hook rebuild keeps --only-dir/--exclude-dir

`.graftignore` — one repo-relative path per line — is read live on every
enumeration (build, check, the freshness probe, refresh, --deep), so a path Git
tracks but graft must never index stays out in a fresh checkout with no flag
and no fingerprint. Only the flags go into the fingerprint.

`MN.foo = …` in a base file and again in an overlay is one symbol assigned
twice, not two candidates to guess between: an untyped `MN.foo()` now links to
both (inferred) instead of dropping as ambiguous, which read as "nothing calls
this" for exactly the functions a second file overrides. Class methods with
several same-named owners still drop.

The Claude Code Stop hook's background sync ran a plain `graft build`, so the
first turn after a --only-dir/--exclude-dir build silently widened the graph
back to the whole tree; it now re-applies the fingerprint's lists, read off
the sidecar with plain fs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
github-actions Bot added a commit that referenced this pull request Sep 7, 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