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

gitnexus-shared/src/scope-resolution/finalize-algorithm.ts

@@ -335,14 +335,11 @@ function makeEdgeDraft(
 
   // Edge is unresolvable at the file level — mark unresolved now.
   if (targetFile === null) {
-    const edgeKind = parsed.kind === 'wildcard' ? 'wildcard-expanded' : parsed.kind;
-    const localName = parsed.kind === 'wildcard' ? '' : parsed.localName;
-    const targetExportedName = extractExportedName(parsed);
     const base: ImportEdge = {
-      localName,
+      localName: extractLocalName(parsed),
       targetFile: null,
-      targetExportedName,
-      kind: edgeKind,
+      targetExportedName: extractExportedName(parsed),
+      kind: edgeKindFor(parsed),
       linkStatus: 'unresolved',
     };
     return {
@@ -356,15 +353,15 @@ function makeEdgeDraft(
   }
 
   // Resolvable at the file level; intra-SCC fixpoint may still fail to fill
-  // in `targetDefId` (e.g., symbol not exported from target).
-  const edgeKind = parsed.kind === 'wildcard' ? 'wildcard-expanded' : parsed.kind;
-  const localName = parsed.kind === 'wildcard' ? '' : parsed.localName;
-  const targetExportedName = extractExportedName(parsed);
+  // in `targetDefId` (e.g., symbol not exported from target). Side-effect
+  // imports are terminal at the file level — no `targetDefId` needed since
+  // they materialize no `BindingRef`. Pre-finalize them here so the
+  // fixpoint loop skips them entirely.
   const base: ImportEdge = {
-    localName,
+    localName: extractLocalName(parsed),
     targetFile,
-    targetExportedName,
-    kind: edgeKind,
+    targetExportedName: extractExportedName(parsed),
+    kind: edgeKindFor(parsed),
   };
   return {
     source: parsed,
@@ -372,10 +369,25 @@ function makeEdgeDraft(
     fromScope: file.moduleScope,
     targetFile,
     base,
-    finalized: null,
+    finalized: parsed.kind === 'side-effect' ? base : null,
   };
 }
 
+function edgeKindFor(parsed: ParsedImport): ImportEdge['kind'] {
+  if (parsed.kind === 'wildcard') return 'wildcard-expanded';
+  return parsed.kind;
+}
+
+function extractLocalName(parsed: ParsedImport): string {
+  switch (parsed.kind) {
+    case 'wildcard':
+    case 'side-effect':
+      return '';
+    default:
+      return parsed.localName;
+  }
+}
+
 function extractExportedName(parsed: ParsedImport): string {
   switch (parsed.kind) {
     case 'named':
@@ -385,6 +397,7 @@ function extractExportedName(parsed: ParsedImport): string {
       return parsed.importedName;
     case 'wildcard':
     case 'dynamic-unresolved':
+    case 'side-effect':
       return '';
   }
 }

gitnexus-shared/src/scope-resolution/types.ts

@@ -182,6 +182,21 @@ export type ParsedImport =
       readonly localName: string;
       /** Source text of the unresolved expression when available; `null` otherwise. */
       readonly targetRaw: string | null;
+    }
+  /**
+   * Bare-source / side-effect import that introduces no local name binding
+   * but still establishes a file-level dependency. Resolves to a concrete
+   * `targetFile` via `resolveImportTarget` and produces a file→file
+   * `ImportEdge` for module-reachability and impact analysis, with no
+   * `BindingRef` materialized.
+   *
+   * Examples:
+   *   - JS / TS `import './polyfill'`        → `{ kind: 'side-effect', targetRaw: './polyfill' }`
+   *   - Rust    `use foo::bar as _`          → side-effect (binding hidden under `_`)
+   */
+  | {
+      readonly kind: 'side-effect';
+      readonly targetRaw: string;
     };
 
 /**
@@ -253,7 +268,8 @@ export interface ImportEdge {
     | 'namespace'
     | 'wildcard-expanded'
     | 'reexport'
-    | 'dynamic-unresolved';
+    | 'dynamic-unresolved'
+    | 'side-effect';
   /** Re-export chain, for provenance (e.g., `['./y']` when re-exported via `./y`). */
   readonly transitiveVia?: readonly string[];
   /** Set to `'unresolved'` when the SCC fixpoint could not link this edge. */

gitnexus/src/core/ingestion/languages/typescript/import-decomposer.ts

@@ -31,10 +31,11 @@
  * TypeScript scope-resolution layer, types and values share the same
  * lookup; runtime-emission is a downstream concern.
  *
- * Side-effect imports (`import './polyfill'`) produce NO decomposed
- * match — there is no local binding to resolve and the finalize
- * algorithm has no ParsedImport variant for bare-source edges. The
- * caller drops the raw anchor.
+ * Side-effect imports (`import './polyfill'`) produce a single match
+ * with `kind: 'side-effect'`. The shared finalize algorithm resolves
+ * the target file and emits a file-level IMPORTS edge, but
+ * materializes no `BindingRef` (matching the legacy DAG, which counts
+ * `import './polyfill'` as a module-reachability dependency only).
  */
 
 import type { Capture, CaptureMatch } from 'gitnexus-shared';
@@ -54,7 +55,8 @@ type ImportKind =
   | 'reexport-alias'
   | 'reexport-wildcard'
   | 'reexport-namespace'
-  | 'dynamic';
+  | 'dynamic'
+  | 'side-effect';
 
 interface ImportSpec {
   readonly kind: ImportKind;
@@ -73,11 +75,9 @@ interface ImportSpec {
 /**
  * Decompose an import anchor. Handles three node types:
  *
- *   - `import_statement`             : all static import forms
+ *   - `import_statement`             : all static import forms (incl. side-effect)
  *   - `export_statement` (w/ source) : re-exports
  *   - `call_expression` (import fn)  : dynamic `import()`
- *
- * Returns `[]` for side-effect imports (no local binding).
  */
 export function splitImportStatement(stmtNode: SyntaxNode): CaptureMatch[] {
   if (stmtNode.type === 'import_statement') return splitImport(stmtNode);
@@ -101,8 +101,17 @@ function splitImport(stmtNode: SyntaxNode): CaptureMatch[] {
 
   const importClause = findChild(stmtNode, 'import_clause');
   if (importClause === null) {
-    // `import './polyfill'` — no clause, no local binding. Drop it.
-    return [];
+    // `import './polyfill'` — no clause, no local binding. Emit a
+    // side-effect match so the finalize layer still produces a
+    // file-level IMPORTS edge (parity with the legacy DAG).
+    return [
+      buildImportMatch(stmtNode, {
+        kind: 'side-effect',
+        source,
+        name: '',
+        atNode: stmtNode,
+      }),
+    ];
   }
 
   const out: CaptureMatch[] = [];

gitnexus/src/core/ingestion/languages/typescript/import-target.ts

@@ -42,23 +42,34 @@ export function resolveTsImportTarget(
   parsedImport: ParsedImport,
   workspaceIndex: WorkspaceIndex,
 ): string | null {
-  const ctx = workspaceIndex as TsResolveContext | undefined;
-  if (
-    ctx === undefined ||
-    typeof (ctx as { fromFile?: unknown }).fromFile !== 'string' ||
-    !((ctx as { allFilePaths?: unknown }).allFilePaths instanceof Set)
-  ) {
-    return null;
-  }
-  if (parsedImport.kind === 'dynamic-unresolved') {
-    // Dynamic imports carry `targetRaw` only for diagnostics; when
-    // the expression isn't a string literal we can't resolve a file.
-    if (parsedImport.targetRaw === null) return null;
-    // A string-literal dynamic import (`import('./m')`) resolves the
-    // same way as a static import, so we fall through.
-  }
+  const ctx = narrowTsContext(workspaceIndex);
+  if (ctx === null) return null;
+
+  // Dynamic imports carry `targetRaw` only for diagnostics; when the
+  // expression isn't a string literal we can't resolve a file.
+  // A string-literal dynamic import (`import('./m')`) resolves like a
+  // static import — fall through to the shared path resolver.
+  if (parsedImport.kind === 'dynamic-unresolved' && parsedImport.targetRaw === null) return null;
   if (parsedImport.targetRaw === null || parsedImport.targetRaw === '') return null;
 
+  return resolveTsTarget(parsedImport.targetRaw, ctx);
+}
+
+/**
+ * Resolve a raw module-path string to a workspace file path using the
+ * same standard-strategy resolver as the legacy DAG. Operates directly on
+ * the source string without requiring a `ParsedImport`, so the
+ * `ScopeResolver.resolveImportTarget` adapter doesn't need to construct
+ * a fake `ParsedImport` to reach the resolver.
+ *
+ * Returns `null` when:
+ *   - the context is malformed (missing `fromFile` / `allFilePaths`)
+ *   - `targetRaw` is empty
+ *   - the resolver finds no matching file
+ */
+export function resolveTsTarget(targetRaw: string, ctx: TsResolveContext): string | null {
+  if (targetRaw === '') return null;
+
   const language = ctx.language ?? SupportedLanguages.TypeScript;
   const allFileList = ctx.allFileList ?? Array.from(ctx.allFilePaths);
   const normalizedFileList = ctx.normalizedFileList ?? allFileList.map((f) => f.toLowerCase());
@@ -66,7 +77,7 @@ export function resolveTsImportTarget(
 
   return resolveImportPath(
     ctx.fromFile,
-    parsedImport.targetRaw,
+    targetRaw,
     ctx.allFilePaths,
     allFileList as string[],
     normalizedFileList as string[],
@@ -75,3 +86,15 @@ export function resolveTsImportTarget(
     ctx.tsconfigPaths ?? null,
   );
 }
+
+function narrowTsContext(workspaceIndex: WorkspaceIndex): TsResolveContext | null {
+  const ctx = workspaceIndex as TsResolveContext | undefined;
+  if (
+    ctx === undefined ||
+    typeof (ctx as { fromFile?: unknown }).fromFile !== 'string' ||
+    !((ctx as { allFilePaths?: unknown }).allFilePaths instanceof Set)
+  ) {
+    return null;
+  }
+  return ctx;
+}

gitnexus/src/core/ingestion/languages/typescript/index.ts

@@ -88,5 +88,5 @@ export { getTypescriptCaptureCacheStats, resetTypescriptCaptureCacheStats } from
 export { interpretTsImport, interpretTsTypeBinding } from './interpret.js';
 export { typescriptMergeBindings } from './merge-bindings.js';
 export { typescriptArityCompatibility } from './arity.js';
-export { resolveTsImportTarget, type TsResolveContext } from './import-target.js';
+export { resolveTsImportTarget, resolveTsTarget, type TsResolveContext } from './import-target.js';
 export { tsBindingScopeFor, tsImportOwningScope, tsReceiverBinding } from './simple-hooks.js';

gitnexus/src/core/ingestion/languages/typescript/interpret.ts

@@ -132,6 +132,13 @@ export function interpretTsImport(captures: CaptureMatch): ParsedImport | null {
         targetRaw: sourceCap?.text ?? null,
       };
     }
+    case 'side-effect': {
+      // `import './polyfill'` — bare-source, no local binding. The
+      // finalize layer resolves to a target file and emits a
+      // file-level IMPORTS edge; no `BindingRef` is materialized.
+      if (sourceCap === undefined) return null;
+      return { kind: 'side-effect', targetRaw: sourceCap.text };
+    }
     default:
       return null;
   }

gitnexus/src/core/ingestion/languages/typescript/scope-resolver.ts

@@ -21,32 +21,56 @@ import { typescriptProvider } from '../typescript.js';
 import {
   typescriptArityCompatibility,
   typescriptMergeBindings,
-  resolveTsImportTarget,
+  resolveTsTarget,
   type TsResolveContext,
 } from './index.js';
 
+/**
+ * Build a `resolveImportTarget` adapter that memoizes the workspace
+ * file list, the lower-cased file list, and the per-pass `resolveCache`
+ * across every import lookup in a single workspace pass. The
+ * orchestrator passes the same `ReadonlySet` reference for every call
+ * within a pass — we use that identity to detect when the workspace
+ * changes and recompute the derived state lazily.
+ *
+ * Without this memoization, `resolveTsTarget` re-derived
+ * `allFileList` and `normalizedFileList` (both O(N_files)) and threw
+ * away the `resolveCache` on every import — O(N_files × N_imports)
+ * total work for what should be O(N_files + N_imports).
+ */
+function makeTsResolveImportTarget(): ScopeResolver['resolveImportTarget'] {
+  let cachedAllFilePaths: ReadonlySet<string> | null = null;
+  let cachedSet: Set<string> | null = null;
+  let cachedAllFileList: readonly string[] | null = null;
+  let cachedNormalizedFileList: readonly string[] | null = null;
+  let cachedResolveCache: Map<string, string | null> | null = null;
+
+  return (targetRaw, fromFile, allFilePaths) => {
+    if (cachedAllFilePaths !== allFilePaths) {
+      cachedAllFilePaths = allFilePaths;
+      cachedSet = new Set(allFilePaths);
+      cachedAllFileList = Array.from(allFilePaths);
+      cachedNormalizedFileList = cachedAllFileList.map((f) => f.toLowerCase());
+      cachedResolveCache = new Map();
+    }
+
+    const ws: TsResolveContext = {
+      fromFile,
+      allFilePaths: cachedSet!,
+      allFileList: cachedAllFileList!,
+      normalizedFileList: cachedNormalizedFileList!,
+      resolveCache: cachedResolveCache!,
+    };
+    return resolveTsTarget(targetRaw, ws);
+  };
+}
+
 const typescriptScopeResolver: ScopeResolver = {
   language: SupportedLanguages.TypeScript,
   languageProvider: typescriptProvider,
   importEdgeReason: 'typescript-scope: import',
 
-  resolveImportTarget: (targetRaw, fromFile, allFilePaths) => {
-    // Copy the orchestrator's `ReadonlySet` into a `Set` because the
-    // underlying standard resolver is typed to receive `Set<string>`
-    // (it uses the set to scan candidate resolutions). O(N) copy
-    // once per import — cost is trivial next to parsing.
-    const ws: TsResolveContext = {
-      fromFile,
-      allFilePaths: new Set(allFilePaths),
-      // tsconfigPaths / pre-normalized lists would be populated by the
-      // orchestrator when available; absent here we let the adapter
-      // derive them on the fly.
-    };
-    return resolveTsImportTarget(
-      { kind: 'named', localName: '_', importedName: '_', targetRaw },
-      ws,
-    );
-  },
+  resolveImportTarget: makeTsResolveImportTarget(),
 
   // TypeScript declaration merging + LEGB: local > import > wildcard,
   // separated by declaration space (value / type / namespace). The

gitnexus/test/fixtures/lang-resolution/typescript-side-effect-imports/src/app.ts

@@ -0,0 +1,7 @@
+import './polyfill';
+import './register';
+import { greet } from './greeter';
+
+export function main(): string {
+  return greet('world');
+}

gitnexus/test/fixtures/lang-resolution/typescript-side-effect-imports/src/greeter.ts

@@ -0,0 +1,3 @@
+export function greet(name: string): string {
+  return `hello, ${name}`;
+}

gitnexus/test/fixtures/lang-resolution/typescript-side-effect-imports/src/polyfill.ts

@@ -0,0 +1,2 @@
+declare const globalThis: { __polyfilled?: boolean };
+globalThis.__polyfilled = true;

gitnexus/test/fixtures/lang-resolution/typescript-side-effect-imports/src/register.ts

@@ -0,0 +1,2 @@
+declare const globalThis: { __registry?: string[] };
+(globalThis.__registry ??= []).push('module-A');

gitnexus/test/integration/resolvers/typescript.test.ts

@@ -359,6 +359,45 @@ describe('TypeScript named import disambiguation', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// Side-effect imports: `import './polyfill'` produces an IMPORTS edge but
+// no local binding (parity with the legacy DAG, which counts side-effect
+// imports as module-reachability dependencies).
+//
+// This describe runs under both `REGISTRY_PRIMARY_TYPESCRIPT=0` (legacy
+// DAG) and `=1` (registry-primary) via the CI parity gate
+// (`.github/workflows/ci-scope-parity.yml`). Both modes must emit the
+// same IMPORTS edges; the registry-primary path emits no extra
+// `BindingRef`s for the side-effect kind.
+// ---------------------------------------------------------------------------
+
+describe('TypeScript side-effect imports', () => {
+  let result: PipelineResult;
+
+  beforeAll(async () => {
+    result = await runPipelineFromRepo(
+      path.join(FIXTURES, 'typescript-side-effect-imports'),
+      () => {},
+    );
+  }, 60000);
+
+  it('emits IMPORTS edges for both side-effect imports + the named import', () => {
+    const imports = getRelationships(result, 'IMPORTS').filter((e) => e.source === 'app.ts');
+    const targets = imports.map((e) => e.targetFilePath).sort();
+    expect(targets).toEqual(['src/greeter.ts', 'src/polyfill.ts', 'src/register.ts']);
+  });
+
+  it('does not synthesize local bindings for side-effect imports', () => {
+    // A side-effect import binds no local name; nothing in `app.ts` should
+    // try to call into `polyfill.ts` or `register.ts`. The only resolved
+    // CALL edge from `main` is to `greet` in `greeter.ts`.
+    const calls = getRelationships(result, 'CALLS').filter((c) => c.source === 'main');
+    expect(calls).toHaveLength(1);
+    expect(calls[0].target).toBe('greet');
+    expect(calls[0].targetFilePath).toBe('src/greeter.ts');
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Alias import resolution: import { User as U } resolves U → User
 // ---------------------------------------------------------------------------

gitnexus/test/unit/scope-resolution/typescript/typescript-captures.test.ts

@@ -230,7 +230,7 @@ describe('emitTsScopeCaptures — imports (decomposed)', () => {
     // `import { A } from './a'`          → 1 match (named)
     // `import B from './b'`              → 1 match (default)
     // `import * as ns from './ns'`       → 1 match (namespace)
-    // `import './polyfill'`              → 0 matches (side-effect; no local binding)
+    // `import './polyfill'`              → 1 match (side-effect; file-level edge only)
     const src = `
       import { A } from './a';
       import B from './b';
@@ -238,7 +238,7 @@ describe('emitTsScopeCaptures — imports (decomposed)', () => {
       import './polyfill';
     `;
     const count = countMatches(src, (t) => t.includes('@import.statement'));
-    expect(count).toBe(3);
+    expect(count).toBe(4);
 
     // Each has the corresponding @import.kind marker.
     const kinds = tagsFor(src)
@@ -247,7 +247,7 @@ describe('emitTsScopeCaptures — imports (decomposed)', () => {
         const idx = tags.findIndex((t) => t === '@import.kind');
         return idx >= 0 ? tags[idx] : null;
       });
-    expect(kinds).toHaveLength(3);
+    expect(kinds).toHaveLength(4);
   });
 
   it('decomposes multi-specifier imports into one match per name', () => {

gitnexus/test/unit/scope-resolution/typescript/typescript-imports.test.ts

@@ -122,9 +122,13 @@ describe('interpretTsImport — static imports', () => {
     expect((ns as { importedName: string }).importedName).toBe('./m');
   });
 
-  it('side-effect: `import "./polyfill"` emits nothing (no local binding)', () => {
+  it('side-effect: `import "./polyfill"` emits a side-effect ParsedImport (no local binding)', () => {
     const imps = importsFor('import "./polyfill";');
-    expect(imps).toHaveLength(0);
+    expect(imps).toHaveLength(1);
+    expect(imps[0]).toEqual({
+      kind: 'side-effect',
+      targetRaw: './polyfill',
+    });
   });
 
   it('preserves the module path as written (no quote stripping leftovers)', () => {