fix(review): let a file's own @import outrank the workspace-wide class fallback

Found by running the local preflight harness before pushing rather than after.
One of its five findings is a real hole in R2-2; the rest are documentation that
overclaimed.

R4-1 (valid, REPRODUCED) — the container channel preempted the file's own
`@import`. R2-2 added the module channel as a FALLBACK after
`findClassBindingInScope`, and that order is wrong: `findClassBindingInScope`
does not stop at the scope chain. When its `isClassLike` walk misses — and a
namespace handle binds a Module, so it always misses — it falls back to
`scopes.qualifiedNames`, a workspace-wide index, and answers with the unique def
of that name anywhere in the repo. A container named `dom_utils` in a file
`Element.zig` never imports therefore captured
`bridge.accessor(dom_utils.compare, …)`, binding
`Method:src/webapi/decoy.zig:dom_utils.compare#2` while the module channel that
would have answered correctly was never reached. R3-1's shadow guard cannot
catch it: the import binds at MODULE scope, which that guard treats as the floor.

Fixed by trying the module channel FIRST. An `@import` written in this file is
the strongest available statement about what the name means here and outranks a
global uniqueness guess; when the handle is not an import of this file the
channel answers nothing and the container path runs exactly as before. Pinned by
`decoy.zig` and a strengthened assertion on the existing module-owner test,
which fails on the old order.

R4-2 — the cause documentation named shapes nothing captures. `tools.ts`
illustrated `callableValueReferences` with "a callback argument", "a stored
function pointer" and `qsort(xs, n, sz, compareItems)`. Only Zig captures a call
argument or a const initialiser; JS/TS capture only object-literal property
values, and C has no value-ref rule, so the `qsort` example is counted in no
language. Both cause blocks now name the captured shapes and say that a bare
JS/TS callback argument is not among them, so a 0 does not rule it out.

R4-3 — the same block exempted itself from the re-index caveat this PR proves it
needs. "Read from the graph, so it needs no index-time metadata" is true of the
probe and false of the edges: an index built before a language emitted these
captures has none and reports 0 — the warm-cache failure R2-1 bumped
SCHEMA_BUMP for. It now says to re-analyze before reading a 0 as measured.

R4-4 — the dispatchability canary was narrower than its own promise. Its header
claimed it fails on "a fourth language emitting value-ref at all"; it reads
`languages/<dir>/query.ts`, so Vue — which owns no query and borrows
`emitTsScopeCaptures` / `emitJsScopeCaptures` — emits value-refs while the
assertion lists three languages, and a capture synthesized in code is invisible
to it. The case now asserts on query OWNERS, which is what it checks and is
sound because a delegating language inherits the rules it borrows; the header
states the synthesized-capture gap instead of letting a green tick imply it away.

R4-5 — the BARE docstring described a lexical walk that is not one.
`findCallableBindingInScope` applies the callable predicate WHILE walking, so a
nearer parameter or local is stepped over: the defect R3-1 fixed on the container
channel, unguarded here, pre-existing since #2437 and reachable in JS/TS. Out of
scope for #3399, so behaviour is unchanged and the sentence now says what the
walk does rather than implying a guarantee it does not give.

Gates: tsc --noEmit clean, npm run build clean, prettier clean.
`test/integration/resolvers` 3,632 passed / 3 skipped (70 files);
`test/unit/scope-resolution` 2,015 passed (120 files);
`impact-callable-value-references` 7 passed under `lbug-db`. Bench --check:
receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages),
scope-emission, callable-value-flow all PASS with no baseline edited.
This commit is contained in:
Navid EMAD 2026-09-08 18:47:37 +02:00
parent bd6e577ef4
commit 1c7c05ffc4
No known key found for this signature in database
6 changed files with 158 additions and 13 deletions

View file

