fix(graph): cold builds disagree on reference edges (#116)

src/graph/extract.ts

@@ -470,6 +470,23 @@ function collectTsImportBindings(
   for (const child of node.namedChildren) collectTsImportBindings(child, specifier, out);
 }
 
+/**
+ * Do these two wrappers stand for the same syntax node? `===` does not answer that:
+ * node-tree-sitter materializes `SyntaxNode` objects on demand and caches them
+ * weakly, so reaching one node twice can return two different JS objects. Comparing
+ * wrappers makes a purely syntactic question depend on collector timing — two cold
+ * builds of unchanged source then disagree on `references` edges (#116).
+ *
+ * `id` is the stable identity, unique within one tree, so the tree is compared too.
+ * A `Tree` is one object per parse (unlike its nodes), so `===` is right for it.
+ */
+function sameSyntaxNode(
+  a: Parser.SyntaxNode | null | undefined,
+  b: Parser.SyntaxNode | null | undefined,
+): boolean {
+  return !!a && !!b && a.tree === b.tree && a.id === b.id;
+}
+
 /**
  * A parameter or local declaration wins over an import inside that function.
  * Drop that imported binding for the whole function rather than create a false
@@ -483,7 +500,7 @@ function withoutShadowedImports(
   const shadowed = new Set<string>();
   const definitionValue = definition.childForFieldName("value");
   const visit = (node: Parser.SyntaxNode): void => {
-    if (node !== definition && node !== definitionValue && isFunctionBoundary(node)) {
+    if (!sameSyntaxNode(node, definition) && !sameSyntaxNode(node, definitionValue) && isFunctionBoundary(node)) {
       const name = node.childForFieldName("name");
       if (name?.type === "identifier") shadowed.add(name.text);
       return;
@@ -521,13 +538,16 @@ function isFunctionBoundary(node: Parser.SyntaxNode): boolean {
 function isDirectCallee(node: Parser.SyntaxNode, callTypes: ReadonlySet<string>): boolean {
   const parent = node.parent;
   if (!parent || !callTypes.has(parent.type)) return false;
-  return parent.childForFieldName("function") === node || parent.childForFieldName("name") === node;
+  return (
+    sameSyntaxNode(parent.childForFieldName("function"), node) ||
+    sameSyntaxNode(parent.childForFieldName("name"), node)
+  );
 }
 
 /** Definition/declaration identifiers name a new binding; they do not use one. */
 function isDeclarationName(node: Parser.SyntaxNode): boolean {
   const parent = node.parent;
-  return parent?.childForFieldName("name") === node;
+  return sameSyntaxNode(parent?.childForFieldName("name"), node);
 }
 
 /** Recognize the definition shapes: mapped node types, Go's type/method forms, and

test/graph-references.test.ts

@@ -2,11 +2,17 @@
  * Regression coverage for issue #34: imported functions used as values must
  * remain visible to `callers`, but as weaker `references` edges rather than
  * being mislabeled as direct calls.
+ *
+ * …and for issue #116, at the bottom: the same rules have to hold in a file
+ * big enough that tree-sitter stops handing back the same JS wrapper object
+ * for a node it already materialized.
  */
 import { test } from "node:test";
 import assert from "node:assert/strict";
 import { mkdirSync, writeFileSync } from "node:fs";
-import { join } from "node:path";
+import { spawnSync } from "node:child_process";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
 import { buildGraph } from "../src/graph/build.js";
 import { callersOf } from "../src/graph/traverse.js";
 import { readGraph, wiringPath } from "../src/graph/write.js";
@@ -85,6 +91,95 @@ test("aliased named imports resolve references to the exported symbol", async ()
   );
 });
 
+/**
+ * Regression for #116. node-tree-sitter may rematerialize SyntaxNode wrappers
+ * after GC, so wrapper identity cannot reliably classify direct callees.
+ *
+ * The probe constrains GC pressure and exercises enough call sites to make the
+ * failure reproducible while preserving the intended calls/references semantics.
+ */
+const CALL_SITES = 60;
+
+test("a direct call is not also recorded as a value reference, at scale (#116)", () => {
+  const root = tmpRepo("graft-reference-scale-");
+  mkdirSync(join(root, "src"), { recursive: true });
+  writeFileSync(
+    join(root, "src", "dep.ts"),
+    [
+      "export function target(): number { return 1; }",
+      "export function register(fn: () => number): unknown { return fn; }",
+      "",
+    ].join("\n"),
+  );
+  writeFileSync(
+    join(root, "src", "consumer.ts"),
+    [
+      'import { target, register } from "./dep.js";',
+      "",
+      // (b) handed over as a value — a `references` edge
+      "export function usesValue(): unknown { return register(target); }",
+      // (c) both, in one body — BOTH edges, from two distinct occurrences
+      "export function usesBoth(): number { register(target); return target(); }",
+      "",
+      // (d) a block-scoped declaration that happens to reuse the imported name.
+      // It DECLARES a binding, so it is not a use of the import. Module-level, so
+      // `withoutShadowedImports` (which only runs inside a function or method) is
+      // not what protects it — `isDeclarationName` is.
+      "{",
+      "  const target = 0;",
+      "}",
+      "",
+      // (a) the case #116 breaks: every one of these invokes `target` directly, so
+      // each must produce a `calls` edge and no `references` edge.
+      ...Array.from(
+        { length: CALL_SITES },
+        (_, i) => `export function callsTarget${i}(): number { return target(); }`,
+      ),
+      "",
+    ].join("\n"),
+  );
+
+  const probe = join(dirname(fileURLToPath(import.meta.url)), "reference-scale-probe.ts");
+  const run = spawnSync(
+    process.execPath,
+    ["--max-semi-space-size=1", "--import", "tsx", probe, root],
+    { encoding: "utf8" },
+  );
+  assert.equal(run.status, 0, `probe failed:\n${run.stderr}`);
+  const line = run.stdout.split("\n").find((l) => l.startsWith("__EDGES__"));
+  assert.ok(line, `probe printed no edges:\n${run.stdout}\n${run.stderr}`);
+  const edges: Array<{ source: string; relation: string }> = JSON.parse(line.slice("__EDGES__".length));
+  const linked = (from: string, relation: string): boolean =>
+    edges.some((e) => e.source === `src/consumer.ts#${from}` && e.relation === relation);
+
+  // (a) a direct invocation is a call, and ONLY a call — at every one of the sites.
+  const callSites = (relation: string): number =>
+    edges.filter((e) => /^src\/consumer\.ts#callsTarget\d+$/.test(e.source) && e.relation === relation).length;
+  assert.equal(callSites("calls"), CALL_SITES, "every direct call resolves to a calls edge");
+  assert.equal(
+    callSites("references"),
+    0,
+    "an identifier that is the direct callee must not also be booked as a value reference",
+  );
+
+  // (b) the recall #34/#49 bought must survive any fix to (a).
+  assert.equal(linked("usesValue", "references"), true, "a function passed as a value is still a reference");
+
+  // (c) `calls` and `references` may legitimately coexist between the same pair —
+  // here two different occurrences of `target` in one body. Whatever fixes (a)
+  // must not become a blanket "references and calls are mutually exclusive" rule.
+  assert.equal(linked("usesBoth", "calls"), true, "the invoked occurrence yields a calls edge");
+  assert.equal(linked("usesBoth", "references"), true, "the value occurrence yields a references edge");
+
+  // (d) the block-scoped redeclaration is attributed to the file node, since it sits
+  // outside any definition. It names a binding, so it must not reference the import.
+  assert.equal(
+    edges.some((e) => e.source === "src/consumer.ts" && e.relation === "references"),
+    false,
+    "a declaration that reuses an imported name is not a use of the import",
+  );
+});
+
 test("a local binding that shadows an import does not create a false reference", async () => {
   const root = tmpRepo("graft-reference-shadow-");
   mkdirSync(join(root, "src"), { recursive: true });

test/reference-scale-probe.ts

@@ -0,0 +1,28 @@
+/**
+ * Child process for the #116 regression test in `graph-references.test.ts`.
+ *
+ * It exists only so that the build can be run under `--max-semi-space-size=1`.
+ * #116 is a garbage-collection race: `isDirectCallee`/`isDeclarationName` compare
+ * tree-sitter `SyntaxNode` objects with `===`, which only answers correctly while
+ * the node is still in node-tree-sitter's tree cache. Whether a scavenge has
+ * cleared that cache mid-walk is otherwise pure luck — measured over a sweep of
+ * fixture sizes, the same input flips between correct and misclassified from one
+ * process to the next. Shrinking the young generation makes scavenges frequent
+ * enough that the cache is reliably cold, which turns the race into a fixed
+ * outcome and the regression test into a deterministic one.
+ *
+ * Prints one JSON line: every edge pointing at `src/dep.ts#target`.
+ */
+import { buildGraph } from "../src/graph/build.js";
+import { readGraph, wiringPath } from "../src/graph/write.js";
+import { join } from "node:path";
+
+const root = process.argv[2];
+
+await buildGraph(root, { reuse: false });
+const graph = readGraph(wiringPath(join(root, "graft")));
+const edges = (graph?.edges ?? [])
+  .filter((edge) => edge.target === "src/dep.ts#target")
+  .map(({ source, relation }) => ({ source, relation }));
+
+console.log(`__EDGES__${JSON.stringify(edges)}`);