From cf53bbaaf6110d32fd5c87ee4d3fa2e252c94919 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 8 Sep 2026 17:46:21 +0200 Subject: [PATCH] fix(review): bump the parse-cache schema and resolve module-owned value references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 on #3219. Four inline items, all reproduced against the worktree before deciding. P1 — the new captures could stay INERT on a warm parse cache. Adding `@reference.value-ref` rules to `ZIG_SCOPE_QUERY` changes `ParsedFile.referenceSites`, which is a PARSE-TIME fact, but `SCHEMA_BUMP` stayed at 93. A repo indexed before this branch and re-analyzed after it replays the old, empty site list for every unchanged `.zig` file — `--force` included, since shards are content-addressed — so no USES edge is emitted, the boundary probe measures a real zero, and `impact` on a registered accessor goes back to `epistemic: "exact"`. #3399 un-fixed on the incremental path most users are on, with every cold-run test still green. DECISIONS D1-1's "zero changes to … the schema" conflated the graph schema with the cache schema. Bumped 93 -> 98, not 94: #3190 claims 94 and #3179 claims 94 through 97 in one PR. `incremental-parse-cache.test.ts` re-pinned, with 93-97 added to the taken list. Re-check against origin/main and open PRs immediately before merging. P2 — a qualified value reference through a MODULE was declined with no hedge. R1-2 resolves a written receiver with `findClassBindingInScope`, which requires `isClassLike`; a namespace-only `@import` handle is not class-like, so const dom_utils = @import("dom_utils.zig"); // no `@This()` in that file pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); resolved to nothing. That is not the conservative half of R1-2's trade-off: a declined site emits NO edge, so there is nothing for the probe to read and `impact` on `compare` reports `exact`. Silence, not a hedge — and the pass comment claiming otherwise was wrong on this path. `resolveValueRefTarget` now tries the second kind of owner a qualified name can have. `findNamespaceValueRefTarget` reads the file's `namespace` import edges for the handle and the target module's own `origin: 'local'` module-scope bindings for the member — the same channel `receiver-bound-calls` Case 1 already trusts for `dom_utils.compare()`, with Case 1's three guards for Case 1's reasons: `isNamespaceNameShadowed`, local-origin bindings only, and two distinct defs under one name resolve nothing. `CALL_TARGET_TYPES` gates module owners exactly as it gates container owners. Language-neutral: it reads generic namespace import edges, names no language. Still declined, deliberately: a receiver this index knows under no name at all (the `@This()`-alias case, owners outside the workspace). There the alternative is a confident edge to a lexically-nearer function the source did not name. P2 — `impact-callable-value-references.test.ts` was in neither vitest list. It opens a real engine via `withTestLbugDB(poolAdapter: true)`, so TESTING.md puts it in the `lbug-db` include list and the `default` exclude list; it was in neither, so `default` also collected it into the parallel pool. Added next to its `impact-epistemic-lower-bound` sibling in both arrays. P3 — DECISIONS.md was stale against HEAD and embedded host paths. D2-3 still described the `LIMIT 50` that R1-5 removed and D1-3 still described the receiver-blind resolution that R1-2 replaced; both now carry explicit "superseded by" pointers. The `~/code/...` and mise-node paths are replaced with placeholders. Two smaller corrections the new cause made necessary: - `formatImpactResult`'s `lower-bound` header hard-coded "callers binding via DI / dynamic dispatch", which now contradicts the value-reference bullet printed directly under it. The bullets carry the cause; the header only states that the count is a floor. - `tools.ts` said a `causes.callableValueReferences` of 0 means "nothing was missed". The dispatch exclusion is symbol-level, not edge-level, so a target with both a followed registration and an unfollowed escape also reads 0. The docs now say a 0 means "no unfollowed registration was proven". Fixture: `src/webapi/dom_utils.zig` (namespace-only) plus three cases in `Element.zig` — the module-qualified registration, a non-callable module member, and a `u8` parameter shadowing the handle. The shadow case was verified to FAIL with the guard disabled, so it is not passing for an unrelated reason. Gates: `tsc --noEmit` clean, `npm run build` clean, prettier clean. `resolvers/` 3,630 passed / 3 skipped; `unit/scope-resolution` 2,010 passed; `impact-callable-value-references` 7 passed under `lbug-db`; `incremental-parse-cache` 40 passed; eval formatters 104 passed. Bench --check all PASS with no baseline edited: receiver-resolution, zig-cross-file-resolution, scope-emission, callable-value-flow, scope-capture (15 languages), python-scope. --- DECISIONS.md | 140 ++++++++++++++++-- gitnexus/src/cli/eval-server.ts | 10 +- .../passes/property-dispatch.ts | 106 +++++++++++-- gitnexus/src/mcp/tools.ts | 4 +- gitnexus/src/storage/parse-cache.ts | 15 +- .../zig-idioms/src/webapi/Element.zig | 27 ++++ .../zig-idioms/src/webapi/dom_utils.zig | 18 +++ .../test/integration/resolvers/zig.test.ts | 33 +++++ .../test/unit/incremental-parse-cache.test.ts | 18 ++- gitnexus/vitest.config.ts | 2 + 10 files changed, 340 insertions(+), 33 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig diff --git a/DECISIONS.md b/DECISIONS.md index 1f555c7da..013f72864 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -14,10 +14,14 @@ rejection: lightpanda-io/browser#3399. Local build reports **1.6.11** (expected: `package.json` on main says 1.6.11 and the 1.6.12-rc.* tags are CI-published without a committed bump). +Measured against a clean index of `lightpanda-io/browser` (the repo #3399 was +filed from), built from this checkout: + ``` -cd ~/code/GitNexus/gitnexus && $HOME/.local/share/mise/installs/node/26.8.1/bin/npm run build # exit 0 -rm -rf ~/code/browser/.gitnexus -cd ~/code/browser && node ~/code/GitNexus/gitnexus/dist/cli/index.js analyze --index-only --skip-agents-md --no-stats +cd /gitnexus && npm run build # exit 0 +rm -rf /.gitnexus # cold index: see D1-note below +cd && node /gitnexus/dist/cli/index.js \ + analyze --index-only --skip-agents-md --no-stats ``` Index: **30,222 nodes | 71,070 edges | 997 clusters | 1155 flows** (34.7 s). @@ -69,10 +73,13 @@ sets `dispatch: 0`). That precedent exists because endpoint metadata *cannot* count omitted symbols. Here the count is available and real, so publishing zero would be inventing an absence. -**D2-3. Unit = distinct referrer SYMBOLS, capped at 50.** The question the count -serves is "how many places does this value escape from"; a table registering the -same callable twice is still one table. The `LIMIT 50` bounds the work on a -promiscuous target — the note only needs to justify "at least N". +**D2-3. Unit = distinct referrer SYMBOLS.** The question the count serves is +"how many places does this value escape from"; a table registering the same +callable twice is still one table. *Originally decided with a `LIMIT 50` to +bound the work; **superseded by R1-5**, which replaced it with +`COUNT(DISTINCT other.id)` after the review pointed out that a capped row count +published a ceiling under a name documented as a symbol count. The current code +has no cap.* **D2-4. Upstream only.** A reference INTO a symbol says nothing about what that symbol reaches, so a `downstream` walk is not shortened by it. Same gate the @@ -126,14 +133,20 @@ qualified (`bridge.accessor(Element.getNamespaceUri, …)`) and its sibling is b (`bridge.accessor(_tagName, …)`); the real table uses both. For the qualified form `@reference.name` is the MEMBER and the object is captured as `@reference.receiver`, because the member is the name the scope walk resolves. -*Trade-off accepted:* the property-dispatch pass ignores the receiver, so a -qualified reference resolves by tail name and could in principle bind a -same-named local callable. Measured on the real corpus this does not bite: all -3,169 emitted edges land on `Method` (3,146) or `Function` (23), and the -94 registrations in `Element.zig`'s `JsApi` read as the DOM Element API surface -one for one. *Rejected:* resolving through the receiver — that is the -receiver-bound-call path, a much larger change, and the callable gate already -carries the precision. +*Trade-off originally accepted:* the property-dispatch pass ignored the receiver, +so a qualified reference resolved by tail name and could in principle bind a +same-named local callable. Measured on the real corpus at the time this did not +appear to bite: all 3,169 emitted edges landed on `Method` (3,146) or `Function` +(23), and the 94 registrations in `Element.zig`'s `JsApi` read as the DOM +Element API surface one for one. *Rejected at the time:* resolving through the +receiver. + +***Superseded by R1-2 and R2-2.*** The trade-off was wrong: the bot's +counter-example reproduced, so the pass now resolves a qualified site through its +written owner — a class-like container (R1-2) or a module handle (R2-2) — and +declines rather than falling back to the lexical walk. The receiver is no longer +ignored, and the corpus census that justified the original decision is recorded +under R1-2 as the measurement of what declining costs. **D1-4. Rules are deliberately broad; the CALLABLE GATE is the filter.** `js.Bridge(Element)` and `register(count)` match too. `findCallableBindingInScope` @@ -158,7 +171,7 @@ match baseline`), so no rebaseline was needed either way. ## Measured result — D1 + D2 together -Re-analyzed from scratch (`rm -rf ~/code/browser/.gitnexus`) with the same +Re-analyzed from scratch (`rm -rf /.gitnexus`) with the same command as the baseline. | | baseline | after | delta | @@ -339,3 +352,98 @@ count, dispatch-modelled zero, probe-failure zero-with-note). - Bench `--check` re-run after the receiver fix: `scope-capture` (15 languages), `receiver-resolution`, `zig-cross-file-resolution`, `callable-value-flow`, `scope-emission`, `python-scope` — all PASS, no baseline edited. + +--- + +## Review round 2 — PR #3219 tri-engine digest + +Four inline items (one P1, two P2, one P3), plus a documented residual. Each was +reproduced against the worktree before deciding. + +**R2-1 (valid, fixed) — the new captures could stay INERT on a warm parse cache.** +The headline finding, and the one that mattered: `ZIG_SCOPE_QUERY` now emits +`@reference.value-ref`, which changes `ParsedFile.referenceSites` — a PARSE-TIME +fact. `SCHEMA_BUMP` was left at 93, so a repo indexed before this change and +re-analyzed after it replays the old, empty site list for every unchanged `.zig` +file, `--force` included (shards are content-addressed). No USES edge, a real +measured zero at the probe, and `impact` back to `epistemic: "exact"` — #3399 +un-fixed on exactly the incremental path most users are on, with every cold-run +test green. D1-1's claim of "zero changes to … the schema" conflated the graph +schema with the cache schema; only the first was true. + +Bumped 93 → **98**, not 94: #3190 claims 94 and #3179 claims 94 through 97 in one +PR. `incremental-parse-cache.test.ts` re-pinned to 98 with 93–97 added to the +taken list. **Re-check against `origin/main` and open PRs immediately before +merging** — the ledger in that test records two PRs that each did this check once +and still collided. + +**R2-2 (valid, REPRODUCED, fixed) — a qualified value-ref through a MODULE was +declined with no hedge.** R1-2 resolves a written receiver through +`findClassBindingInScope`, which requires `isClassLike`. A namespace-only +`@import` handle is not class-like: + + const dom_utils = @import("dom_utils.zig"); // no `@This()` in that file + pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); + +resolved to nothing. That is not the conservative half of R1-2's trade-off: a +declined site emits NO edge, so the boundary probe measures a real zero and +`impact` on `compare` reports `exact`. Silence, not a hedge — and the pass +comment claiming otherwise was wrong on this path (fixed). + +`resolveValueRefTarget` now tries the second kind of owner a qualified name can +have. `findNamespaceValueRefTarget` reads the file's `namespace` import edges for +the handle and the target module's own `origin: 'local'` module-scope bindings +for the member — the same channel `receiver-bound-calls` Case 1 already trusts +for `dom_utils.compare()`. The three guards are Case 1's, for Case 1's reasons: +`isNamespaceNameShadowed` (a local declaration shadowing the handle suppresses +it), `origin === 'local'` only (a name the target merely imported is not +published as its own — the `namespaceExportsIncludeImportedNames` hub opt-in is a +provider decision this language-neutral pass does not make), and two distinct +defs under one name resolve nothing. + +*Rejected:* the alternative the review offered — persisting hedgeable evidence +for declined sites so `impact` cannot stay `exact`. It needs a new metadata +channel and a re-index (the thing D2-1 was written to avoid) to publish a hedge +in the one case where the answer is actually knowable. Resolving the reference is +both cheaper and strictly more informative. + +Fixture: `src/webapi/dom_utils.zig` (namespace-only) plus three cases in +`Element.zig` — the module-qualified registration, a non-callable module member +(`dom_utils.DEFAULT_NS`, the callable gate applies to module owners too), and a +`u8` parameter shadowing the handle. All three pinned in +`resolvers/zig.test.ts`; the shadow case was verified to FAIL with the guard +disabled, so it is not passing for an unrelated reason. + +*Still declined, deliberately:* a receiver this index knows under no name at all +— the `@This()`-alias case R1-2 already recorded, and owners outside the +workspace. There the alternative is not a hedge either, it is a confident edge to +a lexically-nearer function the source did not name. + +**R2-3 (valid, fixed as proposed) — the new suite was in neither vitest list.** +`impact-callable-value-references.test.ts` uses `withTestLbugDB(poolAdapter: true)`, +so TESTING.md puts it in the `lbug-db` project's include list and the `default` +project's exclude list. It was in neither, so `default`'s `test/**/*.test.ts` +also collected it into the parallel pool — the mmap file-lock flake class that +project exists to serialize. Added next to its `impact-epistemic-lower-bound` +sibling in both arrays. + +**R2-4 (valid, fixed) — this file was stale against HEAD and embedded host paths.** +D2-3 still described the `LIMIT 50` that R1-5 removed and D1-3 still described the +receiver-blind resolution that R1-2 replaced; both now carry explicit *superseded +by* pointers rather than reading as current decisions. The three `~/code/...` and +mise-node paths in the baseline block are replaced with `` / `` +placeholders (CONTRIBUTING.md: no machine-specific paths). + +**Residual, re-affirmed not re-litigated — mixed followed/unfollowed +registrations.** R1-4's exclusion is symbol-level: any inbound `property-dispatch` +CALLS edge zeroes the note. A target with both a followed registration and an +unfollowed escape is therefore not hedged. Two engines rated this P1-to-P3; the +reasoning in R1-4 stands unchanged, and the alternative still fires on every +JS/TS hook table in every codebase. It is a JS/TS shape, not the Zig case this PR +exists for. Documented in the code at the exclusion site. + +### Gates after review round 2 + +- `tsc --noEmit -p tsconfig.json` — clean. `npm run build` — clean. +- `resolvers/zig.test.ts` — 98 passed / 1 skipped, including the three new + module-owner cases. diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index b2f656c39..acb04ef24 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -609,9 +609,17 @@ export function formatImpactResult(result: any): string { } // #1858 — an interface / indirection boundary on the path makes this a lower // bound; surface it so the count is not read as exhaustive. + // + // The header names no specific cause. DI / dynamic dispatch was the only + // producer of `lower-bound` when this was written; #3399 added callables + // named in VALUE position (a registration table, a callback argument), and a + // header that keeps asserting "DI / dynamic dispatch" contradicts the + // `boundaries` bullet printed directly under it. The bullets carry the cause + // — they are generated per-cause by `computeEpistemicBoundary` — so the + // header only has to say that the count is a floor. if (result.epistemic === 'lower-bound') { lines.push( - '⚠️ Lower bound — unresolved indirection on the path (callers binding via DI / dynamic dispatch are not traced; actual impact may be higher):', + '⚠️ Lower bound — unresolved indirection on the path; some callers are not traced and actual impact may be higher:', ); for (const b of result.boundaries || []) lines.push(` • ${b}`); } diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts index 4693b7267..c22f491ac 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts @@ -48,6 +48,7 @@ import { findCallableBindingInScope, findClassBindingInScope, findOwnedMember, + isNamespaceNameShadowed, } from '../scope/walkers.js'; import { VALUE_REF_EDGE_REASON } from '../value-ref-edges.js'; import type { SemanticModel } from '../../model/semantic-model.js'; @@ -92,11 +93,27 @@ 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. If the owner cannot be resolved, or resolves but - * owns no such callable, this DECLINES rather than falling back to the lexical - * walk. Declining costs a reference; falling back would mint a confident edge - * to the wrong target, and `impact` now reports the missing reference as - * `lower-bound` rather than as certainty. + * 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 + * 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. + * + * 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. + * + * Be precise about what declining costs, because it is more than one reference: + * a site that emits NO edge leaves no evidence for `impact`'s value-reference + * probe to read, so the target keeps `epistemic: "exact"` — silence, not a + * hedge. That is why the module channel above exists rather than being waved + * through as "just a decline". What remains declined is the case where the + * written receiver names nothing this index knows at all (a Zig `@This()` alias + * whose name differs from its container's, an owner from outside the workspace): + * there the alternative is not a hedge either, it is a confident edge to a + * lexically-nearer function that the source did not name, and a wrong edge is + * strictly worse than a missing one for a tool whose value is that its edges can + * be trusted. * * `CALL_TARGET_TYPES`, not a hand-rolled label set: `findOwnedMember` also * answers with FIELDS, and a field named like the member would otherwise @@ -104,6 +121,7 @@ export const PROPERTY_DISPATCH_CONFIDENCE = 0.7; */ function resolveValueRefTarget( site: ReferenceSite, + filePath: string, scopes: ScopeResolutionIndexes, model: SemanticModel, ): SymbolDefinition | undefined { @@ -112,10 +130,78 @@ function resolveValueRefTarget( return findCallableBindingInScope(site.inScope, site.name, scopes); } const owner = findClassBindingInScope(site.inScope, receiverName, scopes); - if (owner === undefined) return undefined; - const member = findOwnedMember(owner.nodeId, site.name, model); - if (member === undefined || !CALL_TARGET_TYPES.has(member.type)) return undefined; - return member; + if (owner !== undefined) { + const member = findOwnedMember(owner.nodeId, site.name, model); + 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); +} + +/** + * The second kind of owner: a namespace handle. + * + * `const utils = @import("utils.zig"); register(utils.compare);` — `utils` is a + * MODULE, not a class, so `findClassBindingInScope` answers nothing and the + * class path above declines. Declining here would be a silent hole rather than + * a conservative one: no USES edge is emitted, so + * `callableValueReferenceBoundaries` measures a real zero and `impact` on + * `compare` republishes `exact` — the very claim this feature exists to stop + * making. Nothing downstream can hedge on evidence that was never recorded. + * + * So resolve it, through the SAME channel the member-CALL path already trusts + * for `utils.compare()` (receiver-bound-calls Case 1): the file's namespace + * import edges name the target module, and the target module's own local + * module-scope bindings name its members. `utils.compare` and `utils.compare()` + * disagreeing about what `utils` is would be the anomaly. + * + * The same three guards Case 1 applies, for the same reasons: + * - a LOCAL declaration shadowing the handle suppresses the resolution + * (`isNamespaceNameShadowed`) — `fn f(utils: Decoy) { register(utils.compare) }` + * names the parameter's member, and resolving through the import would be a + * wrong edge rather than a missing one; + * - `origin === 'local'` only, so a name the target file merely IMPORTED is + * not published as its own member here (the `namespaceExportsIncludeImportedNames` + * hub opt-in is a provider decision this language-neutral pass does not make); + * - two distinct defs under one name resolve NOTHING. Never guess a namespace + * member — the whole point of reading the written receiver is precision. + * + * `CALL_TARGET_TYPES` gates the answer for the same reason the class path needs + * it: `utils.DEFAULT_PORT` is a module-scope binding too, and a registration + * table full of constants must keep emitting nothing. + */ +function findNamespaceValueRefTarget( + site: ReferenceSite, + filePath: string, + receiverName: string, + scopes: ScopeResolutionIndexes, +): SymbolDefinition | undefined { + const moduleScopeId = scopes.moduleScopes.get(filePath); + if (moduleScopeId === undefined) return undefined; + const targetFiles: string[] = []; + for (const edge of scopes.imports.get(moduleScopeId) ?? []) { + if (edge.kind !== 'namespace' || edge.localName !== receiverName) continue; + if (edge.targetFile === null) continue; + if (!targetFiles.includes(edge.targetFile)) targetFiles.push(edge.targetFile); + } + if (targetFiles.length === 0) return undefined; + if (isNamespaceNameShadowed(receiverName, site.inScope, scopes)) return undefined; + + let picked: SymbolDefinition | undefined; + for (const targetFile of targetFiles) { + const targetScopeId = scopes.moduleScopes.get(targetFile); + if (targetScopeId === undefined) continue; + const refs = scopes.bindings.get(targetScopeId)?.get(site.name); + if (refs === undefined) continue; + for (const ref of refs) { + if (ref.origin !== 'local' || !CALL_TARGET_TYPES.has(ref.def.type)) continue; + if (picked !== undefined && picked.nodeId !== ref.def.nodeId) return undefined; + picked = ref.def; + } + } + return picked; } export function emitPropertyDispatchCalls( @@ -140,7 +226,7 @@ export function emitPropertyDispatchCalls( for (const parsed of parsedFiles) { for (const site of parsed.referenceSites) { if (site.kind !== 'value-ref') continue; - const def = resolveValueRefTarget(site, scopes, model); + const def = resolveValueRefTarget(site, parsed.filePath, scopes, model); if (def === undefined) continue; const ok = tryEmitEdge(graph, scopes, nodeLookup, site, def, VALUE_REF_EDGE_REASON, seen); diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index c7055f2bd..41e9ff485 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -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, because then nothing was missed; 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 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. 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 (nothing was missed, and epistemic stays 'exact' on that account); 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, ...)', '{ 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. 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. diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 743b810b4..2835430b2 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -735,7 +735,20 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // v93: Zig call captures inside a comptime-false branch carry // `@reference.static-gated` (feat/zig-static-gated-edges); the site gains // `staticGated` and the CALLS edge a BOOLEAN column. -const SCHEMA_BUMP = 93; +// v98 (#3219): `ZIG_SCOPE_QUERY` gained three `@reference.value-ref` rules — +// bare call argument, qualified call argument (with `@reference.receiver`), and +// const-binding initialiser — so a Zig callable named in VALUE position now +// produces a `value-ref` entry in `ParsedFile.referenceSites` where it produced +// none before. These captures are PARSE-TIME facts, so a warm v93 cache replays +// unchanged `.zig` files with zero value-ref sites, `--force` included (shards +// are content-addressed): `emitPropertyDispatchCalls` then emits no USES edge, +// `callableValueReferenceBoundaries` measures a real zero, and `impact` on a +// registered accessor republishes `epistemic: "exact"` — the exact #3399 defect +// this change exists to close, silently un-fixed. 98 is the next free value +// above origin/main (93) and every open PR at the time of writing: #3190 claims +// 94, #3179 claims 94-97. RE-CHECK AGAINST origin/main AND OPEN PRs IMMEDIATELY +// BEFORE MERGING. +const SCHEMA_BUMP = 98; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig index 4c5b78e03..4f915ab42 100644 --- a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig @@ -11,6 +11,11 @@ // A file-as-struct, like every webapi module in the real tree. const Element = @This(); +// A NAMESPACE-only module (no `@This()`), imported under a handle. The bridge +// table below registers one of its functions the same way it registers this +// file's own methods. +const dom_utils = @import("dom_utils.zig"); + _namespace: u8 = 0, // ── Registered accessors ──────────────────────────────────────────────────── @@ -84,8 +89,30 @@ pub const JsApi = struct { // safe answer: a missing reference is recoverable, a confident wrong edge // is not. pub const ticker = bridge.accessor(unresolvable_ns.tick, null, .{}); + + // QUALIFIED value reference through a MODULE handle rather than a container. + // `dom_utils` is a namespace, not a class, so the class-owner lookup answers + // nothing here — and declining would be silent rather than safe: with no + // USES edge, `impact` on `compare` measures a real zero and reports `exact`, + // which is the claim this whole change exists to stop making. + pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); + + // A namespace member that is NOT callable. Module receivers get the same + // callable gate as container receivers — a registration table full of + // constants must keep emitting nothing. + pub const defaultNs = bridge.accessor(dom_utils.DEFAULT_NS, null, .{}); }; +// A LOCAL declaration shadowing the module handle. `dom_utils` here is a `u8` +// parameter with no member of its own; resolving `dom_utils.normalize` through +// the file-level import would attach the registration to a module the source +// did not name at this site — a wrong edge, the failure the same guard prevents +// on the member-CALL path. +pub fn shadowsTheModuleHandle(dom_utils: u8) u8 { + register(dom_utils.normalize); + return dom_utils; +} + // ── Const binding initialiser ─────────────────────────────────────────────── fn onReset(self: *Element) u8 { diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig new file mode 100644 index 000000000..754db29da --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig @@ -0,0 +1,18 @@ +// A NAMESPACE-only module: no `const X = @This()`, so this file declares no +// container symbol of its own. Its members are reachable only through an +// `@import` handle — the second kind of owner a qualified name can have, and +// the one `findClassBindingInScope` cannot answer for. +// +// Lightpanda's `libdom.zig` / `parser.zig` helpers are written exactly this +// way, and they are registered into the same bridge tables as the file-struct +// methods next door. + +pub const DEFAULT_NS: u8 = 7; + +pub fn compare(a: u8, b: u8) u8 { + return if (a > b) a else b; +} + +pub fn normalize(v: u8) u8 { + return v; +} diff --git a/gitnexus/test/integration/resolvers/zig.test.ts b/gitnexus/test/integration/resolvers/zig.test.ts index 0319c310c..32de4f5cf 100644 --- a/gitnexus/test/integration/resolvers/zig.test.ts +++ b/gitnexus/test/integration/resolvers/zig.test.ts @@ -334,6 +334,39 @@ describe.skipIf(!zigAvailable)('Zig idioms (zig-idioms fixture)', () => { expect(valueRefs).not.toContain('JsApi → tick'); }); + it('records a QUALIFIED function value owned by a MODULE, not a container (`bridge.accessor(dom_utils.compare, …)`)', () => { + // `dom_utils` is a namespace-only file — no `@This()`, so no container + // symbol to look the member up on. Resolving only through class-like + // owners declines here, and a decline is SILENT: with no USES edge the + // boundary probe measures a real zero and `impact` on `compare` goes back + // to `epistemic: "exact"`, which is the defect, not a conservative answer. + // 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'); + expect(valueRefTargetIds.filter((id) => id.includes('compare'))).toEqual([ + 'Function:src/webapi/dom_utils.zig:compare', + ]); + }); + + it('applies the callable gate to a MODULE owner too', () => { + // `dom_utils.DEFAULT_NS` is a module-scope constant. Widening the owner + // channel must not widen what counts as a registration, or every + // `bridge.accessor(mod.SOME_CONST, …)` in a binding table starts claiming + // a callable was registered. + expect(valueRefs).not.toContain('JsApi → DEFAULT_NS'); + expect(valueRefTargetIds.filter((id) => id.includes('DEFAULT_NS'))).toEqual([]); + }); + + it('declines a module-qualified reference whose handle is locally shadowed', () => { + // `shadowsTheModuleHandle(dom_utils: u8)` names a PARAMETER, not the + // file-level `@import`. Reading through the import here would attach the + // registration to a module this site never named — the same wrong-edge + // failure `isNamespaceNameShadowed` prevents on the member-call path, and + // the reason the module channel is guarded rather than merely added. + expect(valueRefs).not.toContain('shadowsTheModuleHandle → normalize'); + expect(valueRefTargetIds.filter((id) => id.includes('normalize'))).toEqual([]); + }); + it('does not mint a value reference for the CALLEE of an ordinary call', () => { // `register(onTick)` must produce ONE value reference (the argument), not // two: without binding the callee to the `function:` field the same rule diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index f6627e94a..71ef6b5f1 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -260,12 +260,24 @@ describe('PARSE_CACHE_VERSION', () => { // Moved 92 -> 93 for #3161 (Zig static gating): call captures inside a // comptime-false branch gain the `@reference.static-gated` marker, a // parse-time fact a warm cache from an earlier head would replay without. - it('pins SCHEMA_BUMP to 93 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(93); + // Moved 93 -> 98 for #3219 (Zig callable-value references): `ZIG_SCOPE_QUERY` + // gained three `@reference.value-ref` rules, so a `.zig` file now yields + // `value-ref` entries in `ParsedFile.referenceSites` where it yielded none. + // A warm v93 cache replays the old, empty site list for every unchanged file + // — `--force` included, since shards are content-addressed — so no USES edge + // is emitted, the boundary probe measures a real zero, and `impact` on a + // registered accessor goes back to `epistemic: "exact"`: the #3399 defect, + // silently un-fixed on exactly the incremental path most users are on. + // 94-97 are SKIPPED, not free: #3190 claims 94 and #3179 claims 94 through 97 + // in one PR. 98 is the next value above origin/main (93) and above every + // in-flight claim — re-checked at merge, which is the rule the paragraphs + // above were written by two PRs that each checked only once. + it('pins SCHEMA_BUMP to 98 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3219)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(98); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); for (const taken of [ 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, - 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, + 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index d2534c025..7d69f1ce1 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -64,6 +64,7 @@ export default defineConfig({ test: { name: 'lbug-db', include: [ + 'test/integration/impact-callable-value-references.test.ts', 'test/integration/impact-epistemic-lower-bound.test.ts', 'test/integration/impact-scope-omission-persistence.test.ts', 'test/integration/lbug-core-adapter.test.ts', @@ -143,6 +144,7 @@ export default defineConfig({ sequence: { groupOrder: 3 }, include: ['test/**/*.test.ts'], exclude: [ + 'test/integration/impact-callable-value-references.test.ts', 'test/integration/impact-epistemic-lower-bound.test.ts', 'test/integration/impact-scope-omission-persistence.test.ts', 'test/integration/lbug-core-adapter.test.ts',