mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
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>
This commit is contained in:
parent
0920628b7d
commit
6ab3307e7a
2 changed files with 70 additions and 0 deletions
|
|
@ -23,6 +23,7 @@ import {
|
|||
} from 'gitnexus-shared';
|
||||
import { readFileContents } from '../filesystem-walker.js';
|
||||
import { isLanguageAvailable } from '../../tree-sitter/parser-loader.js';
|
||||
import { isRegistryPrimary } from '../registry-primary-flag.js';
|
||||
import { topologicalLevelSort } from '../utils/graph-sort.js';
|
||||
import type { KnowledgeGraph } from '../../graph/types.js';
|
||||
import { isDev } from '../utils/env.js';
|
||||
|
|
@ -136,6 +137,11 @@ export async function runCrossFileBindingPropagation(
|
|||
if (!allPathSet.has(filePath)) continue;
|
||||
const lang = getLanguageFromFilename(filePath);
|
||||
if (!lang || !isLanguageAvailable(lang)) continue;
|
||||
// Registry-primary languages have their call resolution handled by the
|
||||
// scope-resolution pipeline — processCalls skips them immediately. Skip
|
||||
// here too so we avoid the I/O cost (readFileContents) and map-building
|
||||
// overhead for files that would be no-ops anyway.
|
||||
if (isRegistryPrimary(lang)) continue;
|
||||
totalCandidates++;
|
||||
}
|
||||
if (totalCandidates >= MAX_CROSS_FILE_REPROCESS) break;
|
||||
|
|
@ -181,6 +187,10 @@ export async function runCrossFileBindingPropagation(
|
|||
|
||||
const lang = getLanguageFromFilename(filePath);
|
||||
if (!lang || !isLanguageAvailable(lang)) continue;
|
||||
// Registry-primary languages have their call resolution handled by the
|
||||
// scope-resolution pipeline — processCalls skips them immediately. Skip
|
||||
// here to avoid readFileContents I/O and map-building for no-op files.
|
||||
if (isRegistryPrimary(lang)) continue;
|
||||
|
||||
levelCandidates.push({ filePath, seeded, importedReturns, importedRawReturns });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -49,17 +49,34 @@ vi.mock('../../src/core/tree-sitter/parser-loader.js', async (importOriginal) =>
|
|||
};
|
||||
});
|
||||
|
||||
// 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', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<
|
||||
typeof import('../../src/core/ingestion/registry-primary-flag.js')
|
||||
>();
|
||||
return {
|
||||
...actual,
|
||||
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);
|
||||
|
||||
describe('runCrossFileBindingPropagation', () => {
|
||||
beforeEach(() => {
|
||||
processCallsMock.mockClear();
|
||||
isRegistryPrimaryMock.mockReturnValue(false); // reset to non-primary before each test
|
||||
});
|
||||
|
||||
it('returns 0 immediately when namedImportMap is empty', async () => {
|
||||
|
|
@ -300,4 +317,47 @@ describe('runCrossFileBindingPropagation', () => {
|
|||
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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue