diff --git a/gitnexus/src/core/ingestion/languages/javascript/query.ts b/gitnexus/src/core/ingestion/languages/javascript/query.ts index c2db3a8c7..168f866d7 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/query.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/query.ts @@ -610,6 +610,17 @@ export const JAVASCRIPT_SCOPE_QUERY = ` (return_statement (identifier) @reference.name @reference.read.identifier) +;; \`const next = LIMIT\` and \`n > LIMIT\` — both plainly value reads, and both +;; named in review as gaps between what A2 claimed and what it matched. +(variable_declarator + value: (identifier) @reference.name @reference.read.identifier) + +(binary_expression + left: (identifier) @reference.name @reference.read.identifier) + +(binary_expression + right: (identifier) @reference.name @reference.read.identifier) + ;; Destructured PARAMETER keys (R2-1c). \`function exit({ exitMinAtrMult = 0 })\` ;; reads that property off whatever the caller passes, exactly as ;; \`cfg.exitMinAtrMult\` would — the field just never appears in a diff --git a/gitnexus/src/core/ingestion/languages/typescript/query.ts b/gitnexus/src/core/ingestion/languages/typescript/query.ts index b2c0181f9..8a1521ca7 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/query.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/query.ts @@ -1219,6 +1219,34 @@ export const TYPESCRIPT_SCOPE_QUERY = ` (object (shorthand_property_identifier) @reference.name @reference.property-key @reference.value-ref) +;; Bare-identifier reads (A2), VALUE POSITIONS ONLY — a blanket \`(identifier)\` +;; rule would mint a site for every token in the file. +;; +;; These existed only in the JavaScript query, so A2 did not work for +;; TypeScript AT ALL: a \`.ts\` module reading its own \`const\` by bare name +;; produced no reference site, and "who uses this constant?" answered a +;; confident zero for an entire language. Found by writing the namespace +;; fixture below and watching it fail for the wrong reason. +(arguments + (identifier) @reference.name @reference.read.identifier) + +(assignment_pattern + right: (identifier) @reference.name @reference.read.identifier) + +(return_statement + (identifier) @reference.name @reference.read.identifier) + +;; \`const next = LIMIT\` and \`n > LIMIT\` — both plainly value reads, and both +;; named in review as gaps between what A2 claimed and what it matched. +(variable_declarator + value: (identifier) @reference.name @reference.read.identifier) + +(binary_expression + left: (identifier) @reference.name @reference.read.identifier) + +(binary_expression + right: (identifier) @reference.name @reference.read.identifier) + ;; References — TYPE POSITION (R2-2). An annotation naming a declared type is ;; the only thing that makes that type's declaration reachable from the code ;; that depends on it, and TypeScript captured none: only cpp and csharp emitted diff --git a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/references-to-edges.ts b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/references-to-edges.ts index 29ffe84b5..c7ccf2a5b 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/references-to-edges.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/references-to-edges.ts @@ -25,6 +25,7 @@ import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js' import { mapReferenceKindToEdgeType } from '../graph-bridge/edges.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import type { CalleeIdSink } from '../graph-bridge/callee-id-sink.js'; +import { isValueDefinitionLabel } from '../../utils/ast-helpers.js'; /** * Optional opaque skip key — providers may pre-emit edges (e.g. via @@ -40,7 +41,6 @@ type ReferenceSiteSkipSet = ReadonlySet; * only worth an edge when the def lives at module scope — see * `moduleScopeValueDefIds`. */ -const LOCALIZABLE_VALUE_LABELS: ReadonlySet = new Set(['Const', 'Variable', 'Static']); export function emitReferencesViaLookup( graph: KnowledgeGraph, @@ -112,7 +112,7 @@ export function emitReferencesViaLookup( if ( moduleScopeValueDefIds !== undefined && edgeType === 'ACCESSES' && - LOCALIZABLE_VALUE_LABELS.has(targetDef.type) && + isValueDefinitionLabel(targetDef.type) && !moduleScopeValueDefIds.has(targetDef.nodeId) ) { skipped++; diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index 5ee383081..6ff612b8d 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -94,6 +94,7 @@ import { buildWorkspaceResolutionIndex } from '../workspace-index.js'; import type { ResolutionOutcome, ResolutionOutcomeRecorder } from '../resolution-outcome.js'; import { logHeapProbe } from '../../utils/heap-probe.js'; import { parseTruthyEnv } from '../../utils/env.js'; +import { isValueDefinitionLabel } from '../../utils/ast-helpers.js'; import { TransitionalScopeTree } from '../../../../storage/scope-index-store.js'; import { forceGc } from '../../../../storage/parsedfile-store.js'; @@ -794,6 +795,51 @@ export function runScopeResolution( const tResolve = PROF ? process.hrtime.bigint() : 0n; logHeapProbe('sr-post-resolve', `lang=${provider.language}`); + // Value defs bound at MODULE LEVEL. A read of a block-local `const` must not + // mint an edge — that would retain the inert locals `pruneLocalSymbols` drops. + // + // Built HERE, above the out-of-core seal, and deliberately from `parsedFiles` + // rather than `emitParsedFiles`. The seal below replaces the latter with a + // scope-STRIPPED copy, so building this after it walked `scopes: []` for every + // file and produced an empty set — which the filter then reads as "no def is + // module-level" and drops EVERY `Const`/`Variable`/`Static` ACCESSES edge in + // the repo, in all languages, on the one path (`GITNEXUS_DISK_SCOPE_INDEX=1`) + // taken by the largest repos. Nothing failed and nothing logged; the edges + // were simply absent, which is the confident-empty answer this PR exists to + // remove. + // + // `Module` is also not the only module level. A `Namespace` scope (TS + // `namespace`, Rust `mod`, C++/C# `namespace`) holds importable values too, + // and treating its consts as function-locals dropped their edges as well. + // Included when the whole chain to the root is Module/Namespace — a namespace + // declared inside a function body is a local like anything else there. + const moduleScopeValueDefIds = new Set(); + let moduleScopesInspected = false; + for (const parsed of parsedFiles) { + const scopeById = new Map(parsed.scopes.map((sc) => [sc.id, sc])); + for (const scope of parsed.scopes) { + if (scope.kind !== 'Module' && scope.kind !== 'Namespace') continue; + let ancestor = scope.parent === null ? undefined : scopeById.get(scope.parent); + let atModuleLevel = true; + while (ancestor !== undefined) { + if (ancestor.kind !== 'Module' && ancestor.kind !== 'Namespace') { + atModuleLevel = false; + break; + } + ancestor = ancestor.parent === null ? undefined : scopeById.get(ancestor.parent); + } + if (!atModuleLevel) continue; + moduleScopesInspected = true; + for (const [, refs] of scope.bindings) { + for (const ref of refs) { + if (isValueDefinitionLabel(ref.def.type)) { + moduleScopeValueDefIds.add(ref.def.nodeId); + } + } + } + } + } + // ── Out-of-core scope seal boundary ───────────────────────────────────── // Pass-A (finalize + propagate + resolve) is done; all whole-language reads // of `Scope.bindings` are behind us. Emit reaches scopes ONLY via @@ -927,22 +973,6 @@ export function runScopeResolution( ); const referenceSkipSites = new Set(handledSites); for (const key of deferredIndirectSites) referenceSkipSites.add(key); - // Value defs bound at MODULE scope. A read of a block-local `const` must not - // mint an edge — that would retain the inert locals `pruneLocalSymbols` - // drops. Built once here, where the parsed scopes are already in hand. - const moduleScopeValueDefIds = new Set(); - for (const parsed of emitParsedFiles) { - const moduleScope = parsed.scopes.find((sc) => sc.kind === 'Module'); - if (moduleScope === undefined) continue; - for (const [, refs] of moduleScope.bindings) { - for (const ref of refs) { - const t = ref.def.type; - if (t === 'Const' || t === 'Variable' || t === 'Static') { - moduleScopeValueDefIds.add(ref.def.nodeId); - } - } - } - } const { emitted, skipped } = callableFlowOnly ? { emitted: 0, skipped: 0 } : emitReferencesViaLookup( @@ -952,7 +982,12 @@ export function runScopeResolution( postHeritageNodeLookup, referenceSkipSites, calleeIdAccumulator, - moduleScopeValueDefIds, + // FAIL OPEN, not closed. An empty set is a legitimate answer ("this repo + // has no module-level value defs, so every such target is a local"), but + // it is indistinguishable from "the scopes could not be inspected" — and + // in the second case arming the filter deletes a whole edge class. Only + // pass the set when scopes were actually walked. + moduleScopesInspected ? moduleScopeValueDefIds : undefined, ); // Last-resort property resolution by workspace-unique name (A1/A5). Runs // after every precise pass and only sees what they left behind, so a diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/config.ts b/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/config.ts new file mode 100644 index 000000000..ab8a87cc5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/config.ts @@ -0,0 +1,23 @@ +// RV-9: a const declared inside a TS `namespace`. Its binding scope is +// `Namespace`, not `Module`, so the module-level set built for the block-local +// filter did not contain it and its reads were dropped as if it were a local. +// +// The same shape exists in Rust (`mod`), C++ and C# — anywhere a language nests +// an importable value one level below the file root. +export namespace Limits { + export const NAMESPACED_MAX = 42; + + export function withinNamespace(): number { + return NAMESPACED_MAX; + } +} + +// CONTROL: a namespace declared INSIDE a function body is a local like anything +// else there, so its const must stay excluded. +export function makeLocalNamespace(): number { + // eslint-disable-next-line @typescript-eslint/no-namespace + namespace Inner { + export const innerLocalValue = 7; + } + return Inner.innerLocalValue; +} diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/consumer.ts b/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/consumer.ts new file mode 100644 index 000000000..0a3047998 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-namespace-const/consumer.ts @@ -0,0 +1,5 @@ +import { Limits } from './config.js'; + +export function readsNamespacedConst(): number { + return Limits.NAMESPACED_MAX; +} diff --git a/gitnexus/test/integration/resolvers/javascript-const-references.test.ts b/gitnexus/test/integration/resolvers/javascript-const-references.test.ts index a94b4eb49..a2551dece 100644 --- a/gitnexus/test/integration/resolvers/javascript-const-references.test.ts +++ b/gitnexus/test/integration/resolvers/javascript-const-references.test.ts @@ -53,6 +53,54 @@ describe('JavaScript module-scope const references (A2)', () => { expect(toLocal).toEqual([]); }); + // The out-of-core path (#RV-2). Nothing in the suite exercised + // `GITNEXUS_DISK_SCOPE_INDEX`, and that is where the module-level set was + // being built from scope-STRIPPED files: it came out empty, which the filter + // read as "no def is module-level" and used to drop every + // `Const`/`Variable`/`Static` ACCESSES edge in the repo — including the ones + // this suite exists to prove exist. It failed silently, on the path large + // repos take, and no test could see it. + // + // Parity is the assertion: the seal is a memory optimization and must not + // change a single edge. + describe('under the out-of-core scope seal', () => { + let sealed: PipelineResult; + + beforeAll(async () => { + const prev = process.env.GITNEXUS_DISK_SCOPE_INDEX; + process.env.GITNEXUS_DISK_SCOPE_INDEX = '1'; + try { + sealed = await runPipelineFromRepo( + path.join(FIXTURES, 'javascript-const-references'), + () => {}, + ); + } finally { + if (prev === undefined) delete process.env.GITNEXUS_DISK_SCOPE_INDEX; + else process.env.GITNEXUS_DISK_SCOPE_INDEX = prev; + } + }, 60000); + + const sealedReaders = (): Set => + new Set( + getRelationships(sealed, 'ACCESSES') + .filter((e) => e.target === 'DEFAULT_FETCH_LIMIT') + .map((e) => e.source), + ); + + it('keeps the const edges the unsealed run produced', () => { + expect([...sealedReaders()].sort()).toEqual([...readersOfConst()].sort()); + }); + + it('still withholds the block-local edge', () => { + // The filter must fail OPEN when scopes are unavailable, not be disabled: + // the block-local exclusion is a correctness property, not an optimization. + const toLocal = getRelationships(sealed, 'ACCESSES').filter( + (e) => e.target === 'localScratchValue', + ); + expect(toLocal).toEqual([]); + }); + }); + it('targets the Const node itself, not a same-named local', () => { const toConst = getRelationships(result, 'ACCESSES').filter( (e) => e.target === 'DEFAULT_FETCH_LIMIT', diff --git a/gitnexus/test/integration/resolvers/typescript-namespace-const.test.ts b/gitnexus/test/integration/resolvers/typescript-namespace-const.test.ts new file mode 100644 index 000000000..394c59efb --- /dev/null +++ b/gitnexus/test/integration/resolvers/typescript-namespace-const.test.ts @@ -0,0 +1,41 @@ +/** + * RV-9 — a const bound in a `Namespace` scope is module-level, not a local. + * + * The block-local filter added for A2 keeps a read of a block-scoped `const` + * from minting an edge, because such an edge would retain exactly the inert + * locals `pruneLocalSymbols` exists to drop. It decided "is this module-level?" + * by asking `kind === 'Module'`, which is true of the file root and of nothing + * else — so a value declared in a TS `namespace` (or a Rust `mod`, or a C++ / + * C# namespace) was classified as a function-local and its reads were dropped. + * + * The feature simply did not work there. Reported as a gap rather than a + * regression: no pre-existing edge was deleted, because the other languages' + * read/write captures are member-shaped and target `Property`, not + * `Const`/`Variable`/`Static`. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'path'; +import { FIXTURES, getRelationships, runPipelineFromRepo, type PipelineResult } from './helpers.js'; + +describe('TypeScript namespace-scoped const references (RV-9)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'typescript-namespace-const'), () => {}); + }, 60000); + + const readersOf = (name: string): string[] => + getRelationships(result, 'ACCESSES') + .filter((e) => e.target === name) + .map((e) => e.source); + + it('emits an edge for a read of a namespace-scoped const', () => { + expect(readersOf('NAMESPACED_MAX')).toContain('withinNamespace'); + }); + + // The bound. A namespace nested in a function body is a local like anything + // else declared there, so widening "module level" must not reach into one. + it('still withholds an edge to a const in a function-local namespace', () => { + expect(readersOf('innerLocalValue')).toEqual([]); + }); +});