diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 35a59dab4..1b5a234b4 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -766,6 +766,15 @@ export const processCalls = async ( importedRawReturnTypesMap?: ReadonlyMap>, heritageMap?: HeritageMap, bindingAccumulator?: BindingAccumulator, + /** + * Optional cache for compiled `Parser.Query` objects keyed by language name. + * When provided, compiled queries are reused across calls instead of being + * re-compiled from the query string for every file. Callers that invoke + * `processCalls` many times with single-file batches (e.g. the cross-file + * propagation phase) should pass a long-lived map here to avoid O(N) + * query recompilation overhead. + */ + compiledQueryCache?: Map, ): Promise => { const parser = await loadParser(); const collectedHeritage: ExtractedHeritage[] = []; @@ -843,7 +852,11 @@ export const processCalls = async ( let matches; try { const lang = parser.getLanguage(); - const query = new Parser.Query(lang, queryStr); + let query = compiledQueryCache?.get(language); + if (!query) { + query = new Parser.Query(lang, queryStr); + compiledQueryCache?.set(language, query); + } matches = query.matches(tree.rootNode); } catch (queryError) { logger.warn({ queryError }, `Query error for ${file.path}:`); diff --git a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts new file mode 100644 index 000000000..2a9e3bc01 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts @@ -0,0 +1,47 @@ +/** + * C++ conversion-rank scoring for overload resolution (#1578). + * + * Operates on **normalized** type strings (output of + * `normalizeCppParamType` in `arity-metadata.ts`). After normalization: + * - int/long/short/unsigned → 'int' + * - float/double → 'double' + * - char → 'char', bool → 'bool' + * + * Because the normalizer collapses promotion pairs (int↔long, + * float↔double) to the same string, those promotions are invisible at + * this layer — they appear as exact matches (rank 0). + * + * Post-normalization ranking: + * - rank 0 — exact (same normalized type) + * - rank 1 — integral promotion (char→int, bool→int) + * - rank 2 — standard arithmetic conversion (int↔double, char→double, + * bool→double) + * - Infinity — mismatch (string↔int, user types, pointers, etc.) + * + * This function is intentionally C++-specific (issue #1578 pitfall: + * keep conversion-rank tables out of shared overload-narrowing). Other + * languages may define their own `ConversionRankFn` in the future. + */ + +/** Set of normalized arithmetic types that support implicit conversion. */ +const ARITHMETIC = new Set(['int', 'double', 'char', 'bool']); + +/** Integral promotion targets: char→int and bool→int are rank 1. */ +const INTEGRAL_PROMOTION = new Map([ + ['char', 'int'], + ['bool', 'int'], +]); + +/** + * Return the conversion rank from `argType` to `paramType`. + * + * @returns 0 for exact match, 1 for integral promotion (char/bool→int), + * 2 for standard arithmetic conversion, Infinity for mismatch. + */ +export function cppConversionRank(argType: string, paramType: string): number { + if (argType === paramType) return 0; + // Integral promotions: char→int, bool→int (ISO C++ [conv.prom]) + if (INTEGRAL_PROMOTION.get(argType) === paramType) return 1; + if (ARITHMETIC.has(argType) && ARITHMETIC.has(paramType)) return 2; + return Infinity; +} diff --git a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts index 52c575cf4..4e226bbac 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts @@ -9,6 +9,7 @@ import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers. import type { ScopeResolver } from '../../scope-resolution/contract/scope-resolver.js'; import { cppProvider } from '../c-cpp.js'; import { cppArityCompatibility } from './arity.js'; +import { cppConversionRank } from './conversion-rank.js'; import { cppMergeBindings } from './merge-bindings.js'; import { resolveCppImportTarget } from './import-target.js'; import { scanCppHeaderFiles } from './header-scan.js'; @@ -169,6 +170,10 @@ export const cppScopeResolver: ScopeResolver = { propagatesReturnTypesAcrossImports: true, // C++ #include brings in all symbols — enable global free call fallback allowGlobalFreeCallFallback: true, + // C++ standard-conversion-sequence ranking for overload resolution (#1578). + // Disambiguates `f(int)` vs `f(double)` called with `f(2.5)` by scoring + // each candidate's conversion cost; exact match wins over standard conversion. + conversionRankFn: cppConversionRank, // Range-for element type inference: for (auto& user : users) → bind user to User populateRangeBindings: populateCppRangeBindings, // C++ method return-type bindings need to be visible from module scope diff --git a/gitnexus/src/core/ingestion/pipeline-phases/cross-file-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/cross-file-impl.ts index 5c014ed73..6c10ccb11 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/cross-file-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/cross-file-impl.ts @@ -16,12 +16,18 @@ import { } from '../call-processor.js'; import type { createResolutionContext } from '../model/resolution-context.js'; import { createASTCache } from '../ast-cache.js'; -import { type PipelineProgress, getLanguageFromFilename } from 'gitnexus-shared'; +import { + type PipelineProgress, + getLanguageFromFilename, + type SupportedLanguages, +} 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'; +import type Parser from 'tree-sitter'; import { logger } from '../../logger.js'; /** Max AST trees to keep in LRU cache for cross-file binding propagation. */ @@ -114,6 +120,36 @@ export async function runCrossFileBindingPropagation( let crossFileResolved = 0; const crossFileStart = Date.now(); const astCache = createASTCache(AST_CACHE_CAP); + // Compiled query objects keyed by language name. Shared across all processCalls + // invocations in this phase so the same tree-sitter query string is only + // compiled once per language instead of once per file (O(1) vs O(N)). + const compiledQueryCache = new Map(); + + // Snapshot total topological candidates for progress math. We walk the + // levels once more here (fast — no I/O) so we can report meaningful + // percentages rather than a frozen display. + let totalCandidates = 0; + for (const level of levels) { + for (const filePath of level) { + if (totalCandidates >= MAX_CROSS_FILE_REPROCESS) break; + const imports = ctx.namedImportMap.get(filePath); + if (!imports) continue; + 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; + } + const cappedTotal = Math.min(totalCandidates, MAX_CROSS_FILE_REPROCESS); + + /** Emit a progress event every PROGRESS_INTERVAL files so the UI stays alive. */ + const PROGRESS_INTERVAL = 25; for (const level of levels) { const levelCandidates: { @@ -151,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 }); } @@ -188,8 +228,24 @@ export async function runCrossFileBindingPropagation( bindings.size > 0 ? bindings : undefined, importedReturnTypesMap.size > 0 ? importedReturnTypesMap : undefined, importedRawReturnTypesMap.size > 0 ? importedRawReturnTypesMap : undefined, + undefined, + undefined, + compiledQueryCache, ); crossFileResolved++; + + // Emit progress every PROGRESS_INTERVAL files so the UI shows real + // movement instead of a frozen display (cross-file can take minutes + // on large repos with many cross-file imports). + if (crossFileResolved % PROGRESS_INTERVAL === 0 || crossFileResolved === cappedTotal) { + const pct = cappedTotal > 0 ? Math.round((crossFileResolved / cappedTotal) * 8) : 0; + onProgress({ + phase: 'parsing', + percent: 82 + pct, + message: `Cross-file type propagation (${crossFileResolved}/${cappedTotal} files)...`, + stats: { filesProcessed: crossFileResolved, totalFiles, nodesCreated: graph.nodeCount }, + }); + } } if (crossFileResolved >= MAX_CROSS_FILE_REPROCESS) { diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index 315471de1..c6f494368 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -264,6 +264,7 @@ import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { LanguageProvider } from '../../language-provider.js'; import { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import type { SemanticModel } from '../../model/semantic-model.js'; +import type { ConversionRankFn } from '../passes/overload-narrowing.js'; /** A LinearizeStrategy receives the full ancestor map so C3-style * algorithms (which need to merge each parent's MRO) can implement @@ -533,6 +534,20 @@ export interface ScopeResolver { */ readonly allowGlobalFreeCallFallback?: boolean; + /** + * Optional per-slot conversion-rank function for overload resolution. + * When provided, `narrowOverloadCandidates` uses ranked scoring as a + * fallback when the exact-type filter produces no match. The function + * returns a numeric cost (0 = exact, 1 = promotion, 2 = standard + * conversion, Infinity = incompatible) for converting an argument + * type to a parameter type. + * + * The conversion-rank table is language-specific (issue #1578 pitfall: + * keep it out of shared overload-narrowing). C++ provides + * `cppConversionRank`; other languages define their own if needed. + */ + readonly conversionRankFn?: ConversionRankFn; + /** * Optional predicate to identify definitions with file-local linkage * (e.g. C `static` functions). When provided, `pickUniqueGlobalCallable` diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index d9e4cdaa3..2be4a6809 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -25,6 +25,7 @@ import type { WorkspaceResolutionIndex } from '../workspace-index.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js'; import { + findAllCallableBindingsInScope, findCallableBindingInScope, findCallableBindingsAndAdlBlocker, findClassBindingInScope, @@ -32,6 +33,7 @@ import { import { isOverloadAmbiguousAfterNormalization, narrowOverloadCandidates, + type ConversionRankFn, } from './overload-narrowing.js'; export function emitFreeCallFallback( @@ -63,6 +65,7 @@ export function emitFreeCallFallback( scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], ) => readonly SymbolDefinition[] | undefined; + readonly conversionRankFn?: ConversionRankFn; } = {}, ): number { let emitted = 0; @@ -90,16 +93,59 @@ export function emitFreeCallFallback( // the same name in a single class, choose the best match by // arity + argument types. if (fnDef === undefined) { - fnDef = pickImplicitThisOverload(site, scopes, workspaceIndex, model); + fnDef = pickImplicitThisOverload( + site, + scopes, + workspaceIndex, + model, + options.conversionRankFn, + ); } + // Scope-chain callable lookup. First-match preserves scope-chain + // precedence (local shadows import). When a conversion-rank function + // is available AND the binding scope contains multiple overloads, + // refine with `narrowOverloadCandidates` to pick the best overload + // by argument types (#1578). The first-match result is kept as a + // fallback when narrowing is indeterminate. if (fnDef === undefined) { if (options.resolveAdlCandidates === undefined) { + // Non-ADL path: first-match preserves scope-chain precedence + // (local shadows import). When a conversion-rank function is + // available AND the binding scope contains multiple overloads, + // refine with narrowOverloadCandidates (#1578). fnDef = findCallableBindingInScope(site.inScope, site.name, scopes); + if (fnDef !== undefined && options.conversionRankFn !== undefined) { + const allCallables = findAllCallableBindingsInScope(site.inScope, site.name, scopes); + if (allCallables.length > 1) { + const narrowed = narrowOverloadCandidates( + allCallables, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); + if (narrowed.length === 1) { + fnDef = narrowed[0]; + } else if (narrowed.length > 1) { + // Multiple survivors after conversion-rank scoring. + // Suppress when all candidates share the same file (true + // overloads) — mirrors ADL merged-candidate path behavior. + // Cross-file candidates are shadowing; keep first-match. + const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); + if (sameFile) { + handledSites.add( + `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`, + ); + continue; + } + } + // narrowed.length === 0: keep the first-match fnDef — + // preserves local-shadows-import. + } + } } else { - // ISO C++ `[basic.lookup.unqual]` §7: ADL is suppressed when - // ordinary lookup finds a non-function name (variable, class, enum) - // or a block-scope function declaration (not via using-declaration) - // at the nearest scope where the name exists. + // ADL path: ISO C++ `[basic.lookup.unqual]` §7 — ADL is suppressed + // when ordinary lookup finds a non-function name or a block-scope + // function declaration. const { callables: ordinary, nonCallableFound, @@ -120,43 +166,67 @@ export function emitFreeCallFallback( parsedFiles, ); - // Preserve existing ordinary-lookup behavior when ADL contributed - // no candidates. + // When ADL contributed no candidates, narrow ordinary candidates + // with conversion-rank scoring when multiple overloads exist. + // Single candidate or empty falls through to first-match. if (adl === undefined || adl.length === 0) { - fnDef = ordinary[0]; + if (ordinary.length <= 1 || options.conversionRankFn === undefined) { + fnDef = ordinary[0]; + } else { + const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; + const narrowed = narrowOverloadCandidates( + ordinary, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); + if (narrowed.length === 1) { + fnDef = narrowed[0]; + } else if (narrowed.length > 1) { + // Multiple survivors — suppress when same-file (true + // overloads), mirrors ADL merged-candidate behavior. + const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); + if (sameFile) { + handledSites.add(siteKey); + continue; + } + fnDef = ordinary[0]; // cross-file shadowing → first-match + } else { + fnDef = ordinary[0]; // narrowed empty → first-match + } + } } else { const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; const merged: SymbolDefinition[] = []; - const seen = new Set(); + const seenMerge = new Set(); const push = (defs: readonly SymbolDefinition[]): void => { for (const d of defs) { - if (seen.has(d.nodeId)) continue; - seen.add(d.nodeId); + if (seenMerge.has(d.nodeId)) continue; + seenMerge.add(d.nodeId); merged.push(d); } }; push(ordinary); push(adl); - const narrowed = narrowOverloadCandidates(merged, site.arity, site.argumentTypes); + const narrowed = narrowOverloadCandidates( + merged, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); if (narrowed.length === 1) { fnDef = narrowed[0]; } else if (narrowed.length === 0) { - // ADL contributed candidates, but none survived arity/type - // narrowing. Treat as handled to avoid global-name fallback - // binding to the same mismatched symbol by simple-name - // uniqueness. handledSites.add(siteKey); continue; } else if (narrowed.length > 1) { - // Suppress ambiguous overload calls (emit zero edges) when - // merged ordinary+ADL candidate sets cannot be disambiguated. if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { handledSites.add(siteKey); continue; } - // Multiple survivors remain but no conversion-ranking step - // exists yet; suppress instead of picking arbitrarily. + // Multiple survivors remain after conversion-rank scoring; + // suppress instead of picking arbitrarily. handledSites.add(siteKey); continue; } @@ -184,6 +254,8 @@ export function emitFreeCallFallback( scopes, }) : undefined, + site.argumentTypes, + options.conversionRankFn, ); } if (fnDef === undefined) continue; @@ -222,6 +294,8 @@ function pickUniqueGlobalCallable( isFileLocalDef?: (def: SymbolDefinition) => boolean, callArity?: number, isCallerVisible?: (candidate: SymbolDefinition) => boolean, + callArgTypes?: readonly string[], + conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { const scopeDefs: SymbolDefinition[] = []; const scopeSeen = new Set(); @@ -256,6 +330,14 @@ function pickUniqueGlobalCallable( const arityMatch = narrowByArity(scopeDefs, callArity); if (arityMatch !== undefined) return arityMatch; } + // When arity narrowing left >1 candidate, try overload narrowing with + // argument types + conversion ranking (#1578). This picks the unique + // best-rank candidate when exact-type or conversion-rank scoring can + // disambiguate (e.g., `f(int)` vs `f(double)` called with `f(2.5)`). + if (scopeDefs.length > 1) { + const narrowed = narrowOverloadCandidates(scopeDefs, callArity, callArgTypes, conversionRankFn); + if (narrowed.length === 1) return narrowed[0]; + } const defs: SymbolDefinition[] = []; const seen = new Set(); @@ -289,6 +371,11 @@ function pickUniqueGlobalCallable( const arityMatch = narrowByArity(defs, callArity); if (arityMatch !== undefined) return arityMatch; } + // Same argument-type + conversion-rank narrowing for the model pool. + if (defs.length > 1) { + const narrowed = narrowOverloadCandidates(defs, callArity, callArgTypes, conversionRankFn); + if (narrowed.length === 1) return narrowed[0]; + } return undefined; } @@ -362,6 +449,7 @@ export function pickImplicitThisOverload( scopes: ScopeResolutionIndexes, workspaceIndex: WorkspaceResolutionIndex, model: SemanticModel, + conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { // Find the enclosing Class scope by walking parents. let curId: ScopeId | null = site.inScope; @@ -389,7 +477,12 @@ export function pickImplicitThisOverload( // ambiguous narrowing (multiple compatible candidates with no // disambiguating signal) leaves the call unresolved rather than // routing to an arbitrary first overload by registration order. - const candidates = narrowOverloadCandidates(overloads, site.arity, site.argumentTypes); + const candidates = narrowOverloadCandidates( + overloads, + site.arity, + site.argumentTypes, + conversionRankFn, + ); if (candidates.length !== 1) return undefined; return candidates[0]; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts index bff16d27e..5c9338f40 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts @@ -24,15 +24,35 @@ * equality. An empty string in `argTypes[i]` means "unknown" and * counts as a match. Mismatches disqualify. A non-empty typed * result wins; otherwise return the arity-filtered candidates. + * 4b. When the exact-type filter from step 4 returns empty AND a + * `conversionRankFn` is provided, rank candidates via pairwise + * dominance comparison (ISO C++ [over.ics.rank]): F1 beats F2 + * only when F1 is not worse for every arg and better for at + * least one. Non-dominated candidates are returned; multiple + * survivors are genuinely ambiguous. * 5. Empty input returns empty output. */ import type { SymbolDefinition } from 'gitnexus-shared'; +/** + * Per-slot conversion-rank function. Returns a numeric cost for + * converting `argType` to `paramType`: + * - 0 = exact match (no conversion) + * - 1 = promotion (e.g. char→int, bool→int in C++) + * - 2 = standard conversion (e.g. int→double) + * - Infinity = incompatible types + * + * Each language provides its own implementation. The function operates + * on normalized type strings (output of the language's type normalizer). + */ +export type ConversionRankFn = (argType: string, paramType: string) => number; + export function narrowOverloadCandidates( overloads: readonly SymbolDefinition[], argCount: number | undefined, argTypes: readonly string[] | undefined, + conversionRankFn?: ConversionRankFn, ): readonly SymbolDefinition[] { if (overloads.length === 0) return []; @@ -84,11 +104,98 @@ export function narrowOverloadCandidates( return true; }); if (typed.length > 0) return typed; + + // ── Conversion-rank scoring (step 4b) ────────────────────────── + // The exact-type filter above rejected every candidate. When a + // per-language conversion-rank function is available, rank via + // pairwise dominance: F1 beats F2 only when F1 is not worse for + // every arg and better for at least one. Non-dominated candidates + // are returned; multiple survivors are genuinely ambiguous. + if (conversionRankFn !== undefined) { + const ranked = rankByConversion(candidates, argTypes, conversionRankFn); + if (ranked.length > 0) return ranked; + } } return candidates; } +/** + * Pairwise dominance comparison (ISO C++ [over.ics.rank]). + * + * F1 is a better match than F2 when F1's conversion rank is **not + * worse** for every argument AND **strictly better** for at least one. + * Candidates dominated by any other viable candidate are removed. + * If more than one non-dominated candidate remains, they are genuinely + * ambiguous — callers suppress the edge rather than picking arbitrarily. + * + * Candidates with at least one `Infinity`-ranked slot (incompatible + * type) are excluded before pairwise comparison begins. + */ +function rankByConversion( + candidates: readonly SymbolDefinition[], + argTypes: readonly string[], + rankFn: ConversionRankFn, +): readonly SymbolDefinition[] { + // Step 1: compute per-slot ranks and exclude non-viable candidates. + const viable: Array<{ def: SymbolDefinition; ranks: number[] }> = []; + for (const d of candidates) { + const params = d.parameterTypes; + if (params === undefined) continue; + const ranks: number[] = []; + let ok = true; + for (let i = 0; i < argTypes.length && i < params.length; i++) { + if (argTypes[i] === '') { + ranks.push(0); // unknown arg → any-match (rank 0) + continue; + } + const r = rankFn(argTypes[i], params[i]); + if (!isFinite(r)) { + ok = false; + break; + } + ranks.push(r); + } + if (!ok) continue; + viable.push({ def: d, ranks }); + } + if (viable.length <= 1) return viable.map((v) => v.def); + + // Step 2: pairwise dominance — remove candidates dominated by any other. + const dominated = new Set(); + for (let i = 0; i < viable.length; i++) { + if (dominated.has(i)) continue; + for (let j = i + 1; j < viable.length; j++) { + if (dominated.has(j)) continue; + const cmp = pairwiseCompare(viable[i].ranks, viable[j].ranks); + if (cmp < 0) + dominated.add(j); // i dominates j + else if (cmp > 0) dominated.add(i); // j dominates i + } + } + return viable.filter((_, idx) => !dominated.has(idx)).map((v) => v.def); +} + +/** + * Compare two per-slot rank vectors. + * Returns -1 if `a` dominates `b` (not worse everywhere, better somewhere), + * +1 if `b` dominates `a`, + * 0 if neither dominates (incomparable or equal). + */ +function pairwiseCompare(a: readonly number[], b: readonly number[]): -1 | 0 | 1 { + let aBetter = false; + let bBetter = false; + const len = Math.min(a.length, b.length); + for (let i = 0; i < len; i++) { + if (a[i] < b[i]) aBetter = true; + else if (b[i] < a[i]) bBetter = true; + if (aBetter && bBetter) return 0; // incomparable — early exit + } + if (aBetter && !bBetter) return -1; + if (bBetter && !aBetter) return 1; + return 0; +} + /** * Detect when >1 candidate share identical `parameterTypes` after the * per-language normalizer has collapsed distinct underlying types. This diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 92b4c1ac1..ffc29d176 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -73,6 +73,7 @@ type ReceiverBoundProviderSubset = Pick< | 'hoistTypeBindingsToModule' | 'resolveQualifiedReceiverMember' | 'resolveThisViaEnclosingClass' + | 'conversionRankFn' >; function normalizeTemplateArgToken(value: string): string { @@ -343,6 +344,7 @@ export function emitReceiverBoundCalls( methodOverloads, site.arity, site.argumentTypes, + provider.conversionRankFn, ); if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { ambiguous = true; @@ -356,6 +358,12 @@ export function emitReceiverBoundCalls( hiddenByName = true; break; } + // Multiple tied survivors with distinct param types (e.g. + // h(int,double) vs h(double,int) both scoring 2) → ambiguous. + if (narrowed.length > 1) { + ambiguous = true; + break; + } memberDef = narrowed[0] ?? methodOverloads[0]; break; } @@ -640,7 +648,13 @@ export function emitReceiverBoundCalls( let memberDef: SymbolDefinition | undefined; let ambiguous = false; for (const ownerId of chain) { - const picked = pickOverload(ownerId, memberName, site, model); + const picked = pickOverload( + ownerId, + memberName, + site, + model, + provider.conversionRankFn, + ); if (picked === OVERLOAD_AMBIGUOUS) { ambiguous = true; break; @@ -708,6 +722,7 @@ function pickOverload( memberName: string, site: ParsedFile['referenceSites'][number], model: SemanticModel, + conversionRankFn?: (argType: string, paramType: string) => number, ): SymbolDefinition | typeof OVERLOAD_AMBIGUOUS | undefined { const overloads = model.methods.lookupAllByOwner(ownerId, memberName); if (overloads.length === 0) { @@ -718,7 +733,12 @@ function pickOverload( } if (overloads.length === 1) return overloads[0]; - const candidates = narrowOverloadCandidates(overloads, site.arity, site.argumentTypes); + const candidates = narrowOverloadCandidates( + overloads, + site.arity, + site.argumentTypes, + conversionRankFn, + ); // When narrowing leaves >1 candidate that share identical normalized // parameter-types (e.g., C++ `f(int)` vs `f(long)` both collapsed to // `['int']` by `normalizeCppParamType`), suppress the edge entirely. @@ -726,6 +746,11 @@ function pickOverload( // would arbitrarily pick a candidate and lie about the call's target. // PR #1520 review follow-up plan U2 / Claude review Finding 5. if (isOverloadAmbiguousAfterNormalization(candidates, site.arity)) return OVERLOAD_AMBIGUOUS; + // When conversion-rank scoring leaves >1 tied candidate with distinct + // parameter types (e.g. h(int,double) vs h(double,int) both scoring 2), + // suppress rather than picking arbitrarily — C++ would call this + // ambiguous. Mirrors ADL merged-candidate suppression behavior. + if (candidates.length > 1) return OVERLOAD_AMBIGUOUS; return candidates[0] ?? overloads[0]; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index 0809d59ca..5493368da 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -382,6 +382,7 @@ export function runScopeResolution( isFileLocalDef: provider.isFileLocalDef, isCallableVisibleFromCaller: provider.isCallableVisibleFromCaller, resolveAdlCandidates: provider.resolveAdlCandidates, + conversionRankFn: provider.conversionRankFn, }, ); const { emitted, skipped } = emitReferencesViaLookup( diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp new file mode 100644 index 000000000..cd9c51210 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp @@ -0,0 +1,10 @@ +#include "lib.h" + +void Service::f(int x) {} +void Service::f(double x) {} +void Service::g(int x) {} +void Service::g(long x) {} +void Service::h(int a, int b) {} +void Service::h(double a, double b) {} +void Service::p(int x) {} +void Service::p(double x) {} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h new file mode 100644 index 000000000..5a367ba4b --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h @@ -0,0 +1,33 @@ +#pragma once + +class Service { +public: + // Variant 1 & 3: f(int) vs f(double) + void f(int x); + void f(double x); + + // Variant 2: g(int) vs g(long) — both normalize to 'int' + void g(int x); + void g(long x); + + // Variant 4: multi-arg tied total score + void h(int a, int b); + void h(double a, double b); + + // Variant 5: char-literal promotion (exercises conversion ranker) + void p(int x); + void p(double x); + + // Inline: call sites live inside the class scope so the scope-chain + // walk finds the Class scope, enabling pickImplicitThisOverload to + // resolve overloads against the declaration-side Method nodes (which + // carry distinct parameterTypes and graph-node IDs). + void run() { + f(2.5); // Variant 1: double literal -> f(double) wins (exact > standard) + f(42); // Variant 3: int literal -> f(int) wins (exact > standard) + g(42); // Variant 2: int/long both normalize to 'int' -> ambiguous + h(42, 2.5); // Variant 4: incomparable — neither dominates the other -> ambiguous + h('a', 2.5);// Variant 6: asymmetric — h(int,int) better at arg0 (promotion), h(double,double) better at arg1 (exact) -> ambiguous + p('a'); // Variant 5: char literal -> p(int) wins via promotion (rank 1 < rank 2) + } +}; diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index de0ad9632..8e2eaf37f 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1762,6 +1762,80 @@ describe('C++ ambiguous integer-width overloads', () => { }); }); +// --------------------------------------------------------------------------- +// C++ overload resolution: standard-conversion-sequence ranking (#1578) +// Disambiguates overloads when exact normalized-type matching cannot, +// by scoring each candidate's conversion cost. Exact match (rank 0) wins +// over standard conversion (rank 2); same-rank ties still suppress. +// --------------------------------------------------------------------------- + +describe('C++ overload resolution — conversion-rank disambiguation (#1578)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-overload-conversion-rank'), + () => {}, + ); + }, 60000); + + it('f(2.5) resolves to f(double) — exact match beats standard conversion', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'run' && c.target === 'f'); + // Conversion-rank scoring picks f(double) as the unique best: + // f(double) is exact match (rank 0), f(int) is standard conversion (rank 2). + const fDoubleEdges = fCalls.filter((c) => { + const tgt = result.graph.getNode(c.rel.targetId); + return tgt?.properties.parameterTypes?.[0] === 'double'; + }); + expect(fDoubleEdges.length).toBe(1); + }); + + it('f(42) resolves to f(int) — exact match beats standard conversion', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'run' && c.target === 'f'); + // f(int) is exact match (rank 0), f(double) is standard conversion (rank 2). + const fIntEdges = fCalls.filter((c) => { + const tgt = result.graph.getNode(c.rel.targetId); + return tgt?.properties.parameterTypes?.[0] === 'int'; + }); + expect(fIntEdges.length).toBe(1); + }); + + it('g(42) emits zero CALLS edges — int/long normalize to same type, ambiguous', () => { + const calls = getRelationships(result, 'CALLS'); + const gCalls = calls.filter((c) => c.source === 'run' && c.target === 'g'); + // g(int) and g(long) both normalize to parameterTypes=['int'], + // so isOverloadAmbiguousAfterNormalization triggers suppression. + expect(gCalls.length).toBe(0); + }); + + it("p('a') resolves to p(int) — char promotion (rank 1) beats char→double conversion (rank 2)", () => { + const calls = getRelationships(result, 'CALLS'); + const pCalls = calls.filter((c) => c.source === 'run' && c.target === 'p'); + // p('a'): argType='char'. Exact-type filter misses both p(int) and + // p(double), forcing the conversion ranker (step 4b). char→int is an + // integral promotion (rank 1), char→double is a standard conversion + // (rank 2). p(int) wins with the lower total cost. + expect(pCalls.length).toBe(1); + const tgt = result.graph.getNode(pCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes?.[0]).toBe('int'); + }); + + it('h(42, 2.5) emits zero CALLS edges — incomparable multi-arg overloads, ambiguous', () => { + const calls = getRelationships(result, 'CALLS'); + const hCalls = calls.filter((c) => c.source === 'run' && c.target === 'h'); + // h(42, 2.5) + h('a', 2.5): both call sites produce incomparable + // pairwise rankings. For h(42, 2.5) with argTypes=['int','double']: + // h(int,int): [rank('int','int')=0, rank('double','int')=2] + // h(double,double): [rank('int','double')=2, rank('double','double')=0] + // h(int,int) better at arg0, h(double,double) better at arg1 → neither + // dominates → ambiguous. Same pattern for h('a',2.5). + // Contract: zero edges for ALL h() call sites combined (dedup). + expect(hCalls.length).toBe(0); + }); +}); + // --------------------------------------------------------------------------- // U3: anonymous-namespace symbols MUST NOT leak across translation units // (full-pipeline integration test; unit-level coverage exists separately) diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index ae2a75f04..bc18b0374 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -175,6 +175,19 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly::g_unqualified() -> f() does NOT bind to Base::f', 'Derived::g_this() -> this->f() resolves to Base::f (1 edge)', 'Derived::g() -> this->f() emits zero CALLS edges when only hidden derived overload is arity-incompatible', + // Conversion-rank scoring (#1578) disambiguates `f(int)` vs `f(double)` + // by ranking exact match over standard conversion. The legacy DAG has no + // conversion-rank scoring; it either picks arbitrarily or leaves the call + // unresolved. Scope-resolver-only correctness win. + 'f(2.5) resolves to f(double) — exact match beats standard conversion', + 'f(42) resolves to f(int) — exact match beats standard conversion', + 'g(42) emits zero CALLS edges — int/long normalize to same type, ambiguous', + // char-literal promotion exercises the conversion ranker (step 4b). + // Legacy DAG has no conversion-rank scoring. Scope-resolver-only. + "p('a') resolves to p(int) — char promotion (rank 1) beats char→double conversion (rank 2)", + // Multi-arg incomparable overloads: pairwise dominance check finds + // neither h(int,int) nor h(double,double) dominates. Scope-resolver-only. + 'h(42, 2.5) emits zero CALLS edges — incomparable multi-arg overloads, ambiguous', // The legacy DAG path has no inline-namespace same-name ambiguity // detection. When two inline children declare the same name, the // legacy path picks an arbitrary match. The scope-resolver returns diff --git a/gitnexus/test/unit/cross-file-impl.test.ts b/gitnexus/test/unit/cross-file-impl.test.ts index 55a47a5b5..bf5241d1d 100644 --- a/gitnexus/test/unit/cross-file-impl.test.ts +++ b/gitnexus/test/unit/cross-file-impl.test.ts @@ -49,17 +49,36 @@ 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', () => ({ + 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 () => { @@ -162,6 +181,103 @@ describe('runCrossFileBindingPropagation', () => { } }); + 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(); @@ -203,4 +319,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(); + }); });