fix(resolution): fuzzy matching skips a nested function the reference cannot reach

[bompus]

Fixes #1708.

What

matchFuzzy now filters its callable candidates with the same isLexicallyReachable check matchByExactName applies (#1230): a function nested inside another function is a candidate only for references from inside that container.

Why

When the only project symbol with a given name is a nested function, exact-match correctly declines, the ref falls through to fuzzy, and fuzzy adopted the closure as its "unique" candidate at confidence 0.5. Any builtin method call that reaches the resolver as a bare name (settled.value.text(), items.push()) hit whichever file happened to declare a closure of that name.

Measured

Real TS repo, 584 files, same build with and without the change:

	before	after
edges	41,378	41,367
resolvedBy: fuzzy	26	15
fuzzy edges onto nested functions	11	0

Nothing else moves. With #1679 (closure extraction) applied the same repo had 160 such edges, so that PR benefits from this one landing first.

Tests

__tests__/fuzzy-lexical-reach.test.ts: two files, a nested function text() in one, a settled.value.text() call in the other; asserts no calls edge from the caller to the closure and that the in-container call still resolves. Fails on main, passes with the change.

[bompus]

Don't merge this yet — on a real repo it trades 12 false positives for 59

I ran this against vitejs/vite (1,635 files, 9,354 nodes) rather than only the fixtures. Method: index the same checkout with each build, diff the full edge set joined back to symbol names and call-site lines. Indexing is deterministic — two indexes from the same build differ by 0 edges — so everything below is this change.

Edges: 27,499 → 27,546. Not a net removal: 12 lost, 59 gained.

The 12 removals are exactly right

Every one targets a genuinely nested function, unreachable from the call site:

removed edge	target
config.ts:resolveConfig@1489 → importAnalysis.ts:getEnv	function getEnv at :233, nested
scan.ts:build@297 → scan.ts:scan	async function scan at :131, nested in scanImports
.../shared.js@3 → client.ts:wait	function wait at :476, nested in waitForSuccessfulPingInternal
devtools/src/main.ts@14 → counter.ts:decrement	function decrement at :10, returned in an object

That's the fix doing what it says.

The 59 additions are a new false-positive class

52 calls and 7 imports, and nearly all of them point at one target: packages/vite/src/node/server/pluginContainer.ts:resolve. The sources are playground vite configs. Here is one, verbatim:

// playground/multiple-entrypoints/vite.config.js
import { resolve } from 'node:path'
...
    a0: resolve(dirname, 'entrypoints/a0.js'),

That resolve is node:path's. We now link it — and the import statement on line 1 — to vite's plugin container. Before this change there was no edge at all from those lines.

The mechanism follows from the fix. vite has ~12 things named resolve, and most are nested (const resolve = ... inside a function in build.ts, idResolver.ts, optimizer/scan.ts, dynamicImportVars.ts, …). Base saw a crowd of candidates and declined to resolve. Filtering out the unreachable nested ones leaves exactly one reachable candidate — pluginContainer.ts's resolve method — so the matcher now commits to it. The true target is external and not in the graph at all.

So the filter is correct and the bug it exposes is one layer up: when filtering leaves a single survivor, we commit without checking that the call site's binding is even internal. The import statement naming node:path is right there at the top of the file.

I'd rather fix that than ship a net increase in wrong edges. The likely shape: if the name is bound by an import whose source resolves outside the project (a builtin, or a bare dependency specifier), don't fuzzy-match it — and the vite configs above make a good regression fixture, since the count is large and the correct answer is unambiguous.

Marking my own PR as not-ready. The removal half is worth keeping and I'll resubmit once the survivor case is handled.

[bompus]

Follow-up: my numbers above came from a run with no kernel binary staged, so extraction took the wasm path. Since DEFAULT_ROUTED sends TS/JS to the Rust kernel, that left open whether the finding was a routing artifact.

Rebuilt the kernel and re-ran. It is not:

base(kernel): 27,499 edges    #1709(kernel): 27,546 edges
LOST:   12   [calls 10, imports 2]
GAINED: 59   [calls 52, imports 7]

Identical to the wasm arm, down to the counts and the same pluginContainer.ts:resolve targets. For reference, at main the kernel and wasm arms agree exactly on vite (9,354 nodes, 27,778 edges), so there was no divergence to hide behind.

One more thing worth flagging: #1710 is stacked on this commit and carries it verbatim, so it inherits the 59 edges until this is fixed or #1710 is rebased off it.

[bompus]

@danusha2345 measured this on two repos and got resolvedBy: fuzzy 337 → 2 and 142 → 0; I measured vite and got a net increase in wrong edges. Those look contradictory, so I re-ran vite computing both metrics on one tree to make them comparable.

They are not contradictory. Both are real, and the difference is repo shape.

Your metric, applied to vite

Same checkout indexed with b9ca4b7 (this PR's merge-base) and with the PR tip, counting every edge by resolvedBy:

resolvedBy	base	#1709	delta
(none)	8684	8684	0
exact-match	10716	10716	0
file-path	63	63	0
framework	19	19	0
function-ref	99	99	0
fuzzy	13	60	+47
import	5359	5359	0
instance-method	1035	1035	0
qualified-name	1790	1790	0

Every other resolver is unchanged at exactly zero. The whole effect of the commit is the fuzzy row — and on vite it goes the other way: 13 → 60.

So the metric is not the disagreement

I had a hypothesis that your metric couldn't see my failure mode — that counting fuzzy edges wouldn't detect wrong ones. That hypothesis is wrong, and this run is what killed it. Your metric would have caught this immediately on a tree like vite; it just moves in the opposite direction there. I'm glad I measured it rather than posting the theory.

Cross-tabbing the edge-set delta against the resolver settles what the 47 is made of:

LOST   12  — all resolvedBy=fuzzy
GAINED 59  — all resolvedBy=fuzzy   (52 calls, 7 imports)

The 12 removals are the fix working: genuinely nested functions the call site cannot reach. The 59 additions are the regression I described, and nearly all target one symbol:

[fuzzy] calls|playground/css-codesplit/vite.config.js@8 → packages/vite/src/node/server/pluginContainer.ts:resolve
[fuzzy] imports|playground/css-codesplit-cjs/vite.config.js@1 → packages/vite/src/node/server/pluginContainer.ts:resolve

That resolve is node:path's, imported on line 1 of the config. Its true target is external and not in the graph at all.

Why the two repos disagree

vite has ~12 symbols named resolve, most of them nested (const resolve = … inside functions in build.ts, idResolver.ts, optimizer/scan.ts, dynamicImportVars.ts). Base saw a crowd of candidates and declined. Filtering the unreachable nested ones leaves exactly one reachable candidate, so the matcher commits to it.

That is the pathological shape: a common name, many nested definitions, one surviving unrelated method, and the real target external. Your two trees (codegraph's own 787 files, the Kotlin+Go+TS app's 274) plausibly don't contain it — on them the filter only ever removes bad guesses, which is exactly what 337 → 2 and 142 → 0 describe.

So I read your result as correct for those trees and mine as correct for vite, with the underlying bug being one layer up from this commit: when filtering leaves a single survivor, we commit without checking the call site's binding is internal at all.

Reproduction note

12 lost / 59 gained reproduces my earlier numbers exactly. Absolute edge totals differ from my first comment (27,778 → 27,825 here vs 27,499 → 27,546 before) because vite moved — this run is a fresh shallow clone at 8492422. Both arms use the same checkout, so the delta is the commit.

Kernel note: my first run was wasm-only and I re-ran it with the kernel built; the counts were identical. This run is likewise a single vite tree indexed twice with only the codegraph build changing.

On sequencing

I'm not arguing #1709 should be abandoned — the removal half is right and your two repos show it doing real work. But it isn't a strict improvement yet, and landing it ahead of #1679 would ship the single-survivor regression to anyone whose tree has vite's shape. I'd rather fix the survivor check and resubmit, which I'll pick up next unless you'd prefer to take it.

If it's useful, the vite playground configs make an unambiguous regression fixture: the count is large and the correct answer is "no edge".

[bompus]

Tightening one thing in my comment above, since @danusha2345's numbers are "kernel on" and mine need to be comparable.

My kernel note there was ambiguous. To be exact: that run was the wasm arm — the clone had no codegraph-kernel.node staged, so despite TS/JS being in DEFAULT_ROUTED the extraction silently fell back to wasm. The sentence "this run is likewise..." referred to the single-tree method, not to the kernel, and it could reasonably be read the other way.

So I built the native kernel and re-ran both arms. CODEGRAPH_KERNEL_DEBUG=1 confirms it loaded for typescript/tsx/javascript/jsx on every pass.

Every number is identical.

resolvedBy	base	#1709	delta
fuzzy	13	60	+47
exact-match	10716	10716	0
import	5359	5359	0
qualified-name	1790	1790	0
instance-method	1035	1035	0
(none)	8684	8684	0
function-ref	99	99	0
file-path	63	63	0
framework	19	19	0

Totals 27,778 → 27,825; LOST 12, GAINED 59, all resolvedBy=fuzzy. Byte-identical to the wasm arm.

That is the expected result rather than a surprise — the change is in src/resolution/name-matcher.ts and the diff touches no Rust, so extraction arm shouldn't move it. But "shouldn't" is not a measurement, and this number is now load-bearing for a sequencing decision, so it seemed worth removing the doubt rather than leaving it to be raised later.

Credit where due: a colleague caught that ~/.cargo/bin is absent from this shell's PATH, which is why the toolchain looked missing and the binary was never staged.

[bompus]

I said I'd pick up the survivor check rather than leave it as an objection to your PR. That's now open as #1713.

It is standalone — one commit off main at b9ca4b7, and it deliberately does not contain this PR's commit. My first branch was stacked on it, which would have pulled your work into my diff; I cherry-picked onto the merge-base instead. So #1713 neither depends on this nor blocks it, and the two can be reviewed in either order.

Before opening it I measured the guard without your commit, because "it only helps because #1709 broke something" would be a fair challenge:

vite 8492422	LOST	GAINED	fuzzy
base → #1709	12	59	13 → 60
base → #1713 alone	4	0	13 → 9
base → #1709 + #1713	12	15	13 → 16
#1709 → #1709 + #1713	44	0	60 → 16

Every other resolver is 0 delta in all four. #1713 is subtractive on its own — the 4 it removes without your commit in play are import { scan } from 'rolldown/experimental' resolving onto the importing file's own scan, and import { getEnv } from '@vitejs/devtools/config' landing on importAnalysis.ts:getEnv.

On the sequencing question I raised: I think it's resolved, and not in the direction I argued. I said this PR wasn't a strict improvement and shouldn't land first. With #1713 the pair is +3 fuzzy on vite instead of +47, and your 12 removals are correct in every arm I measured — they're the same 12 whether or not my guard is present. If both land, order doesn't matter. If only one lands, #1713 is the one that is subtractive on every tree I have.

What I got wrong along the way, since it's on this thread: I hypothesised your metric couldn't see my failure mode. It can — it just moves the opposite way on vite, and measuring it is what killed the hypothesis.

#1713 does not fix everything on vite. 15 wrong edges remain, and they're a different mode — const resolve = (p) => path.resolve(...) and this.resolve(id, importer). Extraction emits no node for the const-arrow binding at all, so no resolver-layer rule can shadow the cross-file candidate; that's a separate issue and I've left it unclaimed rather than widen this.

[danusha2345]

Measured the survivor question from the other side, on the same tree (vite 8492422, b9ca4b7 base, edge sets joined back to symbols). I can reproduce your arms exactly — base → this tip: LOST 12, GAINED 59, fuzzy 13 → 60.

The 59 appear because the reachability filter runs on the candidate set: it thins a crowd of resolve definitions down to one, and finalCandidates.length === 1 then commits to it. Moving the same check onto the survivor — reachability may reject a unique guess, but never manufacture one — keeps the fix and removes the regression without any knowledge of the binding:

  const callableCandidates = applyLanguageGate(
    candidates.filter((n) => callableKinds.has(n.kind)),   // no reachability here
    ref
  );
  …
  if (finalCandidates.length === 1 && isLexicallyReachable(finalCandidates[0]!, ref, context)) {

Same tree, that variant on top of your commit: base → LOST 12, GAINED 0, fuzzy 13 → 1; against your tip it is LOST 59, GAINED 0 — the 12 correct removals stay, all 59 pluginContainer.ts:resolve edges go, and nothing else moves. Your two-file fixture still passes (the nested text() is rejected as the sole survivor rather than filtered out; the in-container call resolves as before).

It composes with #1713 rather than replacing it: #1713 answers "is the target even in the graph", this answers "did filtering create the uniqueness". The bare-import guard alone leaves 15 (13 → 16 in your table) — your const resolve = … arrow and this.resolve cases — and those go with this too, since the arrow is not a candidate at all and the method call's crowd is not thinned to one. If you want it in this PR, it is a five-line change to matchFuzzy; happy to send it as a commit on your branch or as a follow-up, whichever you prefer.

[danusha2345]

Following up on the variant above with a concrete ask. #1713 landed standalone off main, and the survivor-side reachability check is best measured the same way — its own branch, its own base, base → LOST 12, GAINED 0 as a claim anyone can re-run — rather than stacked on ef2169e where the delta is a statement about two changes.

Would you be fine with a standalone PR off main that carries this PR's two-file fixture (__tests__/fuzzy-lexical-reach.test.ts, credited to you as author in the commit) plus the five-line change to matchFuzzy, with #1709 linked as the origin? Or, if you'd rather keep the fixture here, I'll write my own and only link this thread. Either is fine; I'll wait a bit for your preference before opening anything.

[bompus]

Happy for you to drive this, but two corrections first — the second is the one that matters.

On the stacking. This PR isn't stacked. ef2169e is its own single commit, not a base underneath it: one commit, base b9ca4b7 (which is main), three files, and GitHub reports it mergeable. So a base → branch measurement here is already a statement about one change, and re-creating it off main wouldn't make the delta any cleaner than it already is.

On why it's marked not-ready. That isn't a base or hygiene issue, and rebasing won't clear it. I measured this change on vitejs/vite (1,635 files) and it is a net increase in wrong edges: 27,499 → 27,546, 12 lost and 59 gained. The 12 removals are exactly right — genuinely nested functions the call site cannot reach. The 59 additions are a new false-positive class, 52 calls and 7 imports, and nearly all of them land on pluginContainer.ts:resolve from playground vite configs where the resolve being called is node:path's and isn't in the graph at all. Same counts on the wasm and kernel arms.

The mechanism follows from the fix rather than contradicting it. vite has around a dozen things named resolve, most of them nested. Before, the matcher saw a crowd and declined; filtering the unreachable ones leaves a single survivor, so it commits — without checking that the call site's binding is even internal, when the node:path import is right there on line 1.

So the practical point for your plan: a standalone PR carrying this matchFuzzy change off main would bring those 59 edges with it. The filter needs the survivor-side guard landing first or alongside, which is what #1713 is about. That ordering, not the base, is what's holding this.

On the fixture — yes, please use it, and the author credit you offered is appreciated. It's a two-file reachability case and it should live wherever the change lands. If it's more convenient to write your own, that's fine too.

I'd suggest keeping this one as-is for now: I'll rebase and mark it ready once the survivor case is handled, or close it if the change ends up landing in your PR instead. Either outcome is fine by me — I care about the ordering, not about which PR carries it.

[danusha2345]

Thanks — fixture taken with your authorship on the commit: #1718.

One clarification, because it changes the ordering point. #1718 does not carry this PR's matchFuzzy change and does not bring the 59. It is a different check: reachability is tested on the one candidate matchFuzzy would commit to, not applied as a filter over the candidate set, so a crowd of resolve definitions stays a crowd and never thins to a single survivor. Measured against main (b9ca4b7) on the same vite checkout: LOST 12, GAINED 0, fuzzy 13 → 1, every other resolver at zero — the same 12 removals as here, and the pluginContainer.ts:resolve edges never appear. It composes with #1713 rather than depending on it.

Your fixture is unchanged in it and passes; four direct matchFuzzy cases pin the difference, and swapping in this PR's candidate-set filter fails exactly the "closure + method is ambiguous" one.

On "not stacked": agreed, this PR is one commit off main — I was loose with the word. What I meant was only that a base → branch measurement of the survivor variant needs its own branch, which #1718 now is.

[bompus]

Closing in favor of #1718, which fixes #1708 correctly where this doesn't.

The bug this PR identified is real, and the fix is wrong. Filtering unreachable candidates out of the set makes the strategy see a crowd of one and commit to it — which is how this traded 12 correct removals for 59 wrong additions on vite, nearly all of them import { resolve } from 'node:path' landing on pluginContainer.ts:resolve. #1718 tests reachability on the single candidate matchFuzzy would already have committed to, so a crowd stays a crowd and the uniqueness is never manufactured. Measured −12 / +0, and it holds by construction rather than by measurement: declining on that branch is terminal at both call sites, so it can only remove an edge, never add one.

The fixture from this PR lives on in #1718 with my authorship. Nothing here is lost.

Thanks @danusha2345 for the reproduction and the credit.