GitNexus/gitnexus/test/unit/registry-primary-flag.test.ts
Gergő Magyar ab077b4c29
feat(ingestion): TypeScript registry-primary scope resolution (Ring 3) (#1050)
* feat(ingestion): TypeScript registry-primary scope resolution (Ring 3)

- Add TypeScript ScopeResolver stack (query/captures/interpret, import decomposition, hooks, arity, merge, receiver binding) and register in SCOPE_RESOLVERS.

- Harden shared compound receiver and receiver-bound CALLS pass for map for-of tuple bindings, dotted typeRef shapes, and callable-alias fallbacks.

- Flip TypeScript into MIGRATED_LANGUAGES; refresh AGENTS.md and type-resolution-system.md.

- Shared finalize-algorithm updates for cross-file scope parity.

- Tests: TS scope-resolution unit suite; legacy call-processor suite forces REGISTRY_PRIMARY_TYPESCRIPT=0; registry-primary flag test opts out TS in override scenario.

Made-with: Cursor

* fix(ingestion): SCC-ordered cross-file return-type propagation + multi-hop re-export resolution

Fix CI failures on PR #1050 (TypeScript registry-primary migration) by
making `propagateImportedReturnTypes` deterministic via reverse-
topological SCC ordering and updating the multi-hop re-export contract
to match `followReexportChain` behavior.

Why: the legacy pass mirrored an intermediate ref instead of the
terminal type when an importer was processed before its source module
had its own typeBindings chain-followed (4-file alias chain regression
in `ts-simple` fixture: `models.User -> service.user -> app.user`
collapsed to `getUser` instead of `User`). Reverse-topological walk of
`indexes.sccs` (leaves first) lets every importer see the source's
already-followed terminal type in a single pass.

Changes:
- `imported-return-types.ts`: rewrite to walk SCCs leaves-first, chain-
  follow the source module's typeBindings BEFORE mirroring, and chain-
  follow the importer's typeBindings AFTER mirroring. Cyclic SCCs
  reach a partial fixpoint (no convergence guarantee, ts-circular only
  asserts no-throw).
- `finalize-algorithm.ts`: docstring update on `FinalizeFile.localDefs`
  to reflect that `followReexportChain` resolves multi-hop re-exports
  through barrels even when intermediates do not surface the name -
  surfacing is now a static optimization, not a correctness requirement.
- `contract/scope-resolver.ts` Invariant I3: explicitly document the
  SCC ordering requirement.
- `pipeline/run.ts`: split PROF timer into `finalize` and `propagate`
  so the pass's cost is observable independently.
- `ARCHITECTURE.md` Performance notes: describe SCC-ordered propagation.
- `imported-return-types.ts`: expand chain-depth comment (2x effective
  depth from pre/post follow), add multi-ref break rationale, add
  `ts-simple` motivating-fixture pointer.

Tests:
- `finalize-algorithm.test.ts`: add 4 cases (3-hop chain, cyclic
  re-export visited-set guard, wildcard re-export fall-through,
  multi-source first-match-wins); fix misleading shared nodeId in the
  thick variant; rename and update the multi-hop test for the new
  contract (transitiveVia assertion on the thin variant).
- `imported-return-types.test.ts` (NEW): unit tests for the SCC pass
  pinning topological collapse, local-annotation guard, missing-source
  skip, and cyclic-SCC no-throw.
- `cross-file-binding.test.ts` + `ts-deep-alias-chain` fixture (NEW):
  5-file integration regression guard for SCC-ordered propagation
  through 4 module boundaries.

Validation: 865 scope-resolution + cross-file tests pass on Windows;
typecheck clean across both packages; only pre-existing Swift overload
failures remain (verified on PR base commit, environmental).

Made-with: Cursor

* fix(ingestion): address PR #1050 review findings — side-effect imports, resolve-cache perf, adapter signature

Three independent fixes surfaced by the production-readiness review of
the TypeScript registry-primary scope-resolution migration (RFC #909
Ring 3). All three pass under both REGISTRY_PRIMARY_TYPESCRIPT=0 and =1.

1. Side-effect imports were silently dropped (correctness regression).
   The legacy DAG emitted IMPORTS edges for `import './polyfill'` because
   its tree-sitter query matches `(import_statement source: (string))`
   regardless of clause. The new registry-primary path returned `[]`
   from `splitImportStatement()` for clause-less imports, so no
   ParsedImport / ImportEdge was ever produced — silent file-level edge
   loss. Add a generic 'side-effect' variant to `ParsedImport` and
   `ImportEdge['kind']` in `gitnexus-shared`; finalize resolves the
   target file and pre-finalizes the edge (no `targetDefId`, no
   `BindingRef`) so the SCC fixpoint loop skips it. The TypeScript
   provider now emits + interprets the new kind end-to-end. The
   variant is intentionally generic so other languages (Rust
   `use foo as _`, Python module-init) can adopt it.

2. Per-import re-derivation in `resolveImportTarget` (perf regression).
   The TS adapter built `new Set(allFilePaths)` on every call and let
   `resolveTsImportTarget` re-derive `allFileList` /
   `normalizedFileList` and discard the `resolveCache`. For a workspace
   with N files and M imports that's O(N × M) work per pass. Wrap the
   adapter in a closure that memoizes all five derived values keyed on
   the orchestrator's `ReadonlySet` identity; reset only when the set
   reference changes (start of new pass). New cost: O(N + M).

3. Misleading fake `ParsedImport` in the adapter (architecture).
   The adapter constructed `{ kind: 'named', localName: '_',
   importedName: '_', targetRaw }` to call `resolveTsImportTarget`,
   even though only `targetRaw` and the structural-typed context are
   read. Extract `resolveTsTarget(targetRaw, ctx)` so the adapter has
   an honest signature; `resolveTsImportTarget` still works for other
   callers. Also extract `narrowTsContext` for the type narrowing.

Tests: - New 4-file fixture `typescript-side-effect-imports` with two
    side-effect imports + one named import.
  - New "TypeScript side-effect imports" describe in
    `test/integration/resolvers/typescript.test.ts` (parity-gated by
    `ci-scope-parity.yml` — runs under both flag states).
  - Updated 2 unit tests to expect 1 side-effect ParsedImport and 4
    `@import.statement` matches (was 0 / 3).
  - 785 / 785 TS scope-resolution tests pass under both
    REGISTRY_PRIMARY_TYPESCRIPT=0 and =1.
Made-with: Cursor

* fix(scope): address Codex adversarial review findings on PR #1050

Four findings from the Codex adversarial review broke registry-primary
TypeScript resolution for common patterns. All four now have unit and
integration regression coverage that pass under both
`REGISTRY_PRIMARY_TYPESCRIPT=0` (legacy DAG) and the default
registry-primary path.

[high] tsconfig path aliases dropped:
Threaded `tsconfigPaths` through ScopeResolver via a new opaque
`resolutionConfig` parameter and a `loadResolutionConfig(repoPath)`
hook. The orchestrator (`scopeResolutionPhase` + `runScopeResolution`)
loads it once per workspace pass and forwards into every
`resolveImportTarget` call. TypeScript resolver now resolves
`@/services/user` style imports through the standard resolver's alias
branch.

[high] TSX parsed with the wrong grammar:
`emitTsScopeCaptures` now picks the parser/query by `filePath`
(`.tsx` -> TSX grammar) and validates cached trees against the
expected grammar via the new exported `tsCachedTreeMatchesGrammar`
helper. Stale TS-grammar trees for `.tsx` files no longer leak through
the scope query.

[medium] Literal dynamic imports never linked:
Added `kind: 'dynamic-resolved'` to `ParsedImport` and `ImportEdge`.
The decomposer emits a synthetic `@import.literal` capture for
string-literal dynamic imports; the interpreter maps that to
`dynamic-resolved`; finalize pre-finalizes it as a file-level terminal
(same shape as `side-effect`). `import('./feature')` now produces a
real IMPORTS edge under the registry-primary path. Legacy DAG keeps
its existing behavior — the new integration assertion is gated behind
the flag.

[medium] Namespace re-exports invisible from barrels:
The decomposer now emits TWO captures for `export * as ns from './m'`
— the existing `reexport-namespace` import draft AND a synthetic
`@declaration.namespace` capture (via `buildNamespaceDeclarationMatch`).
The latter creates a Namespace `SymbolDefinition` in the barrel's
`localDefs`, so downstream `import { ns } from './barrel'` resolves
through `findExportByName`.

Regression fixtures under `gitnexus/test/fixtures/lang-resolution/`:
- typescript-tsconfig-aliases (`@/` alias)
- typescript-tsx-jsx (Button.tsx + App.tsx with JSX)
- typescript-dynamic-import (`await import('./feature')`)
- typescript-reexport-namespace (`export * as Models from './base'`)

Validation:
- gitnexus-shared builds clean
- gitnexus typecheck clean
- 385/385 TS scope-resolution tests pass under both
  `REGISTRY_PRIMARY_TYPESCRIPT=0` and default

Made-with: Cursor

* perf(scope): O(1) defById lookup + bounded re-export depth (PR #1050 round 3)

Addresses the round-3 PR #1050 reviews (Claude adversarial + xkonjin):
both flagged the existing O(N²) `findDefById` linear scan in
`materializeBindings` and the unbounded recursion in
`followReexportChain` as production-readiness blockers for TypeScript
monorepos. Both fixes land alongside their regression tests under
both `REGISTRY_PRIMARY_TYPESCRIPT=0` and the default registry-primary
path.

[high] materializeBindings O(N_files × N_defs × N_edges) → O(N_defs + N_edges):
Build a `nodeId → SymbolDefinition` index map once at the top of
`materializeBindings` (one O(N_defs) pass), then replace the per-edge
`findDefById(files, edge.targetDefId)` linear scan with an O(1)
`defById.get(edge.targetDefId)` lookup. Also drop the now-unused
`findDefById` helper. At realistic TypeScript monorepo scale (~5k
files × ~50 defs/file × ~100k linked import edges) this is the
difference between ~25 s and a few ms inside finalize. Regression
test in `finalize-algorithm.test.ts` builds 200 leaf files +
1 consumer importing one symbol from each, asserts every binding
materializes correctly.

[medium] followReexportChain unbounded recursion:
The existing `visited` set caps depth at `O(N_files)` but allows
recursion proportional to barrel-chain depth, mismatching the
explicit "Iterative DFS to avoid stack overflow" policy in
`tarjanSccs`. Added a `MAX_REEXPORT_DEPTH = 100` constant and a
`depth` parameter to `followReexportChain` (defaults to 0); each
recursive call passes `depth + 1` and the function returns `null`
when the cap is exceeded. 100 is comfortably above any realistic
hand-authored barrel chain (typical depth 1-5; auto-generated
barrels rarely exceed 20) while staying well below JS engine call
stack limits. Regression test wires a 200-link reexport chain and
verifies the crawl terminates cleanly with `linkStatus: 'unresolved'`
(no terminal def reachable within the budget).

[low] synthesizeInstanceofNarrowings bare-identifier-only limitation:
xkonjin's review #4 noted that the LHS narrowing only handles bare
identifiers (`if (x instanceof Foo)`), not member expressions
(`if (user.address instanceof Address)`). Added a JSDoc note
explaining the constraint and pointing readers at field-type
resolution as the workaround for member-chain receivers.

Validation:
- gitnexus-shared builds clean
- gitnexus typecheck clean
- 413/413 tests pass under both flag states for finalize-algorithm +
  TS unit + TS integration suites
- 972/972 tests pass across full scope-resolution + Python +
  C# integration smoke (no cross-language regression)

Made-with: Cursor

* refactor(finalize): replace recursive followReexportChain with SCC-condensed iterative closure

The legacy `followReexportChain` walked re-export drafts via mutual
recursion guarded by a per-call visited set + a `MAX_REEXPORT_DEPTH`
ceiling. Recursion is fragile (call-stack ceiling, no bound on depth
that's actually meaningful), so this replaces it with a structurally
better algorithm: a precomputed per-file re-export closure built by
running Tarjan SCC over the re-export sub-graph and propagating names
in reverse-topological order with a bounded intra-SCC fixpoint.

Algorithm (`buildReexportClosures` in finalize-algorithm.ts):

  1. Sub-graph: build the directed graph of `reexport` + `wildcard`
     drafts only (regular/namespace/dynamic imports do not contribute).
  2. SCC condensation: run the same iterative `tarjanSccs` already
     used for the file-level import graph; output is in reverse-topo
     order so out-of-SCC neighbors are always already-finalized.
  3. Per-SCC propagation:
       - Acyclic singleton: one pass populates from neighbors' closures.
       - Cyclic SCC: bounded fixpoint capped at |SCC|+1 iterations.
         With first-wins precedence the closure map is monotone, so
         each name needs at most |SCC| hops to traverse the cycle.

Precedence (preserved from the recursive crawl):
  - Named re-exports take precedence over wildcards.
  - Within each kind, declaration order wins.

Lookup at finalize time becomes O(1) (`lookupReexportedName`), down
from O(chain_depth × drafts) per consult and recursive at that.

Properties vs the legacy implementation:
  - Stack-safe by construction; no `MAX_REEXPORT_DEPTH` guard needed.
  - 1000-hop barrel chains now resolve in full (legacy capped at 100
    and surfaced anything deeper as `unresolved`).
  - Cycles handled structurally via SCC, not via per-call visited set.
  - Same observable semantics: every existing test passes unchanged.

Tests:
  - Replace the obsolete `MAX_REEXPORT_DEPTH (200-hop chain stops
    cleanly without stack overflow)` test (which asserted the OLD
    bug — that deep chains failed to resolve) with a positive
    1000-hop test that asserts full resolution + accurate
    `transitiveVia`. Proves both the recursion is gone AND the
    closure correctly inherits the leaf def across all hops.
  - Update commentary on adjacent re-export tests to reference the
    closure mechanism.
  - Update `FinalizeFile.localDefs` JSDoc + import-decomposer.ts
    inline doc to point at `buildReexportClosures` instead of the
    removed function name.

Validation: - gitnexus-shared builds cleanly.
  - gitnexus typechecks cleanly.
  - 28/28 finalize-algorithm.test.ts tests pass (incl. new 1000-hop).
  - 801/801 TypeScript scope-resolution tests pass under default
    (registry-primary) AND `REGISTRY_PRIMARY_TYPESCRIPT=0` (legacy DAG).
  - 404/404 Python + C# integration tests pass — no regression in
    cross-language consumers of the shared `finalize`.
Made-with: Cursor

* fix(scope): remove non-null assertions from scope resolution

Made-with: Cursor

* fix(scope): address TypeScript review follow-ups

Made-with: Cursor

* fix(scope): address TypeScript import review follow-ups

Add regression coverage for non-binding import edges and circular TypeScript bindings so PR #1050 review concerns stay visible without changing runtime semantics.

Made-with: Cursor
2026-04-26 08:23:08 +01:00

171 lines
7.7 KiB
TypeScript

/**
* Unit tests for `registry-primary-flag` (RFC #909 Ring 2 PKG #924).
*
* Flag is `REGISTRY_PRIMARY_<UPPER(lang)>`. Each test manipulates
* `process.env` directly and restores it in `afterEach` — there is no
* per-process cache to invalidate, so isolation is lexical.
*/
import { describe, it, expect, afterEach, beforeEach } from 'vitest';
import { SupportedLanguages } from 'gitnexus-shared';
import {
envVarNameFor,
isRegistryPrimary,
primaryLanguages,
MIGRATED_LANGUAGES,
} from '../../src/core/ingestion/registry-primary-flag.js';
// ─── Test isolation ─────────────────────────────────────────────────────────
//
// Scrub every `REGISTRY_PRIMARY_*` env var before + after each test so
// parallel vitest runs on the same process don't bleed state.
function clearAllRegistryPrimaryVars(): void {
for (const key of Object.keys(process.env)) {
if (key.startsWith('REGISTRY_PRIMARY_')) delete process.env[key];
}
}
beforeEach(clearAllRegistryPrimaryVars);
afterEach(clearAllRegistryPrimaryVars);
// ─── envVarNameFor ─────────────────────────────────────────────────────────
describe('envVarNameFor', () => {
it('produces upper-cased env-var names from the enum value', () => {
expect(envVarNameFor(SupportedLanguages.Python)).toBe('REGISTRY_PRIMARY_PYTHON');
expect(envVarNameFor(SupportedLanguages.TypeScript)).toBe('REGISTRY_PRIMARY_TYPESCRIPT');
expect(envVarNameFor(SupportedLanguages.JavaScript)).toBe('REGISTRY_PRIMARY_JAVASCRIPT');
});
it('uses the enum VALUE, not the key, for languages whose key differs from the value', () => {
// Key 'CPlusPlus' → value 'cpp' → env var 'REGISTRY_PRIMARY_CPP'.
// Users see the language by its canonical name, not its TS symbol.
expect(envVarNameFor(SupportedLanguages.CPlusPlus)).toBe('REGISTRY_PRIMARY_CPP');
expect(envVarNameFor(SupportedLanguages.CSharp)).toBe('REGISTRY_PRIMARY_CSHARP');
});
it('covers every member of SupportedLanguages', () => {
// Build env-var names for every language and assert no duplicates —
// catches a future enum-value collision or accidental renaming.
const names = new Set<string>();
for (const lang of Object.values(SupportedLanguages)) {
names.add(envVarNameFor(lang));
}
expect(names.size).toBe(Object.values(SupportedLanguages).length);
});
});
// ─── isRegistryPrimary ─────────────────────────────────────────────────────
describe('isRegistryPrimary', () => {
it('returns MIGRATED_LANGUAGES membership by default (no env var set)', () => {
// Ring 3: languages in MIGRATED_LANGUAGES are registry-primary by
// default — operators don't need to set an env var for the rolled-out
// migration to take effect. Unmigrated languages default to false.
for (const lang of Object.values(SupportedLanguages)) {
expect(isRegistryPrimary(lang)).toBe(MIGRATED_LANGUAGES.has(lang));
}
});
it("returns true when the env var is 'true' (lowercase)", () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = 'true';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
});
it("returns true when the env var is '1'", () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = '1';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
});
it("returns true when the env var is 'yes'", () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = 'yes';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
});
it('accepts mixed-case and whitespace-padded truthy values', () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = ' TRUE ';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
process.env['REGISTRY_PRIMARY_PYTHON'] = 'Yes';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
});
it("returns false for falsy-looking values ('false', '0', empty, 'off')", () => {
for (const value of ['false', '0', '', 'off', 'no', 'disabled']) {
process.env['REGISTRY_PRIMARY_PYTHON'] = value;
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(false);
}
});
it('returns false for unrecognized tokens (fail-safe on typos)', () => {
// User meant to type 'true' but fat-fingered — conservative: treat as off.
for (const value of ['ture', 'tru', 'yeah', 'enable', 'y']) {
process.env['REGISTRY_PRIMARY_PYTHON'] = value;
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(false);
}
});
it('isolates flags per-language (one on does not affect others)', () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = 'true';
expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true);
// Java and Go are not in MIGRATED_LANGUAGES — default false stays
// false regardless of Python's flag.
expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false);
expect(isRegistryPrimary(SupportedLanguages.Go)).toBe(false);
});
it('respects a mid-process env-var mutation (no stale cache)', () => {
// Use Java — not in MIGRATED_LANGUAGES — so the unset default is
// deterministically `false`, independent of which languages have
// been flipped to registry-primary.
expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false);
process.env['REGISTRY_PRIMARY_JAVA'] = 'true';
expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(true);
delete process.env['REGISTRY_PRIMARY_JAVA'];
expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false);
});
it('handles the CPlusPlus → REGISTRY_PRIMARY_CPP mapping correctly', () => {
process.env['REGISTRY_PRIMARY_CPP'] = 'true';
expect(isRegistryPrimary(SupportedLanguages.CPlusPlus)).toBe(true);
// Negative: the TS-key-style name is NOT read.
delete process.env['REGISTRY_PRIMARY_CPP'];
process.env['REGISTRY_PRIMARY_CPLUSPLUS'] = 'true';
expect(isRegistryPrimary(SupportedLanguages.CPlusPlus)).toBe(false);
});
});
// ─── primaryLanguages ──────────────────────────────────────────────────────
describe('primaryLanguages', () => {
it('returns MIGRATED_LANGUAGES when no flags are set', () => {
// Default-on for migrated languages (Ring 3); unmigrated stay off.
const enabled = primaryLanguages();
expect(enabled.size).toBe(MIGRATED_LANGUAGES.size);
for (const lang of MIGRATED_LANGUAGES) {
expect(enabled.has(lang)).toBe(true);
}
});
it('returns exactly the flipped languages (env opts in unmigrated, opts out migrated)', () => {
// Migrated languages are default-on; each must be opted out here when
// testing explicit env overrides. Go (unmigrated) opts in; Java stays off.
process.env['REGISTRY_PRIMARY_PYTHON'] = 'false';
process.env['REGISTRY_PRIMARY_CSHARP'] = 'false';
process.env['REGISTRY_PRIMARY_TYPESCRIPT'] = 'false';
process.env['REGISTRY_PRIMARY_GO'] = '1';
const enabled = primaryLanguages();
expect(enabled.has(SupportedLanguages.Python)).toBe(false);
expect(enabled.has(SupportedLanguages.CSharp)).toBe(false);
expect(enabled.has(SupportedLanguages.Go)).toBe(true);
expect(enabled.has(SupportedLanguages.Java)).toBe(false);
// Only Go is on: migrated defaults overridden off, Go explicitly on.
expect(enabled.size).toBe(1);
});
it('returns a plain Set (not a frozen proxy) — consistent shape', () => {
process.env['REGISTRY_PRIMARY_PYTHON'] = 'true';
const enabled = primaryLanguages();
expect(enabled).toBeInstanceOf(Set);
});
});