mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(type-resolution): address Phase 14 code review findings
Critical fixes: - Re-resolution pass now actually re-resolves CALLS edges by calling processCalls with importedBindingsMap (was building typeEnv but discarding it without producing edges) - Worker path populates ExportedTypeMap via buildExportedTypeMapFromGraph using graph node isExported + SymbolTable returnType/declaredType (was dead parameter in processCallsFromExtracted) Important fixes: - Skip threshold denominator uses totalFiles (was exportedTypeMap.size + filesWithGaps which made threshold nearly useless) - processCalls accepts importedBindingsMap parameter to thread cross-file bindings into buildTypeEnv during re-resolution All 3454 tests pass.
This commit is contained in:
parent
a6a1004e82
commit
56bc226a1c
2 changed files with 54 additions and 34 deletions
|
|
@ -1,6 +1,6 @@
|
|||
import { KnowledgeGraph } from '../graph/types.js';
|
||||
import { ASTCache } from './ast-cache.js';
|
||||
import type { SymbolDefinition } from './symbol-table.js';
|
||||
import type { SymbolDefinition, SymbolTable } from './symbol-table.js';
|
||||
import Parser from 'tree-sitter';
|
||||
import type { ResolutionContext } from './resolution-context.js';
|
||||
import { TIER_CONFIDENCE, type ResolutionTier } from './resolution-context.js';
|
||||
|
|
@ -65,6 +65,37 @@ function collectExportedBindings(
|
|||
return exported.size > 0 ? exported : null;
|
||||
}
|
||||
|
||||
/** Build ExportedTypeMap from graph nodes — used for worker path where TypeEnv
|
||||
* is not available in the main thread. Collects returnType/declaredType from
|
||||
* exported symbols that have callables with known return types. */
|
||||
export function buildExportedTypeMapFromGraph(
|
||||
graph: KnowledgeGraph,
|
||||
symbolTable: SymbolTable,
|
||||
): ExportedTypeMap {
|
||||
const result: ExportedTypeMap = new Map();
|
||||
graph.forEachNode(node => {
|
||||
if (!node.properties?.isExported) return;
|
||||
if (!node.properties?.filePath || !node.properties?.name) return;
|
||||
const filePath = node.properties.filePath as string;
|
||||
const name = node.properties.name as string;
|
||||
if (!name || name.length > MAX_TYPE_NAME_LENGTH) return;
|
||||
// For callable symbols, use returnType; for properties/variables, use declaredType
|
||||
const def = symbolTable.lookupExactFull(filePath, name);
|
||||
if (!def) return;
|
||||
const typeName = def.returnType ?? def.declaredType;
|
||||
if (!typeName || typeName.length > MAX_TYPE_NAME_LENGTH) return;
|
||||
// Extract simple type name (strip Promise<>, etc.)
|
||||
const simpleType = typeName.replace(/^(?:Promise|Observable|Task|Future|CompletableFuture)\s*<\s*/, '').replace(/\s*>\s*$/, '') || typeName;
|
||||
if (!simpleType) return;
|
||||
let fileExports = result.get(filePath);
|
||||
if (!fileExports) { fileExports = new Map(); result.set(filePath, fileExports); }
|
||||
if (fileExports.size < MAX_EXPORTS_PER_FILE) {
|
||||
fileExports.set(name, simpleType);
|
||||
}
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
// Stdlib methods that preserve the receiver's type identity. When TypeEnv already
|
||||
// strips nullable wrappers (Option<User> → User), these chain steps are no-ops
|
||||
// for type resolution — the current type passes through unchanged.
|
||||
|
|
@ -171,6 +202,8 @@ export const processCalls = async (
|
|||
ctx: ResolutionContext,
|
||||
onProgress?: (current: number, total: number) => void,
|
||||
exportedTypeMap?: ExportedTypeMap,
|
||||
/** Phase 14: pre-resolved cross-file bindings to seed into buildTypeEnv. Keyed by filePath → Map<localName, typeName>. */
|
||||
importedBindingsMap?: ReadonlyMap<string, ReadonlyMap<string, string>>,
|
||||
): Promise<ExtractedHeritage[]> => {
|
||||
const parser = await loadParser();
|
||||
const collectedHeritage: ExtractedHeritage[] = [];
|
||||
|
|
@ -256,7 +289,8 @@ export const processCalls = async (
|
|||
}
|
||||
}
|
||||
|
||||
const typeEnv = lang ? buildTypeEnv(tree, lang, { symbolTable: ctx.symbols, parentMap }) : null;
|
||||
const importedBindings = importedBindingsMap?.get(file.path);
|
||||
const typeEnv = lang ? buildTypeEnv(tree, lang, { symbolTable: ctx.symbols, parentMap, importedBindings }) : null;
|
||||
if (typeEnv && exportedTypeMap) {
|
||||
const fileExports = collectExportedBindings(typeEnv, file.path, ctx.symbols, graph);
|
||||
if (fileExports) exportedTypeMap.set(file.path, fileExports);
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import {
|
|||
processImportsFromExtracted,
|
||||
buildImportResolutionContext
|
||||
} from './import-processor.js';
|
||||
import { processCalls, processCallsFromExtracted, processAssignmentsFromExtracted, processRoutesFromExtracted, type ExportedTypeMap } from './call-processor.js';
|
||||
import { processCalls, processCallsFromExtracted, processAssignmentsFromExtracted, processRoutesFromExtracted, type ExportedTypeMap, buildExportedTypeMapFromGraph } from './call-processor.js';
|
||||
import { processHeritage, processHeritageFromExtracted } from './heritage-processor.js';
|
||||
import { computeMRO } from './mro-processor.js';
|
||||
import { processCommunities } from './community-processor.js';
|
||||
|
|
@ -398,6 +398,14 @@ export const runPipelineFromRepo = async (
|
|||
// Uses namedImportMap (populated during import processing) to determine
|
||||
// which exported bindings each file needs. Files processed in topological
|
||||
// import order so upstream bindings are available when downstream runs.
|
||||
|
||||
// For the worker path, buildTypeEnv runs inside workers without SymbolTable,
|
||||
// so exported bindings must be collected from graph + SymbolTable in main thread.
|
||||
if (exportedTypeMap.size === 0 && graph.nodeCount > 0) {
|
||||
const graphExports = buildExportedTypeMapFromGraph(graph, ctx.symbols);
|
||||
for (const [fp, exports] of graphExports) exportedTypeMap.set(fp, exports);
|
||||
}
|
||||
|
||||
if (exportedTypeMap.size > 0 && ctx.namedImportMap.size > 0) {
|
||||
const allPathSet = new Set(allPaths);
|
||||
const levels = topologicalLevelSort(ctx.importMap);
|
||||
|
|
@ -414,13 +422,12 @@ export const runPipelineFromRepo = async (
|
|||
}
|
||||
}
|
||||
|
||||
const totalProcessed = exportedTypeMap.size + filesWithGaps;
|
||||
const gapRatio = totalProcessed > 0 ? filesWithGaps / totalProcessed : 0;
|
||||
const CROSS_FILE_SKIP_THRESHOLD = 0.03;
|
||||
const gapRatio = totalFiles > 0 ? filesWithGaps / totalFiles : 0;
|
||||
|
||||
if (gapRatio < CROSS_FILE_SKIP_THRESHOLD) {
|
||||
if (isDev) {
|
||||
console.log(`⏭️ Cross-file re-resolution skipped (${filesWithGaps} files, ${(gapRatio * 100).toFixed(1)}% < ${CROSS_FILE_SKIP_THRESHOLD * 100}% threshold)`);
|
||||
console.log(`⏭️ Cross-file re-resolution skipped (${filesWithGaps}/${totalFiles} files, ${(gapRatio * 100).toFixed(1)}% < ${CROSS_FILE_SKIP_THRESHOLD * 100}% threshold)`);
|
||||
}
|
||||
} else {
|
||||
onProgress({
|
||||
|
|
@ -461,37 +468,16 @@ export const runPipelineFromRepo = async (
|
|||
const content = contentMap.get(filePath);
|
||||
if (!content) continue;
|
||||
|
||||
const parser = await loadParser();
|
||||
await loadLanguage(lang, filePath);
|
||||
const bufferSize = getTreeSitterBufferSize(content.length);
|
||||
if (bufferSize) (parser as any).setBufferSize?.(bufferSize);
|
||||
const tree = parser.parse(content);
|
||||
|
||||
const typeEnv = buildTypeEnv(tree, lang, {
|
||||
symbolTable: ctx.symbols,
|
||||
importedBindings: seeded,
|
||||
});
|
||||
|
||||
// Collect updated exports for downstream propagation
|
||||
const fileScope = typeEnv.env.get('');
|
||||
if (fileScope) {
|
||||
const updated = new Map<string, string>();
|
||||
for (const [varName, typeName] of fileScope) {
|
||||
if (updated.size >= 500) break;
|
||||
if (!typeName || typeName.length > 256) continue;
|
||||
const nodeId = ctx.symbols.lookupExact(filePath, varName);
|
||||
if (!nodeId) continue;
|
||||
const node = graph.getNode(nodeId);
|
||||
if (node?.properties?.isExported) {
|
||||
updated.set(varName, typeName);
|
||||
}
|
||||
}
|
||||
if (updated.size > 0) exportedTypeMap.set(filePath, updated);
|
||||
}
|
||||
// Re-parse and re-resolve calls with cross-file seeded type environment
|
||||
const reFile = [{ path: filePath, content }];
|
||||
const bindings = new Map<string, ReadonlyMap<string, string>>();
|
||||
bindings.set(filePath, seeded);
|
||||
astCache = createASTCache(1);
|
||||
await processCalls(graph, reFile, astCache, ctx, undefined, exportedTypeMap, bindings);
|
||||
astCache.clear();
|
||||
|
||||
crossFileResolved++;
|
||||
}
|
||||
astCache.clear();
|
||||
}
|
||||
|
||||
if (isDev) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue