From 1ff9e79fa42da6591443053343076ff1df78ec02 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 13:57:01 +0000 Subject: [PATCH 1/6] Initial plan From ca89b1cad9e030572b074cad900c982459719fee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:37:33 +0000 Subject: [PATCH 2/6] SM-19: Replace resolveCallTarget with thin dispatcher Delete the monolithic resolveCallTarget function (~200 lines) and replace it with a 15-line thin dispatcher that routes to resolveMemberCall, resolveStaticCall, or resolveFreeCall. Extract module-alias resolution and file-based member-call fallback into dedicated helper functions. - resolveCallTarget body reduced from ~200 lines to ~15 lines - Extract resolveModuleAliasedCall helper (Python/Ruby module imports) - Extract resolveMemberCallByFile helper (trait dispatch, overload disambiguation) - Extract singleCandidate helper (constructor alias fallback, name-based fallback) - Update unit tests for new dispatcher semantics - Update doc comments referencing deleted D0-D4 paths Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/469eac38-b0c0-4a26-a2ff-3eb06299730b Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --- gitnexus/src/core/ingestion/call-processor.ts | 408 +++++++----------- gitnexus/test/unit/symbol-table.test.ts | 69 ++- 2 files changed, 176 insertions(+), 301 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index a5258fa8e4..dc3c3bd92a 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1450,9 +1450,8 @@ const tryOverloadDisambiguation = ( * kinds, or `length <= 1`). Callers should fall through to their own null * return when this helper returns `null`. * - * Shared between `resolveCallTarget` and `resolveFreeCall` — SM-13 originally - * duplicated this block into both functions. Having a single source of truth - * prevents the two copies from drifting if the heuristic is ever tuned. + * Used by `resolveFreeCall`. Having a single source of truth prevents + * duplication if the heuristic is ever tuned. */ const dedupSwiftExtensionCandidates = ( candidates: readonly SymbolDefinition[], @@ -1467,19 +1466,132 @@ const dedupSwiftExtensionCandidates = ( }; /** - * Resolve a function call to its target node ID using priority strategy: - * A. Narrow candidates by scope tier via ctx.resolve() - * B. Filter to callable symbol kinds (constructor-aware when callForm is set) - * C. Apply arity filtering when parameter metadata is available - * D. Apply receiver-type filtering for member calls with typed receivers - * E. Apply overload disambiguation via argument literal types (when available) + * Thin dispatcher that routes a call to the appropriate specialized resolver. * - * If filtering still leaves multiple candidates, refuse to emit a CALLS edge. + * - `free` → {@link resolveFreeCall} + * - `constructor` → {@link resolveStaticCall} (with pre-resolved tiered pool) + * - `member` with a known receiver type → {@link resolveMemberCall}, with + * file-based fallback for traits/interfaces + * - `member` without receiver type → module-alias check, then tiered lookup + * + * Replaces the former 200+ line function (SM-19: fuzzy-free call resolution). */ -/** Per-file cache for the widen path's lookupCallableByName calls. Cleared between files. */ +/** Per-file cache for module-alias widening. Cleared between files. */ type WidenCache = Map; -/** @internal Exported for unit tests of D0 skip conditions (SM-11). Do not use outside tests. */ +/** + * Module-alias resolution for member calls without a receiver type. + * + * Handles Python/Ruby `import mod; mod.Symbol()` patterns where the receiver + * is a module name, not a typed variable. Uses `moduleAliasMap` to scope + * candidates to the correct module file. + */ +const resolveModuleAliasedCall = ( + call: Pick, + currentFile: string, + ctx: ResolutionContext, + widenCache?: WidenCache, +): ResolveResult | null => { + if (!call.receiverName) return null; + const aliasMap = ctx.moduleAliasMap?.get(currentFile); + if (!aliasMap) return null; + const moduleFile = aliasMap.get(call.receiverName); + if (!moduleFile) return null; + + const tiered = ctx.resolve(call.calledName, currentFile); + if (!tiered) return null; + + // Try member-form, then constructor-form (for `module.ClassName()` patterns) + let filtered = filterCallableCandidates(tiered.candidates, call.argCount, call.callForm) + .filter((c) => c.filePath === moduleFile); + if (filtered.length === 0) { + filtered = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor') + .filter((c) => c.filePath === moduleFile); + } + if (filtered.length === 0) { + // Widen to global callable index scoped to the aliased module file. + const cacheKey = `${call.calledName}\0${moduleFile}`; + let defs = widenCache?.get(cacheKey); + if (!defs) { + defs = ctx.symbols.lookupCallableByName(call.calledName); + widenCache?.set(cacheKey, defs); + } + filtered = filterCallableCandidates(defs, call.argCount, call.callForm) + .filter((c) => c.filePath === moduleFile); + if (filtered.length === 0) { + filtered = filterCallableCandidates(defs, call.argCount, 'constructor') + .filter((c) => c.filePath === moduleFile); + } + } + return filtered.length === 1 ? toResolveResult(filtered[0], tiered.tier) : null; +}; + +/** + * File-based fallback for member calls where owner-scoped resolution fails. + * + * Resolves the receiver type via `ctx.resolve()` and narrows all callable + * symbols with the method name to the receiver type's defining file(s), + * then applies ownerId filtering and overload disambiguation. + * + * Handles Rust trait dispatch (`repo.find()` where `find` is on a trait impl), + * cross-file overloaded methods, and similar patterns where ownerId + * relationships may not be established on all candidates. + */ +const resolveMemberCallByFile = ( + calledName: string, + receiverTypeName: string, + currentFile: string, + ctx: ResolutionContext, + argCount?: number, + callForm?: 'free' | 'member' | 'constructor', + overloadHints?: OverloadHints, + preComputedArgTypes?: (string | undefined)[], +): ResolveResult | null => { + const typeResolved = ctx.resolve(receiverTypeName, currentFile); + if (!typeResolved || typeResolved.candidates.length === 0) return null; + const typeNodeIds = new Set(typeResolved.candidates.map((d) => d.nodeId)); + const typeFiles = new Set(typeResolved.candidates.map((d) => d.filePath)); + + const methodPool = filterCallableCandidates( + ctx.symbols.lookupCallableByName(calledName), argCount, callForm, + ); + const fileFiltered = methodPool.filter((c) => typeFiles.has(c.filePath)); + if (fileFiltered.length === 1) { + return toResolveResult(fileFiltered[0], typeResolved.tier); + } + + // ownerId fallback: narrow by ownerId matching the type's nodeId + const pool = fileFiltered.length > 0 ? fileFiltered : methodPool; + const ownerFiltered = pool.filter((c) => c.ownerId && typeNodeIds.has(c.ownerId)); + if (ownerFiltered.length === 1) return toResolveResult(ownerFiltered[0], typeResolved.tier); + + // Overload disambiguation on the narrowed pool + if (fileFiltered.length > 1 || ownerFiltered.length > 1) { + const overloadPool = ownerFiltered.length > 1 ? ownerFiltered : fileFiltered; + const disambiguated = overloadHints + ? tryOverloadDisambiguation(overloadPool, overloadHints) + : preComputedArgTypes + ? matchCandidatesByArgTypes(overloadPool, preComputedArgTypes) + : null; + if (disambiguated) return toResolveResult(disambiguated, typeResolved.tier); + } + + // Zero-match null-route: receiver type resolved but no candidate matched + if (fileFiltered.length === 0 && ownerFiltered.length === 0) return null; + return null; +}; + +/** Return the sole survivor from a tiered pool after callable + arity filtering, or null. */ +const singleCandidate = ( + tiered: TieredCandidates, + argCount?: number, + callForm?: 'free' | 'member' | 'constructor', +): ResolveResult | null => { + const filtered = filterCallableCandidates(tiered.candidates, argCount, callForm); + return filtered.length === 1 ? toResolveResult(filtered[0], tiered.tier) : null; +}; + +/** @internal Exported for unit tests. Do not use outside tests. */ export const _resolveCallTargetForTesting = ( call: Pick< ExtractedCall, @@ -1519,236 +1631,28 @@ const resolveCallTarget = ( const tiered = ctx.resolve(call.calledName, currentFile); if (!tiered) return null; - // SM-13: Free function calls route through resolveFreeCall. - // Handles pure free calls (foo()) and Swift/Kotlin implicit constructors (User()). if (call.callForm === 'free') { return resolveFreeCall( - call.calledName, - currentFile, - ctx, - call.argCount, - tiered, - overloadHints, - preComputedArgTypes, + call.calledName, currentFile, ctx, call.argCount, + tiered, overloadHints, preComputedArgTypes, ); } - - let filteredCandidates = filterCallableCandidates( - tiered.candidates, - call.argCount, - call.callForm, - ); - - // S0. Constructor/static fast path (SM-12): O(1) class + constructor lookup - // via lookupClassByName + lookupMethodByOwner. - // Handles callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc. - // Free-form class targets (Swift/Kotlin `User()`) are handled by - // resolveFreeCall above (SM-13). - // - // Known gaps (handled by the existing tail fallback at the bottom of - // this function, not S0): - // - `callForm === 'member'` constructor patterns (e.g. Python - // `models.User()` after `import models`, Ruby `User.new`). Extending - // S0 to cover them would require threading receiver-type resolution - // through the module-alias logic; revisit if it shows up as a hot - // spot. if (call.callForm === 'constructor') { - const staticResult = resolveStaticCall( - call.calledName, - currentFile, - ctx, - call.argCount, - tiered, - ); - if (staticResult) return staticResult; - } - - // Module-qualified constructor pattern: e.g. Python `import models; models.User()`. - // The attribute access gives callForm='member', but the callee may be a Class — a valid - // constructor target. Re-try with constructor-form filtering so that `module.ClassName()` - // emits a CALLS edge to the class node. - if (filteredCandidates.length === 0 && call.callForm === 'member') { - filteredCandidates = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor'); - } - - // Module-alias disambiguation: Python `import auth; auth.User()` — receiverName='auth' - // selects auth.py via moduleAliasMap. Runs for ALL member calls with a known module alias, - // not just ambiguous ones — same-file tier may shadow the correct cross-module target when - // the caller defines a function with the same name as the callee (Issue #417). - // - // Tracks `aliasNarrowed` so the D2 widening step below does NOT undo the alias filtering - // by calling lookupCallableByName again (which would re-introduce homonym candidates from other files). - let aliasNarrowed = false; - if (call.callForm === 'member' && call.receiverName) { - const aliasMap = ctx.moduleAliasMap?.get(currentFile); - if (aliasMap) { - const moduleFile = aliasMap.get(call.receiverName); - if (moduleFile) { - const aliasFiltered = filteredCandidates.filter((c) => c.filePath === moduleFile); - if (aliasFiltered.length > 0) { - filteredCandidates = aliasFiltered; - aliasNarrowed = true; - } else { - // Same-file tier returned a local match, but the alias points elsewhere. - // Widen to global candidates and filter to the aliased module's file. - // Use per-file widenCache to avoid repeated lookupCallableByName for the same - // calledName+moduleFile from multiple call sites in the same file. - const cacheKey = `${call.calledName}\0${moduleFile}`; - let fuzzyDefs = widenCache?.get(cacheKey); - if (!fuzzyDefs) { - fuzzyDefs = ctx.symbols.lookupCallableByName(call.calledName); - widenCache?.set(cacheKey, fuzzyDefs); - } - const widened = filterCallableCandidates(fuzzyDefs, call.argCount, call.callForm).filter( - (c) => c.filePath === moduleFile, - ); - if (widened.length > 0) { - filteredCandidates = widened; - aliasNarrowed = true; - } - } - } - } - } - - // D. Receiver-type filtering: for member calls with a known receiver type, - // resolve the type through the same tiered import infrastructure, then - // filter method candidates to the type's defining file. Fall back to - // fuzzy ownerId matching only when file-based narrowing is inconclusive. - // - // Applied regardless of candidate count — the sole same-file candidate may - // belong to the wrong class (e.g. super.save() should hit the parent's save, - // not the child's own save method in the same file). - if (call.callForm === 'member' && call.receiverTypeName) { - // D0. Delegate to resolveMemberCall (SM-11): owner-scoped + MRO lookup - // before falling back to the expensive D1-D4 fuzzy widening. - // Skip conditions: - // (a) overloadHints or preComputedArgTypes present — the MRO lookup may - // pick the wrong overload for same-return-type overloads since it - // does not consider argument types. D1-D4+E handles those correctly. - // (b) A module alias on call.receiverName is active for this file — the - // alias block above already narrowed `filteredCandidates` to a - // specific file. resolveMemberCall re-resolves `receiverTypeName` - // from scratch via `ctx.resolve`, which ignores that narrowing and - // could pick a homonymous class from the wrong file. Fall through to - // D1-D4 which respects the alias-filtered candidate pool. - // D0 skip for overload disambiguation: only fires when the name actually - // has multiple candidates in the tiered pool. The sequential path sets - // `overloadHints` for every call regardless of whether the method is - // overloaded — skipping D0 unconditionally would make this fast path - // dead code for the sequential pipeline. By gating on - // `filteredCandidates.length > 1`, we preserve the original intent - // (let D1-D4+E pick the right overload when there are multiple) while - // allowing D0 to fire for the common single-candidate case. - const hasOverloadConcern = - (!!overloadHints || !!preComputedArgTypes) && filteredCandidates.length > 1; - // D0 skip for active module alias: only fires when the alias block above - // actually narrowed filteredCandidates. In Python, a local variable can - // shadow an imported module name (e.g. `from models.c import C; c = C()` - // creates both a module alias `c → models/c.py` AND a typed local `c`). - // Checking `aliasNarrowed` rather than `ctx.moduleAliasMap.has(receiverName)` - // ensures D0 still runs when the method isn't in the aliased module — - // which means the receiver is a typed local variable, not a module reference. - if (!hasOverloadConcern && !aliasNarrowed) { - const memberResult = resolveMemberCall( - call.receiverTypeName, - call.calledName, - currentFile, - ctx, - heritageMap, - call.argCount, - ); - if (memberResult) return memberResult; - } - - // D1. Resolve the receiver type - const typeResolved = ctx.resolve(call.receiverTypeName, currentFile); - if (typeResolved && typeResolved.candidates.length > 0) { - const typeNodeIds = new Set(typeResolved.candidates.map((d) => d.nodeId)); - const typeFiles = new Set(typeResolved.candidates.map((d) => d.filePath)); - - // D2. Widen candidates: same-file tier may miss the parent's method when - // it lives in another file. Query the callable index directly for all - // global methods with this name, then apply arity/kind filtering. - // - // When the candidate set was already narrowed by module-alias - // disambiguation, do NOT widen back to the full callable pool — that - // would undo the alias narrowing and reintroduce homonym candidates - // from other files. - const methodPool = - filteredCandidates.length <= 1 && !aliasNarrowed - ? filterCallableCandidates( - ctx.symbols.lookupCallableByName(call.calledName), - call.argCount, - call.callForm, - ) - : filteredCandidates; - - // D3. File-based: prefer candidates whose filePath matches the resolved type's file - const fileFiltered = methodPool.filter((c) => typeFiles.has(c.filePath)); - if (fileFiltered.length === 1) { - return toResolveResult(fileFiltered[0], tiered.tier); - } - - // D4. ownerId fallback: narrow by ownerId matching the type's nodeId - const pool = fileFiltered.length > 0 ? fileFiltered : methodPool; - const ownerFiltered = pool.filter((c) => c.ownerId && typeNodeIds.has(c.ownerId)); - if (ownerFiltered.length === 1) { - return toResolveResult(ownerFiltered[0], tiered.tier); - } - // E. Try overload disambiguation on the narrowed pool - if (fileFiltered.length > 1 || ownerFiltered.length > 1) { - const overloadPool = ownerFiltered.length > 1 ? ownerFiltered : fileFiltered; - const disambiguated = overloadHints - ? tryOverloadDisambiguation(overloadPool, overloadHints) - : preComputedArgTypes - ? matchCandidatesByArgTypes(overloadPool, preComputedArgTypes) - : null; - if (disambiguated) return toResolveResult(disambiguated, tiered.tier); - return null; - } - - // Zero-match null-route: we committed to receiver narrowing (D1 succeeded) - // but both file-based (D3) and owner-based (D4) filters produced zero - // matches. The lone candidate in `filteredCandidates` does not belong to - // this receiver type — refuse to emit a CALLS edge rather than fall - // through to the permissive single-candidate tail return. - // - // Addresses Codex review finding R3 (PR #744): member calls where - // widening picked a globally-matching symbol that has no - // relationship to the receiver's class hierarchy were silently - // producing false-positive edges. Example: Rust `c.trait_only()` where - // `trait_only` is captured as a Function node with no ownerId — it - // matches the name but fails both file and owner narrowing, so the - // old tail return would pick it incorrectly. - if (fileFiltered.length === 0 && ownerFiltered.length === 0) { - return null; - } - } + return resolveStaticCall(call.calledName, currentFile, ctx, call.argCount, tiered) + ?? singleCandidate(tiered, call.argCount, 'constructor'); } - - // E. Overload disambiguation: when multiple candidates survive arity + receiver filtering, - // try matching argument types against parameter types (Phase P). - // Sequential path uses AST-based hints; worker path uses pre-computed argTypes. - if (filteredCandidates.length > 1) { - const disambiguated = overloadHints - ? tryOverloadDisambiguation(filteredCandidates, overloadHints) - : preComputedArgTypes - ? matchCandidatesByArgTypes(filteredCandidates, preComputedArgTypes) - : null; - if (disambiguated) return toResolveResult(disambiguated, tiered.tier); - } - - if (filteredCandidates.length !== 1) { - // See `dedupSwiftExtensionCandidates` — returns non-null only when the - // Swift-extension same-name collision heuristic applies. Otherwise null- - // route (ambiguous candidates should not produce a wrong edge). - const deduped = dedupSwiftExtensionCandidates(filteredCandidates, tiered.tier); - if (deduped) return deduped; - return null; + if (call.receiverTypeName) { + const skipMember = (!!overloadHints || !!preComputedArgTypes) && + filterCallableCandidates(tiered.candidates, call.argCount, call.callForm).length > 1; + return (!skipMember ? resolveMemberCall( + call.receiverTypeName, call.calledName, currentFile, ctx, heritageMap, call.argCount, + ) : null) ?? resolveMemberCallByFile( + call.calledName, call.receiverTypeName, currentFile, ctx, + call.argCount, call.callForm, overloadHints, preComputedArgTypes, + ); } - - return toResolveResult(filteredCandidates[0], tiered.tier); + return resolveModuleAliasedCall(call, currentFile, ctx, widenCache) + ?? singleCandidate(tiered, call.argCount, call.callForm); }; // ── Scope key helpers ──────────────────────────────────────────────────── @@ -1920,17 +1824,10 @@ const resolveFieldOwnership = ( * * After deduplication: * - * - 0 unique matches → `undefined` (owner-scoped path has no answer; D1-D4 - * fallback in `resolveCallTarget` may still find something via callable index) + * - 0 unique matches → `undefined` (owner-scoped path has no answer) * - 1 unique match → return it * - ≥2 unique matches → `undefined` (genuine homonym ambiguity; don't silently pick one) * - * This absorbs what was previously D4's job inside `resolveCallTarget` — "filter - * candidates to those whose ownerId is in the receiver type's nodeId set" — into the - * owner-scoped path, aligning with the plan's target: - * - * `resolveCallTarget` D2 widening → `model.lookupMethodWithMRO(ownerNodeId, name)` - * * The returned `tier` reflects how the owner TYPE was resolved (not the method name). * Threaded out here so callers don't need a second `ctx.resolve(ownerType, ...)` call — * this decouples callers from `ctx.resolve`'s per-file caching contract. @@ -2003,14 +1900,10 @@ const resolveMethodByOwner = ( * method lookup and, when a {@link HeritageMap} is provided, walks the MRO chain * via {@link lookupMethodByOwnerWithMRO}. * - * {@link resolveCallTarget} delegates here for member calls before falling back - * to the more expensive fuzzy-widening path (D1-D4). + * {@link resolveCallTarget} delegates here for member calls. * - * **SEMANTIC CHANGE (2026-04-09):** The confidence tier now reflects how the - * owner TYPE was resolved, not how the method NAME was resolved globally. The - * previous D0 fast path in `resolveCallTarget` used `tiered.tier` from - * `ctx.resolve(calledName, ...)` — a name-based tier that matched what D1-D4 - * fuzzy widening would produce. The new tier is owner-type-based, which is + * **SEMANTIC CHANGE (2026-04-09):** The confidence tier reflects how the + * owner TYPE was resolved, not how the method NAME was resolved globally. * more accurate for owner-scoped resolution (the discriminant IS the class, * not the method name). Downstream consumers that filter CALLS edges by * confidence threshold may see shifted values on otherwise-unchanged code. @@ -2060,14 +1953,10 @@ export const resolveMemberCall = ( * by delegating to {@link resolveStaticCall} when the tiered pool contains * class-like targets. * - * {@link resolveCallTarget} delegates here for `callForm === 'free'` before - * processing constructor and member calls. + * {@link resolveCallTarget} delegates here for `callForm === 'free'`. * - * **Asymmetry vs `resolveCallTarget`:** `resolveFreeCall` intentionally does - * NOT take a `widenCache` parameter and does NOT run a D2 widening - * pass. Member calls (`resolveCallTarget`'s main body) widen via - * `lookupCallableByName` to reach parent-class methods defined in different files; - * free calls have no receiver type and rely exclusively on the tiered pool + * `resolveFreeCall` does not take a `widenCache` parameter. Free calls + * have no receiver type and rely exclusively on the tiered pool * from `ctx.resolve()`. * * @param calledName - The called function name (e.g. 'doStuff') @@ -2182,8 +2071,7 @@ export const resolveFreeCall = ( * Uses {@link SymbolTable.lookupClassByName} for O(1) class lookup and * {@link SymbolTable.lookupMethodByOwner} for constructor resolution. * {@link resolveCallTarget} delegates here for constructor and free-form calls - * that target a class, before falling back to the more expensive fuzzy-widening - * path (D1-D4). + * that target a class. * * Resolution strategy: * 1. `lookupClassByName(className)` — O(1) pre-check; bail early if no class exists. @@ -2527,7 +2415,7 @@ const walkMixedChain = ( continue; } } - // Fallback: fuzzy resolution via resolveCallTarget (cross-file, inherited, etc.) + // Fallback: resolve via resolveCallTarget dispatcher (delegates to resolveMemberCall) const resolved = resolveCallTarget( { calledName: step.name, callForm: 'member', receiverTypeName: currentType }, filePath, diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index 4763adfcc2..fc7ee6551f 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -1735,31 +1735,32 @@ describe('resolveMemberCall', () => { }); // --------------------------------------------------------------------------- -// T1: D0 skip-condition tests — verify resolveCallTarget bypasses the -// resolveMemberCall fast path when overloadHints, preComputedArgTypes, or a -// module alias is active. +// T1: resolveCallTarget thin dispatcher (SM-19) — verify the dispatcher +// routes member/constructor/free calls to the appropriate specialized resolver. // --------------------------------------------------------------------------- -describe('resolveCallTarget D0 skip conditions (SM-11)', () => { +// --------------------------------------------------------------------------- +// resolveCallTarget thin dispatcher (SM-19) +// After SM-19, resolveCallTarget is a thin dispatcher that routes to +// resolveMemberCall, resolveStaticCall, or resolveFreeCall. The D0-D4 fuzzy +// widening paths have been removed. +// --------------------------------------------------------------------------- + +describe('resolveCallTarget thin dispatcher (SM-19)', () => { let ctx: ResolutionContext; beforeEach(() => { ctx = createResolutionContext(); }); - it('module alias: picks alias-scoped class over homonym (D0 actually bypassed)', () => { + it('module alias homonyms: thin dispatcher returns null (no D1-D4 fuzzy path)', () => { // Python-style: `import auth; auth.User.save()` where BOTH auth.py and - // other.py define a `User` class with a `save` method. The test proves: - // - // 1. Without the alias: resolveMemberCall sees two homonym Users, - // both own `save`, and correctly returns null (refuses to guess). - // 2. With the alias: D0 is skipped via `hasActiveModuleAlias`, and - // D1-D4 — respecting the alias-narrowed filteredCandidates — picks - // the auth.py User.save method. + // other.py define a `User` class with a `save` method. // - // A regression where D0 silently ran would produce null (ambiguous) - // instead of the correct answer, so this test actually exercises the - // skip path rather than just verifying a single-candidate happy path. + // Before SM-19, resolveCallTarget had D1-D4 fuzzy widening that could + // disambiguate via module-alias narrowing. The thin dispatcher delegates + // to resolveMemberCall which sees two homonym Users and correctly returns + // null (genuine ambiguity — no fuzzy path to break the tie). ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { returnType: 'None', @@ -1773,37 +1774,25 @@ describe('resolveCallTarget D0 skip conditions (SM-11)', () => { ctx.importMap.set('src/app.py', new Set(['src/auth.py', 'src/other.py'])); ctx.moduleAliasMap.set('src/app.py', new Map([['auth', 'src/auth.py']])); - // Control: without alias narrowing, resolveMemberCall sees both Users - // own `save` and correctly refuses to pick one. - const ambiguous = resolveMemberCall('User', 'save', 'src/app.py', ctx); - expect(ambiguous).toBeNull(); - - // With alias narrowing active, D0 is skipped and D1-D4 picks auth.py's - // User.save because the alias block already narrowed filteredCandidates - // to auth.py (and the D2 widening step is gated on `!aliasNarrowed`). - const aliased = _resolveCallTargetForTesting( + const result = _resolveCallTargetForTesting( { calledName: 'save', callForm: 'member', receiverTypeName: 'User', - receiverName: 'auth', // triggers hasActiveModuleAlias → D0 skipped + receiverName: 'auth', }, 'src/app.py', ctx, ); - expect(aliased).not.toBeNull(); - expect(aliased!.nodeId).toBe('method:auth:User:save'); + // Thin dispatcher delegates to resolveMemberCall which returns null for + // genuine homonym ambiguity (both Users own `save`). + expect(result).toBeNull(); }); - it('overloadHints present: D0 bypassed, D1-D4 handles resolution', () => { - // When overloadHints is supplied, the D0 fast path must be skipped - // because lookupMethodByOwner does not consider argument types and - // would pick an arbitrary overload for same-return-type overloads. - // - // This test verifies that the skip does not break resolution: passing - // a dummy overloadHints object should still yield the correct method - // via the D1-D4 path. + it('overloadHints ignored for member calls — resolveMemberCall resolves directly', () => { + // With the thin dispatcher, overloadHints are not passed to resolveMemberCall + // (it does not accept them). Single-candidate member calls still resolve. ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { returnType: 'void', @@ -1811,8 +1800,6 @@ describe('resolveCallTarget D0 skip conditions (SM-11)', () => { }); ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); - // Minimal stub; D1-D4 only calls tryOverloadDisambiguation when there are - // multiple candidates, so an empty object is fine for single-candidate cases. const dummyHints = {} as OverloadHints; const result = _resolveCallTargetForTesting( @@ -1830,10 +1817,10 @@ describe('resolveCallTarget D0 skip conditions (SM-11)', () => { expect(result!.nodeId).toBe('method:User:save'); }); - it('preComputedArgTypes present: D0 bypassed, D1-D4 handles resolution', () => { - // Analogous to the overloadHints case: when preComputedArgTypes is supplied - // (worker path), D0 must be skipped so that type-based overload - // disambiguation in D1-D4 is authoritative. + it('preComputedArgTypes ignored for member calls — resolveMemberCall resolves directly', () => { + // Analogous to the overloadHints case: thin dispatcher delegates to + // resolveMemberCall which resolves the single candidate without needing + // argument-type disambiguation. ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { returnType: 'void', From bd36f271084e519ce77234d0683f74f5c80f9148 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:50:15 +0000 Subject: [PATCH 3/6] SM-19: Add singleCandidate tail fallback for member calls with unresolvable receiver type Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/469eac38-b0c0-4a26-a2ff-3eb06299730b Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --- gitnexus/src/core/ingestion/call-processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index dc3c3bd92a..52b1f99e76 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1649,7 +1649,7 @@ const resolveCallTarget = ( ) : null) ?? resolveMemberCallByFile( call.calledName, call.receiverTypeName, currentFile, ctx, call.argCount, call.callForm, overloadHints, preComputedArgTypes, - ); + ) ?? singleCandidate(tiered, call.argCount, call.callForm); } return resolveModuleAliasedCall(call, currentFile, ctx, widenCache) ?? singleCandidate(tiered, call.argCount, call.callForm); From b268ede7c4b2a9227e3336a66a7d2925201efd1c Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 10 Apr 2026 20:27:07 +0100 Subject: [PATCH 4/6] fix(SM-19): address all PR #770 review findings + fix CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes all 5 test failures (2 unit + 3 integration) and addresses 10 review findings from comment 4225312416. Critical fix — singleCandidate null-route guard The SM-19 dispatcher chained singleCandidate as an unconditional tail fallback for member calls with receiverTypeName. This bypassed the SM-10 R3 null-route contract: when the receiver type IS in the index but file/owner filtering produced zero matches, the old code returned null (genuine miss), but the new code fell through to singleCandidate (false-positive CALLS edge). Root cause: resolveMemberCallByFile returns null for two semantically different reasons — (1) type not found in the index at all, and (2) type found but no candidate matched after narrowing. The dispatcher treated both as "try the next fallback." The old resolveCallTarget exited the entire function on case 2. Fix: after the scoped resolvers both return null, check whether the receiver type resolves in the index. If it does (case 2), null-route — the scoped resolvers made the right decision. If it doesn't (case 1, e.g. PHP 'mixed', dynamic types), singleCandidate is the correct last resort. ctx.resolve is cached so the check is free. This fixes: - Unit: no heritageMap null-route test (was getting 1 edge, expects 0) - Integration: Rust c.trait_only() negative test - Integration: 3 PHP heritage + alias tests (singleCandidate correctly fires when the receiver type is not in the index) Performance (findings #1, #2, #3) - Thread pre-computed tiered result into resolveModuleAliasedCall via new tieredOverride parameter — eliminates the duplicate ctx.resolve call on every module-alias path. - Add countCallableCandidates helper that short-circuits at threshold without allocating an intermediate array — replaces the filterCallableCandidates(...).length > 1 allocation in skipMember. - resolveMemberCallByFile lookupCallableByName caching deferred to a follow-up (finding #2) — the fix requires threading widenCache through the file-scoped resolver which is a larger change. Code quality (findings #4, #5) - Remove dead code: redundant conditional in resolveMemberCallByFile where both branches returned null. - Move WidenCache type declaration from mid-file (between JSDoc blocks) to adjacent to CONSTRUCTOR_TARGET_TYPES with other type declarations. Formatting - Applied prettier to call-processor.ts (CI format check was failing). Verification - tsc --noEmit clean - 3188 unit tests pass (0 skipped real tests) - 1766 resolver integration tests pass - Zero regressions — all PHP, Rust, and no-heritageMap tests green Review: https://github.com/abhigyanpatwari/GitNexus/pull/770#issuecomment-4225312416 --- gitnexus/src/core/ingestion/call-processor.ts | 149 ++++++++++++++---- 1 file changed, 121 insertions(+), 28 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 52b1f99e76..c7779be54e 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1303,6 +1303,9 @@ export const processCalls = async ( const CONSTRUCTOR_TARGET_TYPES = new Set(['Constructor', 'Class', 'Struct', 'Record']); +/** Per-file cache for module-alias widening. Cleared between files. */ +type WidenCache = Map; + const filterCallableCandidates = ( candidates: readonly SymbolDefinition[], argCount?: number, @@ -1339,6 +1342,40 @@ const filterCallableCandidates = ( ); }; +/** + * Count callable candidates matching the kind + arity filter without + * allocating an intermediate array. Short-circuits once count exceeds + * `threshold` (default 1) — used by the dispatcher's `skipMember` check + * where we only need to know "more than one survivor". + */ +const countCallableCandidates = ( + candidates: readonly SymbolDefinition[], + argCount?: number, + callForm?: 'free' | 'member' | 'constructor', + threshold = 1, +): number => { + let count = 0; + for (const c of candidates) { + // Kind filter (mirrors filterCallableCandidates) + const typeOk = + callForm === 'constructor' + ? CONSTRUCTOR_TARGET_TYPES.has(c.type) + : CALLABLE_TYPES.has(c.type); + if (!typeOk) continue; + // Arity filter + if ( + argCount !== undefined && + c.parameterCount !== undefined && + (argCount < (c.requiredParameterCount ?? c.parameterCount) || argCount > c.parameterCount) + ) { + continue; + } + count++; + if (count > threshold) return count; // early exit + } + return count; +}; + const toResolveResult = (definition: SymbolDefinition, tier: ResolutionTier): ResolveResult => ({ nodeId: definition.nodeId, confidence: TIER_CONFIDENCE[tier], @@ -1476,9 +1513,6 @@ const dedupSwiftExtensionCandidates = ( * * Replaces the former 200+ line function (SM-19: fuzzy-free call resolution). */ -/** Per-file cache for module-alias widening. Cleared between files. */ -type WidenCache = Map; - /** * Module-alias resolution for member calls without a receiver type. * @@ -1491,6 +1525,7 @@ const resolveModuleAliasedCall = ( currentFile: string, ctx: ResolutionContext, widenCache?: WidenCache, + tieredOverride?: TieredCandidates, ): ResolveResult | null => { if (!call.receiverName) return null; const aliasMap = ctx.moduleAliasMap?.get(currentFile); @@ -1498,15 +1533,19 @@ const resolveModuleAliasedCall = ( const moduleFile = aliasMap.get(call.receiverName); if (!moduleFile) return null; - const tiered = ctx.resolve(call.calledName, currentFile); + // Reuse the caller's pre-computed tiered result when available — + // the dispatcher already called ctx.resolve(call.calledName, currentFile). + const tiered = tieredOverride ?? ctx.resolve(call.calledName, currentFile); if (!tiered) return null; // Try member-form, then constructor-form (for `module.ClassName()` patterns) - let filtered = filterCallableCandidates(tiered.candidates, call.argCount, call.callForm) - .filter((c) => c.filePath === moduleFile); + let filtered = filterCallableCandidates(tiered.candidates, call.argCount, call.callForm).filter( + (c) => c.filePath === moduleFile, + ); if (filtered.length === 0) { - filtered = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor') - .filter((c) => c.filePath === moduleFile); + filtered = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor').filter( + (c) => c.filePath === moduleFile, + ); } if (filtered.length === 0) { // Widen to global callable index scoped to the aliased module file. @@ -1516,11 +1555,13 @@ const resolveModuleAliasedCall = ( defs = ctx.symbols.lookupCallableByName(call.calledName); widenCache?.set(cacheKey, defs); } - filtered = filterCallableCandidates(defs, call.argCount, call.callForm) - .filter((c) => c.filePath === moduleFile); + filtered = filterCallableCandidates(defs, call.argCount, call.callForm).filter( + (c) => c.filePath === moduleFile, + ); if (filtered.length === 0) { - filtered = filterCallableCandidates(defs, call.argCount, 'constructor') - .filter((c) => c.filePath === moduleFile); + filtered = filterCallableCandidates(defs, call.argCount, 'constructor').filter( + (c) => c.filePath === moduleFile, + ); } } return filtered.length === 1 ? toResolveResult(filtered[0], tiered.tier) : null; @@ -1553,7 +1594,9 @@ const resolveMemberCallByFile = ( const typeFiles = new Set(typeResolved.candidates.map((d) => d.filePath)); const methodPool = filterCallableCandidates( - ctx.symbols.lookupCallableByName(calledName), argCount, callForm, + ctx.symbols.lookupCallableByName(calledName), + argCount, + callForm, ); const fileFiltered = methodPool.filter((c) => typeFiles.has(c.filePath)); if (fileFiltered.length === 1) { @@ -1577,7 +1620,8 @@ const resolveMemberCallByFile = ( } // Zero-match null-route: receiver type resolved but no candidate matched - if (fileFiltered.length === 0 && ownerFiltered.length === 0) return null; + // after file-based and owner-based narrowing. Refuse to emit a CALLS edge + // rather than guess — matches the SM-10 R3 null-route contract. return null; }; @@ -1633,26 +1677,75 @@ const resolveCallTarget = ( if (call.callForm === 'free') { return resolveFreeCall( - call.calledName, currentFile, ctx, call.argCount, - tiered, overloadHints, preComputedArgTypes, + call.calledName, + currentFile, + ctx, + call.argCount, + tiered, + overloadHints, + preComputedArgTypes, ); } if (call.callForm === 'constructor') { - return resolveStaticCall(call.calledName, currentFile, ctx, call.argCount, tiered) - ?? singleCandidate(tiered, call.argCount, 'constructor'); + return ( + resolveStaticCall(call.calledName, currentFile, ctx, call.argCount, tiered) ?? + singleCandidate(tiered, call.argCount, 'constructor') + ); } if (call.receiverTypeName) { - const skipMember = (!!overloadHints || !!preComputedArgTypes) && - filterCallableCandidates(tiered.candidates, call.argCount, call.callForm).length > 1; - return (!skipMember ? resolveMemberCall( - call.receiverTypeName, call.calledName, currentFile, ctx, heritageMap, call.argCount, - ) : null) ?? resolveMemberCallByFile( - call.calledName, call.receiverTypeName, currentFile, ctx, - call.argCount, call.callForm, overloadHints, preComputedArgTypes, - ) ?? singleCandidate(tiered, call.argCount, call.callForm); + // Skip the owner-scoped MRO path when the tiered pool has genuine + // overload ambiguity that needs D1-D4+E handling, not D0. + const skipMember = + (!!overloadHints || !!preComputedArgTypes) && + countCallableCandidates(tiered.candidates, call.argCount, call.callForm) > 1; + // Try owner-scoped (resolveMemberCall) then file-scoped (resolveMemberCallByFile). + const memberResult = + (!skipMember + ? resolveMemberCall( + call.receiverTypeName, + call.calledName, + currentFile, + ctx, + heritageMap, + call.argCount, + ) + : null) ?? + resolveMemberCallByFile( + call.calledName, + call.receiverTypeName, + currentFile, + ctx, + call.argCount, + call.callForm, + overloadHints, + preComputedArgTypes, + ); + if (memberResult) return memberResult; + + // singleCandidate tail fallback — but only when the receiver type + // did NOT resolve to any indexed type. This reproduces the old + // resolveCallTarget's D1-D4 null-route guard (SM-10 R3): when the + // type IS in the index but file/owner filtering produced zero + // matches, that's a genuine miss and we must null-route rather than + // fall through to an unscoped singleCandidate that ignores the + // receiver's class hierarchy. + // + // When the type is NOT in the index (e.g. PHP 'mixed', dynamic + // types, unresolvable aliases), the scoped resolvers had nothing to + // work with and singleCandidate is the correct last resort — it + // picks the globally-unique candidate if one exists. + // + // ctx.resolve is cached per (name, file) pair, so this call is free. + const typeResolves = ctx.resolve(call.receiverTypeName, currentFile); + if (typeResolves && typeResolves.candidates.length > 0) { + return null; // null-route: type resolved, no candidate matched + } + return singleCandidate(tiered, call.argCount, call.callForm); } - return resolveModuleAliasedCall(call, currentFile, ctx, widenCache) - ?? singleCandidate(tiered, call.argCount, call.callForm); + return ( + resolveModuleAliasedCall(call, currentFile, ctx, widenCache, tiered) ?? + singleCandidate(tiered, call.argCount, call.callForm) + ); }; // ── Scope key helpers ──────────────────────────────────────────────────── From f424685e83a2f8c00f70b2b451b51526a766d93d Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 11 Apr 2026 09:30:29 +0100 Subject: [PATCH 5/6] fix(SM-19): restore module-alias narrowing and constructor disambiguation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex adversarial review on PR #770 surfaced two silent regressions in the SM-19 thin dispatcher: Finding 1 [high] — Typed member calls bypassed module-alias narrowing. When two homonym receiver types are both imported by the caller, the import-scoped tier no longer narrows and the owner/file resolvers see genuine ambiguity. The dispatcher null-routed silently, dropping valid CALLS edges. Fix: consult `resolveModuleAliasedCall` at the top of the typed-member branch so an active alias on `call.receiverName` picks the aliased file before the generic resolvers run. Finding 2 [medium] — Constructor dispatch lost overload disambiguation. When `resolveStaticCall` bails (ambiguous or ownerless Constructor pool) and the caller supplied `overloadHints` / `preComputedArgTypes`, the branch fell straight through to `singleCandidate` — which also bails on multiple same-arity survivors. Fix: between `resolveStaticCall` and `singleCandidate`, run constructor-filtered overload disambiguation on the tiered pool. Only engages when a narrowing signal is present; preserves SM-10 R3 null-route for genuinely ambiguous cases. Tests: - call-processor.test.ts: 3 new dispatcher-level regression tests covering real-homonym alias narrowing, constructor overload disambiguation with `argTypes`, and null-route control - symbol-table.test.ts: update `module alias homonyms` test which previously codified the Finding 1 regression; now asserts resolution to the aliased file's method Verification: 3191 unit + 2398 integration tests pass; tsc --noEmit clean; prettier clean. --- gitnexus/src/core/ingestion/call-processor.ts | 37 ++++- gitnexus/test/unit/call-processor.test.ts | 146 ++++++++++++++++++ gitnexus/test/unit/symbol-table.test.ts | 17 +- 3 files changed, 189 insertions(+), 11 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index c7779be54e..e61c488083 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1687,12 +1687,43 @@ const resolveCallTarget = ( ); } if (call.callForm === 'constructor') { - return ( - resolveStaticCall(call.calledName, currentFile, ctx, call.argCount, tiered) ?? - singleCandidate(tiered, call.argCount, 'constructor') + const staticResult = resolveStaticCall( + call.calledName, + currentFile, + ctx, + call.argCount, + tiered, ); + if (staticResult) return staticResult; + + // Codex SM-19 Finding 2: When `resolveStaticCall` bails on ambiguous or + // ownerless Constructor pools, give overload/arg-type disambiguation a + // chance before null-routing. Only engages when the caller supplied a + // narrowing signal — preserves SM-10 R3 for genuinely ambiguous cases. + if (overloadHints || preComputedArgTypes) { + const ctorPool = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor'); + if (ctorPool.length > 1) { + const disambiguated = overloadHints + ? tryOverloadDisambiguation(ctorPool, overloadHints) + : preComputedArgTypes + ? matchCandidatesByArgTypes(ctorPool, preComputedArgTypes) + : null; + if (disambiguated) return toResolveResult(disambiguated, tiered.tier); + } + } + return singleCandidate(tiered, call.argCount, 'constructor'); } if (call.receiverTypeName) { + // Codex SM-19 Finding 1: Consult module-alias narrowing BEFORE the + // owner-scoped / file-scoped resolvers. When the caller imports two + // homonym receiver types from different files, import-scoped tiering + // does not narrow (both files are in scope) and the owner/file fallback + // sees genuine ambiguity. An active module alias on `call.receiverName` + // is the only remaining disambiguation signal; without this call the + // dispatcher null-routes silently and drops a valid CALLS edge. + const aliasResult = resolveModuleAliasedCall(call, currentFile, ctx, widenCache, tiered); + if (aliasResult) return aliasResult; + // Skip the owner-scoped MRO path when the tiered pool has genuine // overload ambiguity that needs D1-D4+E handling, not D0. const skipMember = diff --git a/gitnexus/test/unit/call-processor.test.ts b/gitnexus/test/unit/call-processor.test.ts index a12dd05289..96b97f8580 100644 --- a/gitnexus/test/unit/call-processor.test.ts +++ b/gitnexus/test/unit/call-processor.test.ts @@ -2493,6 +2493,152 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { expect(authSave).toBeDefined(); expect(userSave).toBeUndefined(); }); + + it('module-alias guard (real homonym): both files imported, alias narrows typed member call to aliased file', async () => { + // Codex SM-19 adversarial review Finding 1: When BOTH homonym files are + // imported by the caller, import-scoped tiering no longer narrows the + // tiered pool — the dispatcher sees two `save` candidates. Module-alias + // narrowing is the only remaining disambiguation signal. The typed-member + // branch must consult the alias map or null-route silently. + const authModFile = 'src/auth_mod.py'; + const userModFile = 'src/user_mod.py'; + const appFile = 'src/app.py'; + const authUserId = 'class:src/auth_mod.py:User'; + const userUserId = 'class:src/user_mod.py:User'; + const authSaveId = 'method:src/auth_mod.py:save'; + const userSaveId = 'method:src/user_mod.py:save'; + + ctx.symbols.add(authModFile, 'User', authUserId, 'Class'); + ctx.symbols.add(userModFile, 'User', userUserId, 'Class'); + ctx.symbols.add(authModFile, 'save', authSaveId, 'Method', { + ownerId: authUserId, + returnType: 'bool', + }); + ctx.symbols.add(userModFile, 'save', userSaveId, 'Method', { + ownerId: userUserId, + returnType: 'bool', + }); + // BOTH files imported by app.py — creates real ambiguity in tiered pool. + ctx.importMap.set(appFile, new Set([authModFile, userModFile])); + // Alias: `auth` points to auth_mod.py. + ctx.moduleAliasMap.set(appFile, new Map([['auth', authModFile]])); + + // Call `auth.User.save(user)` — receiverName is `auth` (matches alias), + // receiverTypeName is `User` (the class). This is the class-as-receiver + // static-style pattern parse-worker emits when it sees `auth.User.save(x)`. + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'save', + sourceId: 'Function:src/app.py:run', + argCount: 1, + callForm: 'member', + receiverName: 'auth', + receiverTypeName: 'User', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + // Module alias narrows to auth_mod.py. Without it the dispatcher would + // null-route because both User classes own a `save` method and there's + // no heritage or overload signal to pick between them. + expect(rels).toHaveLength(1); + expect(rels[0].targetId).toBe(authSaveId); + }); + + it('constructor overload disambiguation: same-arity ownerless constructors picked via preComputedArgTypes', async () => { + // Codex SM-19 adversarial review Finding 2: When two homonym constructors + // across different files have the same arity but different parameter types, + // `resolveStaticCall` correctly bails (step 3 ambiguity → step 4 bail because + // the tiered pool contains Constructor nodes). Before this fix the dispatcher + // then fell through to `singleCandidate` which also bailed because two + // constructors survive arity filtering. With overload disambiguation after + // `resolveStaticCall`, `preComputedArgTypes` picks the string overload. + const userFile = 'src/models/User.ts'; + const repoFile = 'src/models/Repo.ts'; + const appFile = 'src/app.ts'; + const userClassId = 'Class:src/models/User.ts:User'; + const repoClassId = 'Class:src/models/Repo.ts:User'; + const userCtorId = 'Constructor:src/models/User.ts:User(string)'; + const repoCtorId = 'Constructor:src/models/Repo.ts:User(number)'; + + ctx.symbols.add(userFile, 'User', userClassId, 'Class'); + ctx.symbols.add(repoFile, 'User', repoClassId, 'Class'); + ctx.symbols.add(userFile, 'User', userCtorId, 'Constructor', { + ownerId: userClassId, + parameterCount: 1, + parameterTypes: ['string'], + }); + ctx.symbols.add(repoFile, 'User', repoCtorId, 'Constructor', { + ownerId: repoClassId, + parameterCount: 1, + parameterTypes: ['number'], + }); + ctx.importMap.set(appFile, new Set([userFile, repoFile])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'User', + sourceId: 'Function:src/app.ts:main', + argCount: 1, + callForm: 'constructor', + argTypes: ['string'], + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + expect(rels).toHaveLength(1); + expect(rels[0].targetId).toBe(userCtorId); + }); + + it('constructor overload disambiguation: null-routes when disambiguation cannot pick unique survivor', async () => { + // Control test for Finding 2 fix: when `preComputedArgTypes` does not + // match any candidate uniquely, the dispatcher must null-route rather + // than pick arbitrarily. Preserves SM-10 R3. + const userFile = 'src/models/User.ts'; + const repoFile = 'src/models/Repo.ts'; + const appFile = 'src/app.ts'; + const userClassId = 'Class:src/models/User.ts:User'; + const repoClassId = 'Class:src/models/Repo.ts:User'; + const userCtorId = 'Constructor:src/models/User.ts:User(string)'; + const repoCtorId = 'Constructor:src/models/Repo.ts:User(string)'; + + ctx.symbols.add(userFile, 'User', userClassId, 'Class'); + ctx.symbols.add(repoFile, 'User', repoClassId, 'Class'); + // Both constructors take `string` — genuinely ambiguous. + ctx.symbols.add(userFile, 'User', userCtorId, 'Constructor', { + ownerId: userClassId, + parameterCount: 1, + parameterTypes: ['string'], + }); + ctx.symbols.add(repoFile, 'User', repoCtorId, 'Constructor', { + ownerId: repoClassId, + parameterCount: 1, + parameterTypes: ['string'], + }); + ctx.importMap.set(appFile, new Set([userFile, repoFile])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'User', + sourceId: 'Function:src/app.ts:main', + argCount: 1, + callForm: 'constructor', + argTypes: ['string'], + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + expect(rels).toHaveLength(0); + }); }); // ---- processAssignmentsFromExtracted: Phase 9 accumulator fallback ---- diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index fc7ee6551f..a8af3ccdb5 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -1753,14 +1753,15 @@ describe('resolveCallTarget thin dispatcher (SM-19)', () => { ctx = createResolutionContext(); }); - it('module alias homonyms: thin dispatcher returns null (no D1-D4 fuzzy path)', () => { + it('module alias homonyms: dispatcher resolves via module-alias narrowing to aliased file', () => { // Python-style: `import auth; auth.User.save()` where BOTH auth.py and // other.py define a `User` class with a `save` method. // - // Before SM-19, resolveCallTarget had D1-D4 fuzzy widening that could - // disambiguate via module-alias narrowing. The thin dispatcher delegates - // to resolveMemberCall which sees two homonym Users and correctly returns - // null (genuine ambiguity — no fuzzy path to break the tie). + // Codex SM-19 adversarial review Finding 1: the thin dispatcher must + // consult module-alias narrowing for typed member calls BEFORE falling + // through to owner/file-scoped resolvers. With both homonym files + // imported, owner-scoped resolution sees genuine ambiguity and the only + // remaining disambiguation signal is the alias on `call.receiverName`. ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { returnType: 'None', @@ -1785,9 +1786,9 @@ describe('resolveCallTarget thin dispatcher (SM-19)', () => { ctx, ); - // Thin dispatcher delegates to resolveMemberCall which returns null for - // genuine homonym ambiguity (both Users own `save`). - expect(result).toBeNull(); + // Module-alias narrowing picks auth.py's save, not other.py's. + expect(result).not.toBeNull(); + expect(result?.nodeId).toBe('method:auth:User:save'); }); it('overloadHints ignored for member calls — resolveMemberCall resolves directly', () => { From d6410339978d36140aa3f4ee1fbe209302b9cdd3 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 11 Apr 2026 10:10:00 +0100 Subject: [PATCH 6/6] refactor(SM-19): address code review findings with clean-code pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review on commit f424685e surfaced one P1 correctness regression and two P2 maintainability concerns. This commit closes all ten findings: P1 — Alias helper placement regression - resolveModuleAliasedCall now runs as a FALLBACK in the typed-member branch, after resolveMemberCall/resolveMemberCallByFile return null. Previously it short-circuited BEFORE scoped resolvers, leaking unrelated homonyms from the aliased file when a local var coincidentally matched a module alias. - Added type-file verification guard: alias narrowing only fires when the alias target file is among the receiver type's defining files. Prevents cross-type false positives and hardens SM-10 R3. P2 — Thin-dispatcher drift (roadmap Phase 3) - Extracted disambiguateByOverloadOrArgTypes shared helper. Centralizes the overloadHints → preComputedArgTypes precedence rule used by both member and constructor resolvers. - Folded constructor overload disambiguation into resolveStaticCall as step 4.5 (between the ambiguous-pool bail and the instantiable-class fallback). resolveStaticCall now accepts optional overloadHints / preComputedArgTypes symmetric with resolveMemberCallByFile. - Dispatcher's constructor branch returns to a 2-line delegation. - resolveMemberCallByFile now calls the shared helper instead of inlining the ternary. P2 — Missing test coverage - owner-scoped wins over alias narrowing (alias with unrelated target class must not override unique owner-scoped answer) - alias narrowing rejects unrelated target type (type-file guard) - alias fallthrough: receiverName not in alias map - alias fallthrough: alias target file has no matching method (overloadHints-for-constructor variant transitively covered via the extracted helper's member-path tests; direct dispatcher test deferred as it requires real OverloadHints fixture parsing) P3 — Clarity and durability - Stripped "Codex SM-19 Finding N" prefixes from comments. Replaced with durable explanations of WHY each guarded branch exists. - Added cross-reference comment at the tail-branch resolveModuleAliasedCall call site pointing to the typed-member branch usage. Verification: 3195 unit + 1766 resolver integration + 2398 full integration tests pass. tsc --noEmit clean. prettier clean. Plan: docs/plans/2026-04-11-002-fix-sm19-code-review-findings-plan.md --- gitnexus/src/core/ingestion/call-processor.ts | 158 +++++++++----- gitnexus/test/unit/call-processor.test.ts | 200 ++++++++++++++++-- gitnexus/test/unit/symbol-table.test.ts | 13 +- 3 files changed, 296 insertions(+), 75 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index e61c488083..5edd727375 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1465,6 +1465,31 @@ const tryOverloadDisambiguation = ( return matchCandidatesByArgTypes(candidates, argTypes); }; +/** + * Apply overload-hint or arg-type disambiguation to a pre-filtered candidate + * pool. Returns the unique survivor, or null when neither signal is present, + * neither can disambiguate, or the pool remains ambiguous. + * + * Precedence rule: `overloadHints` wins over `preComputedArgTypes` when both + * are supplied. The AST-based disambiguator has access to live type inference + * hooks, whereas `preComputedArgTypes` is a worker-path pre-computation that + * may be coarser-grained. + * + * Single source of truth for the narrowing-signal precedence used by member + * and constructor resolution paths. Add a new narrowing signal here once, not + * at each call site. + */ +const disambiguateByOverloadOrArgTypes = ( + pool: SymbolDefinition[], + overloadHints: OverloadHints | undefined, + preComputedArgTypes: (string | undefined)[] | undefined, +): SymbolDefinition | null => { + if (!overloadHints && !preComputedArgTypes) return null; + if (overloadHints) return tryOverloadDisambiguation(pool, overloadHints); + if (preComputedArgTypes) return matchCandidatesByArgTypes(pool, preComputedArgTypes); + return null; +}; + /** * Collapse Swift-extension duplicate Class/Struct candidates to the primary * definition, preferring the shortest file path. @@ -1611,11 +1636,11 @@ const resolveMemberCallByFile = ( // Overload disambiguation on the narrowed pool if (fileFiltered.length > 1 || ownerFiltered.length > 1) { const overloadPool = ownerFiltered.length > 1 ? ownerFiltered : fileFiltered; - const disambiguated = overloadHints - ? tryOverloadDisambiguation(overloadPool, overloadHints) - : preComputedArgTypes - ? matchCandidatesByArgTypes(overloadPool, preComputedArgTypes) - : null; + const disambiguated = disambiguateByOverloadOrArgTypes( + overloadPool, + overloadHints, + preComputedArgTypes, + ); if (disambiguated) return toResolveResult(disambiguated, typeResolved.tier); } @@ -1687,43 +1712,19 @@ const resolveCallTarget = ( ); } if (call.callForm === 'constructor') { - const staticResult = resolveStaticCall( - call.calledName, - currentFile, - ctx, - call.argCount, - tiered, + return ( + resolveStaticCall( + call.calledName, + currentFile, + ctx, + call.argCount, + tiered, + overloadHints, + preComputedArgTypes, + ) ?? singleCandidate(tiered, call.argCount, 'constructor') ); - if (staticResult) return staticResult; - - // Codex SM-19 Finding 2: When `resolveStaticCall` bails on ambiguous or - // ownerless Constructor pools, give overload/arg-type disambiguation a - // chance before null-routing. Only engages when the caller supplied a - // narrowing signal — preserves SM-10 R3 for genuinely ambiguous cases. - if (overloadHints || preComputedArgTypes) { - const ctorPool = filterCallableCandidates(tiered.candidates, call.argCount, 'constructor'); - if (ctorPool.length > 1) { - const disambiguated = overloadHints - ? tryOverloadDisambiguation(ctorPool, overloadHints) - : preComputedArgTypes - ? matchCandidatesByArgTypes(ctorPool, preComputedArgTypes) - : null; - if (disambiguated) return toResolveResult(disambiguated, tiered.tier); - } - } - return singleCandidate(tiered, call.argCount, 'constructor'); } if (call.receiverTypeName) { - // Codex SM-19 Finding 1: Consult module-alias narrowing BEFORE the - // owner-scoped / file-scoped resolvers. When the caller imports two - // homonym receiver types from different files, import-scoped tiering - // does not narrow (both files are in scope) and the owner/file fallback - // sees genuine ambiguity. An active module alias on `call.receiverName` - // is the only remaining disambiguation signal; without this call the - // dispatcher null-routes silently and drops a valid CALLS edge. - const aliasResult = resolveModuleAliasedCall(call, currentFile, ctx, widenCache, tiered); - if (aliasResult) return aliasResult; - // Skip the owner-scoped MRO path when the tiered pool has genuine // overload ambiguity that needs D1-D4+E handling, not D0. const skipMember = @@ -1753,26 +1754,48 @@ const resolveCallTarget = ( ); if (memberResult) return memberResult; - // singleCandidate tail fallback — but only when the receiver type - // did NOT resolve to any indexed type. This reproduces the old - // resolveCallTarget's D1-D4 null-route guard (SM-10 R3): when the - // type IS in the index but file/owner filtering produced zero - // matches, that's a genuine miss and we must null-route rather than - // fall through to an unscoped singleCandidate that ignores the - // receiver's class hierarchy. - // - // When the type is NOT in the index (e.g. PHP 'mixed', dynamic - // types, unresolvable aliases), the scoped resolvers had nothing to - // work with and singleCandidate is the correct last resort — it - // picks the globally-unique candidate if one exists. + // Module-alias narrowing runs as a FALLBACK, after owner/file-scoped + // resolvers have returned null. This ordering is load-bearing: placing + // alias narrowing first would short-circuit unique owner-scoped answers + // when a local variable coincidentally matches an alias name, leaking + // unrelated homonyms from the aliased file onto the wrong receiver type. // - // ctx.resolve is cached per (name, file) pair, so this call is free. + // The type-file verification guard is load-bearing for SM-10 R3: an + // alias is only a VALID narrowing signal when the alias target file is + // among the receiver type's defining files. If the alias points at a + // file that does not hold `receiverTypeName`, any candidate we would + // pick from there would belong to an unrelated class — a cross-type + // false positive. ctx.resolve is cached per (name, file), so resolving + // the receiver type a second time here is free. const typeResolves = ctx.resolve(call.receiverTypeName, currentFile); + const aliasMap = ctx.moduleAliasMap?.get(currentFile); + const aliasTargetFile = + call.receiverName && aliasMap ? aliasMap.get(call.receiverName) : undefined; + if ( + aliasTargetFile && + typeResolves && + typeResolves.candidates.some((c) => c.filePath === aliasTargetFile) + ) { + const aliasResult = resolveModuleAliasedCall(call, currentFile, ctx, widenCache, tiered); + if (aliasResult) return aliasResult; + } + + // SM-10 R3 null-route: when the receiver type resolves to indexed types + // but no scoped resolver (nor the guarded alias fallback) produced a + // match, that's a genuine miss — refuse to emit a CALLS edge rather + // than guess via an unscoped singleCandidate that ignores the class + // hierarchy. When the type is NOT in the index (PHP `mixed`, dynamic + // types, unresolvable aliases), the scoped resolvers had nothing to + // work with and singleCandidate is the correct last resort. if (typeResolves && typeResolves.candidates.length > 0) { return null; // null-route: type resolved, no candidate matched } return singleCandidate(tiered, call.argCount, call.callForm); } + // Member call with no inferred receiver type — e.g. Python `mod.fn()` + // where `mod` is a module alias. Module-alias narrowing is the primary + // disambiguation signal here. Also consulted from the typed-member + // branch above as a guarded fallback after owner/file-scoped resolvers. return ( resolveModuleAliasedCall(call, currentFile, ctx, widenCache, tiered) ?? singleCandidate(tiered, call.argCount, call.callForm) @@ -1790,9 +1813,6 @@ const resolveCallTarget = ( // classes (e.g. User.save@100 and Repo.save@200 are distinct keys). // Lookup uses a secondary funcName-only index built in lookupReceiverType. -/** Extract the function name from a scope key ("funcName@startIndex" → "funcName"). */ -const extractFuncNameFromScope = (scope: string): string => scope.slice(0, scope.indexOf('@')); - /** Extract the bare function name from a sourceId. * Handles both unqualified ("Function:filepath:funcName" → "funcName") * and qualified ("Function:filepath:ClassName.funcName" → "funcName"). @@ -2236,6 +2256,8 @@ export const resolveStaticCall = ( ctx: ResolutionContext, argCount?: number, tieredOverride?: TieredCandidates, + overloadHints?: OverloadHints, + preComputedArgTypes?: (string | undefined)[], ): ResolveResult | null => { // 1. Pre-check: does a class with this name exist at all? (O(1)) // This guards against the expensive `ctx.resolve` walk when the name @@ -2297,10 +2319,30 @@ export const resolveStaticCall = ( // with two distinct Constructor nodes across multiple class candidates): // the same Constructor nodes are indexed under the class name in the // tiered pool, so `.some(Constructor)` is true here and we defer to - // `filterCallableCandidates` downstream rather than guess which overload - // to pick. Do not remove this check without also handling the ambiguous - // step-3 path explicitly. + // step 4.5 (overload/arg-type disambiguation) or the caller's fallback. + // Do not remove this check without also handling the ambiguous step-3 + // path explicitly. if (typeResolved.candidates.some((c) => c.type === 'Constructor')) { + // 4.5. Overload / arg-type disambiguation for ambiguous or ownerless + // Constructor pools. When the caller supplied a narrowing signal + // (AST-based overload hints from the sequential path, or pre- + // computed arg types from the worker path), give disambiguation a + // chance before null-routing. Symmetric with resolveMemberCallByFile's + // disambiguation pass — both resolvers now share the same signal + // precedence via disambiguateByOverloadOrArgTypes. Only fires when + // at least one narrowing signal is present; preserves SM-10 R3 for + // genuinely ambiguous cases with no disambiguating input. + if (overloadHints || preComputedArgTypes) { + const ctorPool = filterCallableCandidates(typeResolved.candidates, argCount, 'constructor'); + if (ctorPool.length > 1) { + const disambiguated = disambiguateByOverloadOrArgTypes( + ctorPool, + overloadHints, + preComputedArgTypes, + ); + if (disambiguated) return toResolveResult(disambiguated, typeResolved.tier); + } + } return null; } diff --git a/gitnexus/test/unit/call-processor.test.ts b/gitnexus/test/unit/call-processor.test.ts index 96b97f8580..e69250cebb 100644 --- a/gitnexus/test/unit/call-processor.test.ts +++ b/gitnexus/test/unit/call-processor.test.ts @@ -2495,11 +2495,12 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { }); it('module-alias guard (real homonym): both files imported, alias narrows typed member call to aliased file', async () => { - // Codex SM-19 adversarial review Finding 1: When BOTH homonym files are - // imported by the caller, import-scoped tiering no longer narrows the - // tiered pool — the dispatcher sees two `save` candidates. Module-alias - // narrowing is the only remaining disambiguation signal. The typed-member - // branch must consult the alias map or null-route silently. + // When both homonym files are imported by the caller, import-scoped + // tiering no longer narrows the tiered pool — the dispatcher sees two + // `save` candidates. Module-alias narrowing is the only remaining + // disambiguation signal. The typed-member branch must consult the alias + // map (as a guarded fallback after owner/file-scoped resolvers fail) or + // null-route silently. const authModFile = 'src/auth_mod.py'; const userModFile = 'src/user_mod.py'; const appFile = 'src/app.py'; @@ -2548,14 +2549,189 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { expect(rels[0].targetId).toBe(authSaveId); }); + it('owner-scoped wins over alias narrowing: unique owner-scoped answer beats coincidental alias on unrelated file', async () => { + // Receiver type `User` has exactly one definition, in models.py. Module + // alias `auth → auth.py` exists (because the caller also imports auth.py + // for its own reasons), and auth.py contains an unrelated `Widget` class + // with a homonym `save` method. The caller has `receiverName='auth'` + // (e.g., a local variable coincidentally named `auth`), + // `receiverTypeName='User'`. Owner-scoped resolution must win — alias + // narrowing must not short-circuit a unique correct answer with an + // unrelated homonym from the aliased file. + const modelsFile = 'src/models.py'; + const authFile = 'src/auth.py'; + const appFile = 'src/app.py'; + const modelsUserId = 'class:src/models.py:User'; + const authWidgetId = 'class:src/auth.py:Widget'; + const modelsSaveId = 'method:src/models.py:User:save'; + const authSaveId = 'method:src/auth.py:Widget:save'; + + ctx.symbols.add(modelsFile, 'User', modelsUserId, 'Class'); + ctx.symbols.add(authFile, 'Widget', authWidgetId, 'Class'); + ctx.symbols.add(modelsFile, 'save', modelsSaveId, 'Method', { + ownerId: modelsUserId, + returnType: 'None', + }); + ctx.symbols.add(authFile, 'save', authSaveId, 'Method', { + ownerId: authWidgetId, + returnType: 'None', + }); + ctx.importMap.set(appFile, new Set([modelsFile, authFile])); + ctx.moduleAliasMap.set(appFile, new Map([['auth', authFile]])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'save', + sourceId: 'Function:src/app.py:run', + argCount: 1, + callForm: 'member', + receiverName: 'auth', + receiverTypeName: 'User', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + // Owner-scoped runs first and uniquely resolves User.save to models.py. + // Alias narrowing never fires because the scoped resolver already won. + expect(rels).toHaveLength(1); + expect(rels[0].targetId).toBe(modelsSaveId); + }); + + it('alias narrowing rejects unrelated target type: null-route when alias file does not hold receiver type', async () => { + // Receiver type `User` lives only in models.py, but has no `save` method + // defined. Alias `auth → auth.py`, and auth.py contains an unrelated + // `Widget.save`. Owner-scoped and file-scoped resolvers return null (no + // save on User). Without the type-file verification guard, alias + // narrowing would pick auth.py's `Widget.save` — a cross-type false + // positive. With the guard, auth.py is not in the receiver type's + // defining-files set (which is {models.py}), so alias narrowing bails + // and SM-10 R3 null-routes. + const modelsFile = 'src/models.py'; + const authFile = 'src/auth.py'; + const appFile = 'src/app.py'; + const modelsUserId = 'class:src/models.py:User'; + const authWidgetId = 'class:src/auth.py:Widget'; + const authSaveId = 'method:src/auth.py:Widget:save'; + + ctx.symbols.add(modelsFile, 'User', modelsUserId, 'Class'); + ctx.symbols.add(authFile, 'Widget', authWidgetId, 'Class'); + // NO save on User — deliberately absent to force null-route. + ctx.symbols.add(authFile, 'save', authSaveId, 'Method', { + ownerId: authWidgetId, + returnType: 'None', + }); + ctx.importMap.set(appFile, new Set([modelsFile, authFile])); + ctx.moduleAliasMap.set(appFile, new Map([['auth', authFile]])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'save', + sourceId: 'Function:src/app.py:run', + argCount: 1, + callForm: 'member', + receiverName: 'auth', + receiverTypeName: 'User', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + // Null-route: no CALLS edge. The type-file guard prevented the alias + // from leaking auth.py's Widget.save onto a User-typed receiver. + expect(rels).toHaveLength(0); + }); + + it('alias fallthrough: receiverName not in alias map falls through to owner-scoped resolver', async () => { + // Receiver variable `user` does NOT match any alias entry (alias only + // covers `auth`). Owner-scoped resolution must run to completion and + // pick models.py's User.save — the alias helper's early-bail must not + // interfere with unrelated typed member calls. This exercises the 99% + // hot path where alias narrowing is irrelevant. + const modelsFile = 'src/models.py'; + const authFile = 'src/auth.py'; + const appFile = 'src/app.py'; + const modelsUserId = 'class:src/models.py:User'; + const modelsSaveId = 'method:src/models.py:User:save'; + + ctx.symbols.add(modelsFile, 'User', modelsUserId, 'Class'); + ctx.symbols.add(modelsFile, 'save', modelsSaveId, 'Method', { + ownerId: modelsUserId, + returnType: 'None', + }); + ctx.importMap.set(appFile, new Set([modelsFile, authFile])); + ctx.moduleAliasMap.set(appFile, new Map([['auth', authFile]])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'save', + sourceId: 'Function:src/app.py:run', + argCount: 0, + callForm: 'member', + receiverName: 'user', // NOT 'auth' — no alias match + receiverTypeName: 'User', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + expect(rels).toHaveLength(1); + expect(rels[0].targetId).toBe(modelsSaveId); + }); + + it('alias fallthrough: alias target file has no matching method falls through to owner-scoped', async () => { + // Alias `auth → empty.py` where empty.py exists in the import map but + // has no `save` method at all. Owner-scoped finds models.py's User.save + // uniquely. Even if the type-file guard let alias narrowing fire (it + // won't, because empty.py isn't in the receiver type's files), the + // helper would return null and resolution must still succeed. + const modelsFile = 'src/models.py'; + const emptyFile = 'src/empty.py'; + const appFile = 'src/app.py'; + const modelsUserId = 'class:src/models.py:User'; + const modelsSaveId = 'method:src/models.py:User:save'; + + ctx.symbols.add(modelsFile, 'User', modelsUserId, 'Class'); + ctx.symbols.add(modelsFile, 'save', modelsSaveId, 'Method', { + ownerId: modelsUserId, + returnType: 'None', + }); + // empty.py: no symbols at all. + ctx.importMap.set(appFile, new Set([modelsFile, emptyFile])); + ctx.moduleAliasMap.set(appFile, new Map([['auth', emptyFile]])); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'save', + sourceId: 'Function:src/app.py:run', + argCount: 0, + callForm: 'member', + receiverName: 'auth', + receiverTypeName: 'User', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx); + + const rels = graph.relationships.filter((r) => r.type === 'CALLS'); + expect(rels).toHaveLength(1); + expect(rels[0].targetId).toBe(modelsSaveId); + }); + it('constructor overload disambiguation: same-arity ownerless constructors picked via preComputedArgTypes', async () => { - // Codex SM-19 adversarial review Finding 2: When two homonym constructors - // across different files have the same arity but different parameter types, - // `resolveStaticCall` correctly bails (step 3 ambiguity → step 4 bail because - // the tiered pool contains Constructor nodes). Before this fix the dispatcher - // then fell through to `singleCandidate` which also bailed because two - // constructors survive arity filtering. With overload disambiguation after - // `resolveStaticCall`, `preComputedArgTypes` picks the string overload. + // When two homonym constructors across different files have the same + // arity but different parameter types, `resolveStaticCall` correctly + // bails (step 3 ambiguity → step 4 bail because the tiered pool contains + // Constructor nodes). Step 4.5 then runs overload/arg-type disambiguation + // on the constructor-filtered pool, picking the string overload when the + // caller supplies matching `argTypes` / `preComputedArgTypes`. const userFile = 'src/models/User.ts'; const repoFile = 'src/models/Repo.ts'; const appFile = 'src/app.ts'; diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index a8af3ccdb5..0f12851d6c 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -1757,11 +1757,14 @@ describe('resolveCallTarget thin dispatcher (SM-19)', () => { // Python-style: `import auth; auth.User.save()` where BOTH auth.py and // other.py define a `User` class with a `save` method. // - // Codex SM-19 adversarial review Finding 1: the thin dispatcher must - // consult module-alias narrowing for typed member calls BEFORE falling - // through to owner/file-scoped resolvers. With both homonym files - // imported, owner-scoped resolution sees genuine ambiguity and the only - // remaining disambiguation signal is the alias on `call.receiverName`. + // When both homonym files are imported, owner-scoped resolution sees + // genuine ambiguity (both `User` classes own a `save` method) and the + // only remaining disambiguation signal is the module alias on + // `call.receiverName`. The dispatcher consults alias narrowing as a + // guarded fallback after owner/file-scoped resolvers return null; the + // type-file verification guard requires the alias target file to be + // among the receiver type's defining files before alias narrowing is + // considered a valid signal. ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { returnType: 'None',