GitNexus/gitnexus/test/unit/cross-file-impl.test.ts
Copilot fa06c5610b
fix: resolve cross-file type propagation stall on large repos (#1626)
* Initial plan

* fix: add time-based deadline to cross-file type propagation to prevent stalling on large repos

Adds a 2-minute wall-clock time limit (DEFAULT_CROSS_FILE_ELAPSED_MS) to
runCrossFileBindingPropagation. When exceeded, the phase gracefully stops
and logs a warning. Users can override via GITNEXUS_CROSS_FILE_TIMEOUT_MS
env var. This prevents the analyze command from stalling for hours on very
large repositories where per-file re-resolution is expensive.

Fixes the reported issue where gitnexus analyze stalls at "Cross-file type
propagation" for several hours on repos with 15000+ files.

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b8341947-557c-4111-a3a8-991ba455ab01

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* fix: root cause - cache tree-sitter queries across files, add live progress reporting

Root cause: cross-file propagation called processCalls() with 1 file at a time,
causing Parser.Query to be recompiled from the query string for every single file
(O(N) compilations vs O(1) for the whole phase). Additionally, progress was only
reported once at the start, making the phase appear completely frozen.

Fixes:
- Add optional `compiledQueryCache` parameter to `processCalls` so callers that
  invoke it with single-file batches can share compiled query objects across calls.
  The cross-file phase now compiles each language's query string exactly once and
  reuses it for all files of that language (e.g. 1 TypeScript compile for 595+ files).
- Pre-count candidate files and emit onProgress every 25 files showing
  "Cross-file type propagation (N/M files)..." so the UI shows real movement
  instead of a frozen bar.
- Keep the wall-clock deadline (GITNEXUS_CROSS_FILE_TIMEOUT_MS) as a safety
  net for pathological inputs.

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/f5028cc8-4bc9-4309-8ffb-798fe2bd7a0a

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* fix: address code review - use SupportedLanguages key type, rename queryCache to compiledQueryCache

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/f5028cc8-4bc9-4309-8ffb-798fe2bd7a0a

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

* fix(cross-file): remove wall-clock timeout from type propagation

The query compilation cache and live progress reporting address the
original stall; the 2-minute deadline could truncate cross-file work on
large repos. MAX_CROSS_FILE_REPROCESS (2000) remains as the only cap.

* test(cross-file): verify compiledQueryCache is shared across all processCalls invocations

Finding 1: O(N) query recompilation was fixed by sharing a compiledQueryCache Map
across all processCalls invocations in runCrossFileBindingPropagation. This test
verifies the fix is correctly wired: the same Map instance is passed as the
12th argument to every call, proving queries are compiled once per language,
not once per file.

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* test(cross-file): verify live progress events are emitted with N/M format

Finding 2: frozen progress display was fixed by emitting onProgress every 25 files
with "Cross-file type propagation (N/M files)..." messages instead of calling it
once at phase start. This test verifies the fix with 50 candidate files: expects
onProgress called 3 times (1 initial + at 25 + at 50) with correct N/M counters.

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* fix(cross-file): skip registry-primary language files before readFileContents

Finding 3 (from comment 4466231612): cross-file-impl was calling processCalls
for every candidate file even when that file's language is registry-primary
(TypeScript, C++, Python, Go, C#, PHP, C — since AGENTS.md v1.7.0). processCalls
would immediately skip those files via its own isRegistryPrimary guard, but
cross-file-impl still paid the full cost: readFileContents I/O, buildImportedReturnTypes,
buildImportedRawReturnTypes, and Map allocation — all discarded.

Fix: check isRegistryPrimary(lang) in both the totalCandidates pre-count loop
and the levelCandidates builder, before any file I/O or map building. This
eliminates 595+ no-op processCalls invocations on large TypeScript repos.

Test: mocks isRegistryPrimary to always return true and verifies that
processCalls is never invoked and result is 0. The mock also defaults to false
in beforeEach so existing tests using .ts files are unaffected.

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* refactor(test): address code review - simplify mock factory, name the arg index constant

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-05-16 10:02:40 +01:00

365 lines
13 KiB
TypeScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* Coverage tests for cross-file-impl.ts — `runCrossFileBindingPropagation`.
*
* Scenarios aimed at branches the integration tests exercise only on the
* happy path:
* 1. gapRatio < CROSS_FILE_SKIP_THRESHOLD → returns 0 without reprocess.
* 2. MAX_CROSS_FILE_REPROCESS cap → outer level loop breaks.
* 3. parse-supplied exportedTypeMap is NEVER mutated by crossFile (cross
* file builds its own local working copy for re-resolution writes).
* 4. namedImportMap.size === 0 → returns 0 immediately.
*
* Note: `processCalls`, `readFileContents`, and `isLanguageAvailable` are
* mocked so the test doesn't require tree-sitter or filesystem access.
* `buildImportedReturnTypes` and `buildImportedRawReturnTypes` are preserved
* via `importOriginal`. The graph-fallback enrichment that used to live here
* was moved into parse-impl's `runChunkedParseAndResolve` so the parse phase
* hands crossFile a fully-populated, truly read-only map.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
vi.mock('../../src/core/ingestion/call-processor.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('../../src/core/ingestion/call-processor.js')>();
return {
...actual,
processCalls: vi.fn(async () => {}),
};
});
vi.mock('../../src/core/ingestion/filesystem-walker.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('../../src/core/ingestion/filesystem-walker.js')>();
return {
...actual,
readFileContents: vi.fn(async (_repo: string, paths: string[]) => {
const m = new Map<string, string>();
for (const p of paths) m.set(p, '// stub');
return m;
}),
};
});
vi.mock('../../src/core/tree-sitter/parser-loader.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('../../src/core/tree-sitter/parser-loader.js')>();
return {
...actual,
isLanguageAvailable: vi.fn(() => true),
};
});
// Default to non-registry-primary so existing tests (which use .ts files) are
// not affected by the isRegistryPrimary guard added in cross-file-impl. Tests
// that verify the skip behavior can override this with mockReturnValue(true).
vi.mock('../../src/core/ingestion/registry-primary-flag.js', () => ({
isRegistryPrimary: vi.fn(() => false),
}));
import { runCrossFileBindingPropagation } from '../../src/core/ingestion/pipeline-phases/cross-file-impl.js';
import { processCalls } from '../../src/core/ingestion/call-processor.js';
import { isRegistryPrimary } from '../../src/core/ingestion/registry-primary-flag.js';
import { createResolutionContext } from '../../src/core/ingestion/model/resolution-context.js';
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
import type { ExportedTypeMap } from '../../src/core/ingestion/call-processor.js';
const processCallsMock = vi.mocked(processCalls);
const isRegistryPrimaryMock = vi.mocked(isRegistryPrimary);
/**
* Index of the `compiledQueryCache` parameter in the `processCalls` signature.
* graph(0), files(1), astCache(2), ctx(3), onProgress?(4), exportedTypeMap?(5),
* importedBindingsMap?(6), importedReturnTypesMap?(7),
* importedRawReturnTypesMap?(8), heritageMap?(9), bindingAccumulator?(10),
* compiledQueryCache?(11).
*/
const COMPILED_QUERY_CACHE_ARG_INDEX = 11;
describe('runCrossFileBindingPropagation', () => {
beforeEach(() => {
processCallsMock.mockClear();
isRegistryPrimaryMock.mockReturnValue(false); // reset to non-primary before each test
});
it('returns 0 immediately when namedImportMap is empty', async () => {
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
const result = await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(['upstream.ts']),
1,
'/repo',
Date.now(),
() => {},
);
expect(result).toBe(0);
expect(processCallsMock).not.toHaveBeenCalled();
});
it('returns 0 when gapRatio < CROSS_FILE_SKIP_THRESHOLD', async () => {
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
// 100 files total; exportedTypeMap has an export but no downstream
// namedImportMap entry references a matching name → zero gaps.
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
// One downstream importer whose binding points at a symbol NOT in
// exportedTypeMap and NOT in ctx.model.symbols → no gap-filling seed
// available, so filesWithGaps stays at 0.
const downstreamBindings = new Map();
downstreamBindings.set('Missing', {
sourcePath: 'upstream.ts',
exportedName: 'Missing',
});
ctx.namedImportMap.set('downstream.ts', downstreamBindings);
const totalFiles = 100; // threshold = ceil(100 * 0.03) = 3
const result = await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(['downstream.ts', 'upstream.ts']),
totalFiles,
'/repo',
Date.now(),
() => {},
);
expect(result).toBe(0);
expect(processCallsMock).not.toHaveBeenCalled();
});
it('does not mutate the parse-supplied exportedTypeMap (works on a local copy)', async () => {
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
// Seed a single upstream export and a downstream importer so the gap
// ratio crosses the skip threshold and processCalls (mocked) is invoked.
const parseExports: ExportedTypeMap = new Map([['upstream.ts', new Map([['User', 'User']])]]);
const parseExportsSnapshot = new Map(
Array.from(parseExports, ([k, v]) => [k, new Map(v)] as const),
);
const bindings = new Map();
bindings.set('User', { sourcePath: 'upstream.ts', exportedName: 'User' });
ctx.namedImportMap.set('downstream.ts', bindings);
ctx.importMap.set('upstream.ts', new Set());
ctx.importMap.set('downstream.ts', new Set(['upstream.ts']));
await runCrossFileBindingPropagation(
graph,
ctx,
parseExports,
new Set(['downstream.ts', 'upstream.ts']),
10,
'/repo',
Date.now(),
() => {},
);
// Outer map identity preserved, sizes unchanged, inner Maps unchanged —
// crossFile must operate on its own working copy.
expect(parseExports.size).toBe(parseExportsSnapshot.size);
for (const [k, v] of parseExportsSnapshot) {
const after = parseExports.get(k);
expect(after).toBeDefined();
expect(after!.size).toBe(v.size);
for (const [innerK, innerV] of v) {
expect(after!.get(innerK)).toBe(innerV);
}
}
});
it('passes the same compiledQueryCache Map instance to every processCalls call', async () => {
// Verifies that the O(N)→O(1) query-cache fix is correctly wired: the
// `compiledQueryCache` created in runCrossFileBindingPropagation is shared
// across all processCalls invocations so each language's Parser.Query is
// compiled exactly once, not once per file.
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
ctx.importMap.set('upstream.ts', new Set());
const allPaths = ['upstream.ts'];
for (let i = 0; i < 3; i++) {
const file = `downstream${i}.ts`;
allPaths.push(file);
const bindings = new Map();
bindings.set('User', { sourcePath: 'upstream.ts', exportedName: 'User' });
ctx.namedImportMap.set(file, bindings);
ctx.importMap.set(file, new Set(['upstream.ts']));
}
await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(allPaths),
allPaths.length,
'/repo',
Date.now(),
() => {},
);
expect(processCallsMock).toHaveBeenCalledTimes(3);
// Argument index 11 is compiledQueryCache — see COMPILED_QUERY_CACHE_ARG_INDEX.
const caches = processCallsMock.mock.calls.map((call) => call[COMPILED_QUERY_CACHE_ARG_INDEX]);
// Every call must receive a non-null Map (not undefined).
for (const cache of caches) {
expect(cache).toBeDefined();
expect(cache).toBeInstanceOf(Map);
}
// All calls share the SAME instance — the whole point of the cache.
expect(caches[1]).toBe(caches[0]);
expect(caches[2]).toBe(caches[0]);
});
it('emits live onProgress events every 25 files with N/M format', async () => {
// Verifies that the frozen-progress-display fix is correctly wired:
// onProgress must be called multiple times from the processing loop,
// not just once at phase start, so large repos show real movement in
// the UI instead of a frozen percentage bar.
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
ctx.importMap.set('upstream.ts', new Set());
const allPaths = ['upstream.ts'];
for (let i = 0; i < 50; i++) {
const file = `downstream${i}.ts`;
allPaths.push(file);
const bindings = new Map();
bindings.set('User', { sourcePath: 'upstream.ts', exportedName: 'User' });
ctx.namedImportMap.set(file, bindings);
ctx.importMap.set(file, new Set(['upstream.ts']));
}
const progressMessages: string[] = [];
const onProgress = vi.fn((p: { phase: string; percent: number; message: string }) => {
progressMessages.push(p.message);
});
await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(allPaths),
allPaths.length,
'/repo',
Date.now(),
onProgress,
);
// 1 initial call at phase start + 2 loop calls (at 25 and 50 files).
expect(onProgress).toHaveBeenCalledTimes(3);
// Loop messages must carry the "N/M files" format so the UI is informative.
const loopMessages = progressMessages.filter((m) => m.match(/\(\d+\/\d+ files\)/));
expect(loopMessages).toHaveLength(2);
expect(loopMessages[0]).toContain('(25/50 files)');
expect(loopMessages[1]).toContain('(50/50 files)');
});
it('caps processing at MAX_CROSS_FILE_REPROCESS (2000)', async () => {
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
// Seed one upstream export reused by every downstream file.
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
const allPaths: string[] = ['upstream.ts'];
// Create 2100 downstream importers — each will qualify as a candidate
// (seeded.size === 1 because upstream.ts has the export we bind to).
// Populate ctx.importMap so topologicalLevelSort returns real levels.
ctx.importMap.set('upstream.ts', new Set());
for (let i = 0; i < 2100; i++) {
const file = `downstream${i}.ts`;
allPaths.push(file);
const bindings = new Map();
bindings.set('User', { sourcePath: 'upstream.ts', exportedName: 'User' });
ctx.namedImportMap.set(file, bindings);
ctx.importMap.set(file, new Set(['upstream.ts']));
}
const totalFiles = allPaths.length;
const result = await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(allPaths),
totalFiles,
'/repo',
Date.now(),
() => {},
);
// Hard cap is 2000. The function returns `crossFileResolved`, which
// equals MAX_CROSS_FILE_REPROCESS once the cap is hit.
expect(result).toBe(2000);
expect(processCallsMock).toHaveBeenCalledTimes(2000);
});
it('skips registry-primary language files without calling processCalls', async () => {
// Finding 3: on large TypeScript/C++ repos (registry-primary since v1.6.4+)
// cross-file-impl was calling processCalls 595× per candidate only for
// processCalls to immediately return (isRegistryPrimary guard inside).
// Now cross-file-impl filters them out BEFORE readFileContents so we avoid
// the I/O cost and map-building overhead entirely.
const graph = createKnowledgeGraph();
const ctx = createResolutionContext();
const exportedTypeMap: ExportedTypeMap = new Map([
['upstream.ts', new Map([['User', 'User']])],
]);
ctx.importMap.set('upstream.ts', new Set());
const allPaths = ['upstream.ts'];
for (let i = 0; i < 5; i++) {
const file = `downstream${i}.ts`;
allPaths.push(file);
const bindings = new Map();
bindings.set('User', { sourcePath: 'upstream.ts', exportedName: 'User' });
ctx.namedImportMap.set(file, bindings);
ctx.importMap.set(file, new Set(['upstream.ts']));
}
// Simulate all files being registry-primary (e.g. TypeScript on main branch).
isRegistryPrimaryMock.mockReturnValue(true);
const result = await runCrossFileBindingPropagation(
graph,
ctx,
exportedTypeMap,
new Set(allPaths),
allPaths.length,
'/repo',
Date.now(),
() => {},
);
// No files are candidates; no processCalls invocations.
expect(result).toBe(0);
expect(processCallsMock).not.toHaveBeenCalled();
});
});