From f424685e83a2f8c00f70b2b451b51526a766d93d Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 11 Apr 2026 09:30:29 +0100 Subject: [PATCH] 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 c7779be54..e61c48808 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 a12dd0528..96b97f858 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 fc7ee6551..a8af3ccdb 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', () => {