@ -537,3 +537,87 @@ reference AND the hedge, and why that is still the right trade.
`callable-value-flow` failed once on its TIMING budget (widening overhead
2.006 > 1.9) with a byte-identical fingerprint, then passed twice at 1.788 /
1.813 — machine load, not a regression.
---
## Review round 4 — local preflight, before pushing
`gitnexus-check[bot]` is a hosted GitHub App and cannot be run locally, so the
`.local-preflight/` harness (out of git) reproduces the two halves its output is
built from: the deterministic gates and blast radius off this checkout's own
graph, and an LLM reviewer pointed at the failure classes this bot has actually
reported here. Run on `3bd1337a` it returned five findings; one was a real hole
in R2-2, which is the point of running it before pushing rather than after.
**R4-1 (valid, REPRODUCED, fixed) — the container channel preempted the file's
own `@import`.** R2-2 added the module channel as a FALLBACK after
`findClassBindingInScope`. That is the wrong order, because
`findClassBindingInScope` does not stop at the scope chain: when its `isClassLike`
walk misses — and a namespace handle binds a Module, so it always misses — it
falls back to `scopes.qualifiedNames`, a workspace-wide index, and answers with
the unique def of that name anywhere in the repo. So:
// decoy.zig — never imported by Element.zig
pub const dom_utils = struct { pub fn compare(a: u8, b: u8) u8 {…} };
// Element.zig
const dom_utils = @import("dom_utils.zig");
pub const comparator = bridge.accessor(dom_utils.compare, null, .{});
bound `Method:src/webapi/decoy.zig:dom_utils.compare#2` — a wrong edge, and the
module channel that would have answered correctly was never reached. R3-1's
shadow guard cannot catch it either: the import binds at MODULE scope, which the
guard treats as the floor.
Fixed by trying the module channel FIRST. An import written in this file is the
strongest available statement about what the name means here, and it outranks a
global uniqueness guess; when the handle is not an import of this file the
channel answers nothing and the container path runs exactly as before. Pinned by
`decoy.zig` plus a strengthened assertion on the existing module-owner test,
which fails on the old order.
**R4-2 (valid, fixed) — the cause documentation named shapes nothing captures.**
`tools.ts` illustrated `callableValueReferences` with "a callback argument", "a
stored function pointer" and `qsort(xs, n, sz, compareItems)`. Only Zig captures
a call argument or a const initialiser; JS/TS capture only object-literal
property values, and C has no value-ref rule at all, so the `qsort` example is
counted in no language. Both cause blocks now name the shapes that are actually
captured and say plainly that a bare JS/TS callback argument is not among them,
so a 0 does not rule it out.
**R4-3 (valid, fixed) — the same block exempted itself from the re-index caveat
this PR proves it needs.** "Read from the graph, so it needs no index-time
metadata" is true of the probe and false of the edges: an index built before a
language emitted these captures has none and reports 0 — which is exactly the
warm-cache failure R2-1 bumped `SCHEMA_BUMP` for. The doc now says to re-analyze
before reading a 0 as measured.
**R4-4 (valid, fixed) — the dispatchability canary was narrower than its own
promise.** Its header said it fails on "a fourth language emitting `value-ref` at
all"; it reads `languages/<dir>/query.ts`, so Vue — which owns no query and
borrows `emitTsScopeCaptures` / `emitJsScopeCaptures` — emits value-refs while
the assertion lists three languages, and a capture synthesized in code (the
mechanism `@reference.static-gated` uses) is invisible to it entirely. The case
now asserts on query OWNERS, which is what it actually checks and is sound
because a delegating language inherits the classification of the rules it
borrows, and the header states the synthesized-capture gap rather than leaving a
green tick to imply it away.
**R4-5 (valid, fixed as documentation) — the BARE docstring described a lexical
walk that is not one.** "resolved up the lexical chain, which is what an
unqualified name means" — `findCallableBindingInScope` applies the callable
predicate WHILE walking, so a nearer parameter or local is stepped over. Same
defect R3-1 fixed on the container channel, unguarded here, pre-existing since
#2437 and reachable in JS/TS. Out of scope for #3399, so the behaviour is
unchanged and the sentence now says what the walk does instead of implying a
guarantee it does not give.
### Gates after review round 4
- `tsc --noEmit` clean, `npm run build` clean, `prettier --check` clean.
- `test/integration/resolvers` — 3,632 passed / 3 skipped (70 files).
- `test/unit/scope-resolution` — 2,015 passed (120 files).
- `impact-callable-value-references` under `lbug-db` — 7 passed.
- Bench `--check`: `receiver-resolution`, `zig-cross-file-resolution`,
`scope-capture` (15 languages), `scope-emission`, `callable-value-flow` all
PASS, no baseline edited.

View file

@ -82,7 +82,16 @@ export const PROPERTY_DISPATCH_CONFIDENCE = 0.7;
* Two shapes, and the difference matters:
*
* - BARE (`{ handler: onClick }`, `register(onTick)`) — the name is resolved
* up the lexical chain, which is what an unqualified name means.
* up the lexical chain, which is what an unqualified name means. Be exact
* about what that walk does, because it is not a plain lexical lookup:
* `findCallableBindingInScope` applies the callable predicate WHILE walking,
* so a scope binding the name to a parameter or a local contributes nothing
* and the walk continues outward. A nearer non-callable binding is stepped
* over — the same shape the qualified path guards against below, unguarded
* here. Pre-existing (#2437) and out of scope for #3399; reachable in JS/TS
* (`function outer(handler) { return { h: handler }; }` beside a top-level
* `function handler`), and narrow in Zig, which rejects a local shadowing a
* container declaration.
*
* - QUALIFIED (`bridge.accessor(Element.getNamespaceUri, …)`) — the source
* WROTE the owner, so the lexical chain is the wrong instrument. It gives
@ -94,12 +103,13 @@ export const PROPERTY_DISPATCH_CONFIDENCE = 0.7;
* allows a same-named callable in a nested container, Zig included.
*
* So a qualified site resolves through its receiver: name the owner, then take
* the member off that owner. An owner is either a CLASS-like container or a
* MODULE — `Element.getNamespaceUri` and `utils.compare` are the same shape
* the member off that owner. An owner is either a MODULE or a CLASS-like
* container — `utils.compare` and `Element.getNamespaceUri` are the same shape
* written against the two kinds of namespace a language has, and the member-call
* path already resolves both (receiver-bound-calls Case 2 / Case 1). Both are
* tried here for the same reason: see `findNamespaceValueRefTarget` for why a
* module receiver cannot simply be declined.
* path already resolves both (receiver-bound-calls Case 1 / Case 2). Both are
* tried here, MODULE FIRST: see `findNamespaceValueRefTarget` for why a module
* receiver cannot simply be declined, and the comment on the call below for why
* the container channel must not go first.
*
* If neither channel names the owner, or the owner is named but owns no such
* callable, this DECLINES rather than falling back to the lexical walk.
@ -130,6 +140,25 @@ function resolveValueRefTarget(
if (receiverName === undefined) {
return findCallableBindingInScope(site.inScope, site.name, scopes);
}
// NAMESPACE FIRST, and the order is load-bearing. `findClassBindingInScope`
// does not stop at the scope chain: when its `isClassLike` walk misses — and a
// namespace handle binds a Module, so it always misses — it falls back to
// `scopes.qualifiedNames`, a WORKSPACE-wide index, and answers with the unique
// def of that name anywhere in the repo. Trying it first therefore lets a
// same-named container in a file this one never imported preempt the `@import`
// this file actually wrote:
//
// const dom_utils = @import("dom_utils.zig"); // namespace-only module
// … bridge.accessor(dom_utils.compare, …) // → decoy.zig's compare
//
// The shadow guard below cannot catch it: the import binds at MODULE scope,
// which the guard treats as the floor. An import written in this file is the
// strongest statement about what the name means here, so it outranks a global
// guess — and when the handle is not an import of this file, this answers
// nothing and the container channel runs exactly as before.
const viaNamespace = findNamespaceValueRefTarget(site, filePath, receiverName, scopes);
if (viaNamespace !== undefined) return viaNamespace;
const owner = findClassBindingInScope(site.inScope, receiverName, scopes);
if (owner !== undefined) {
// The container lookup is a CLASS-ONLY walk: `walkScopeChain` filters by
@ -155,9 +184,7 @@ function resolveValueRefTarget(
if (member === undefined || !CALL_TARGET_TYPES.has(member.type)) return undefined;
return member;
}
// A receiver that is not class-like may still be a MODULE — the other kind of
// owner a qualified name can have. See `findNamespaceValueRefTarget`.
return findNamespaceValueRefTarget(site, filePath, receiverName, scopes);
return undefined;
}
/**

View file

@ -299,7 +299,7 @@ COMPLETENESS OF incoming: alongside symbol/incoming/outgoing the result carries
- causes.externalBoundary (unit: call sites) > 0 — the calls left the indexed program (System.out.println, fetch(...)). NOT a defect: no in-graph node could have been reached. An epistemic:'exact' result can carry this.
- causes.dispatchBoundary (unit: symbols) > 0 — DI or interface dispatch: that many symbols sit on or beyond a boundary static analysis cannot cross. Irreducible. A symbol count, not a site count — per-site multiplicity is not retained for these edges — so compare its magnitude with receiverTyping, not its exact value. A framework runtime-proxy boundary can make epistemic lower-bound while this value remains 0 because endpoint metadata proves the gap but cannot count omitted symbols.
- causes.undecidedSatisfaction (unit: unjudged interface/type pairs) > 0 — the analyzer could not decide whether a type satisfies an interface, so no IMPLEMENTS edge exists and no dispatch boundary was left for the walk to notice. Usually fixable by making the missing dependency available to analysis.
- causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE instead of calling it (a registration table, a callback argument, a stored function pointer). The reference is in the graph as a USES edge; the call made THROUGH the value is not, because it is dispatched later from wherever the value was stored. incoming.calls is therefore a floor. Follow the USES edges to find the registration, then the code that reads it. It is 0 when the analyzer DID synthesize the dispatch through a registered property key. That exclusion is per SYMBOL, not per registration: a target with BOTH a followed registration and an unfollowed escape reads 0 here, so a 0 means 'no unfollowed registration was proven', not 'this symbol escapes nowhere'. A 0 alongside epistemic 'lower-bound' can also mean the probe itself could not run — read boundaries for which.
- causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE instead of calling it (a Zig registration table or const initialiser, a JS/TS object-literal property value). A bare callback argument in JS/TS is not captured today and is not counted, so a 0 does not rule that shape out; nor does it, on an index built before the language emitted these captures — re-analyze first. The reference is in the graph as a USES edge; the call made THROUGH the value is not, because it is dispatched later from wherever the value was stored. incoming.calls is therefore a floor. Follow the USES edges to find the registration, then the code that reads it. It is 0 when the analyzer DID synthesize the dispatch through a registered property key. That exclusion is per SYMBOL, not per registration: a target with BOTH a followed registration and an unfollowed escape reads 0 here, so a 0 means 'no unfollowed registration was proven', not 'this symbol escapes nowhere'. A 0 alongside epistemic 'lower-bound' can also mean the probe itself could not run — read boundaries for which.
REQUIRES RE-INDEX: causes.scopeExtractionFiles, causes.receiverTyping, causes.externalBoundary, causes.undecidedSatisfaction, and framework runtime-proxy boundary detection depend on index-time metadata that only a current analyzer writes. Against an older index the metadata can be absent, which is indistinguishable from "nothing was dropped" unless the schema probe detects the stale index — re-run \`gitnexus analyze\` before trusting a zero or an apparently exact result.
@ -498,7 +498,7 @@ Output includes:
- causes.dispatchBoundary (unit: symbols) > 0 — DI or interface dispatch: that many symbols sit on or beyond a boundary a static walk cannot cross. Irreducible. A symbol count, not a site count — per-site multiplicity is not retained for these edges — so compare its magnitude with receiverTyping, not its exact value. A framework runtime-proxy boundary can make epistemic lower-bound while this value remains 0 because endpoint metadata proves the gap but cannot count omitted symbols.
- causes.undecidedSatisfaction (unit: unjudged interface/type pairs) > 0 — the analyzer could not DECIDE whether a type satisfies an interface (a type in a required signature named a package it could not resolve), so no IMPLEMENTS edge exists and no dispatch boundary was left for the walk to notice. Distinct from every cause above, which count decided facts that could not be attributed; this one counts questions never answered. It is the only cause that shortens a result WITHOUT leaving a trace in the graph, so an unhedged zero on a symbol reached only through such an interface would otherwise read as 'nobody calls this'. Usually fixable: it most often means a dependency is missing from the analyzed tree.
- causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE rather than calling it: 'bridge.accessor(Element.getNamespaceUri, ...)', '{ onClick: handler }', a comparator handed to a sort. The registration IS modelled (a USES edge); the invocation through the stored value is NOT, because it happens later via a struct field, a registry lookup or comptime reflection. So impactedCount is a floor and a LOW risk verdict on such a symbol is a floor too. Unlike dispatchBoundary this is often reducible — it usually means the language provider does not yet follow that store/load — but until it is, do NOT read an empty or small caller set as 'safe to change'. It is an exact count, not a capped sample. It is 0 when the analyzer DID synthesize the dispatch through a registered property key, and epistemic stays 'exact' on that account. That exclusion is symbol-level, not edge-level — the graph does not record which registration produced which synthesized call — so a symbol with a mix of followed and unfollowed registrations also reads 0: treat a 0 as 'no unfollowed registration was proven', not as proof the value escapes nowhere. A 0 alongside epistemic 'lower-bound' can instead mean the probe could not run at all, so read boundaries to tell those apart. Read from the graph, so it needs no index-time metadata.
- causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE rather than calling it: 'bridge.accessor(Element.getNamespaceUri, ...)' and 'pub const h = onReset;' in Zig, '{ onClick: handler }' in JS/TS. Those are the shapes actually captured today — a bare callback argument in JS/TS ('qsort'-style, 'setTimeout(tick)') is NOT one of them and is not counted, so a 0 here does not rule that shape out. The registration IS modelled (a USES edge); the invocation through the stored value is NOT, because it happens later via a struct field, a registry lookup or comptime reflection. So impactedCount is a floor and a LOW risk verdict on such a symbol is a floor too. Unlike dispatchBoundary this is often reducible — it usually means the language provider does not yet follow that store/load — but until it is, do NOT read an empty or small caller set as 'safe to change'. It is an exact count, not a capped sample. It is 0 when the analyzer DID synthesize the dispatch through a registered property key, and epistemic stays 'exact' on that account. That exclusion is symbol-level, not edge-level — the graph does not record which registration produced which synthesized call — so a symbol with a mix of followed and unfollowed registrations also reads 0: treat a 0 as 'no unfollowed registration was proven', not as proof the value escapes nowhere. A 0 alongside epistemic 'lower-bound' can instead mean the probe could not run at all, so read boundaries to tell those apart. Read from the graph, so it needs no index-time metadata BEYOND the edges being there: an index built by an analyzer that did not yet emit this language's value-ref captures has none, and reports 0. Re-analyze before reading a 0 as measured.
REQUIRES RE-INDEX: causes.scopeExtractionFiles, causes.receiverTyping, causes.externalBoundary, causes.undecidedSatisfaction, and framework runtime-proxy boundary detection depend on index-time metadata that only a current analyzer writes. Against an older index the metadata can be absent, which is indistinguishable from "nothing was dropped" unless the schema probe detects the stale index — re-run \`gitnexus analyze\` before trusting a zero or an apparently exact result.

View file

@ -0,0 +1,14 @@
// A container whose NAME collides with `Element.zig`'s `dom_utils` import
// handle, in a file `Element.zig` never imports.
//
// `findClassBindingInScope` does not stop at the scope chain: when its
// `isClassLike` walk misses — and a namespace import binds a Module, not a
// class — it falls back to `scopes.qualifiedNames`, a WORKSPACE-wide index, and
// answers with the unique def of that name. This struct is that unique def. A
// registration written `dom_utils.compare` in `Element.zig` must still bind
// `dom_utils.zig`'s function, not this one: the file said which module it meant.
pub const dom_utils = struct {
pub fn compare(a: u8, b: u8) u8 {
return if (a < b) a else b;
}
};

View file

@ -348,6 +348,13 @@ describe.skipIf(!zigAvailable)('Zig idioms (zig-idioms fixture)', () => {
// The member-CALL path already resolves `dom_utils.compare()` through the
// file's namespace import; the registration reads the same channel.
expect(valueRefs).toContain('JsApi → compare');
// And it must be dom_utils.zig's `compare`, not `decoy.zig`'s. That file
// declares a CONTAINER also called `dom_utils`, with its own `compare`,
// and `Element.zig` never imports it. `findClassBindingInScope` does not
// stop at the scope chain: a namespace handle binds a Module, so the
// `isClassLike` walk misses and its workspace-wide `qualifiedNames`
// fallback answers with that unique container — preempting the `@import`
// this very file wrote. The written import has to outrank a global guess.
expect(valueRefTargetIds.filter((id) => id.includes('compare'))).toEqual([
'Function:src/webapi/dom_utils.zig:compare',
]);

View file

@ -31,6 +31,15 @@
* This test fails instead. If it fails, do not relax it: go and decide what
* `callableValueReferenceBoundaries` should do about a mixed symbol (the
* options are recorded at the exclusion site), then update this file.
*
* WHAT IT DOES NOT COVER, stated so the green tick is not read as more than it
* is. It reads query SOURCES, so a capture synthesized in code rather than
* matched by a rule — the mechanism `@reference.static-gated` uses — can break
* the partition with this test green. A provider adding one has to come here by
* hand. Languages that own no query and delegate to another's captures (Vue →
* `emitTsScopeCaptures` / `emitJsScopeCaptures`) are covered transitively, by
* the rules they borrow, which is why the last case asserts on query OWNERS
* rather than on the set of languages that can emit a value-ref.
*/
import { describe, it, expect } from 'vitest';
import fs from 'node:fs';
@ -132,10 +141,14 @@ describe('value-ref dispatchability partition', () => {
expect(rules.filter((r) => r.includes(PROPERTY_KEY))).toEqual([]);
});
it('no OTHER language emits a value-ref capture', () => {
// The three above are hand-classified. A fourth language emitting
it('no OTHER language OWNS a value-ref rule', () => {
// The three above are hand-classified. A fourth query declaring
// `value-ref` has not been classified by anyone, so the exclusion's premise
// is unverified for it — classify it here and in the exclusion's comment.
// "Owns", not "emits": Vue has no query of its own and borrows TypeScript's
// and JavaScript's captures, so it inherits their classification rather than
// needing one. A capture synthesized in code owns no rule either and is
// invisible here — see the header.
const languagesDir = path.join(
path.dirname(fileURLToPath(import.meta.url)),
'../../../src/core/ingestion/languages',