mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(SM-19): restore module-alias narrowing and constructor disambiguation
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.
This commit is contained in:
parent
b268ede7c4
commit
f424685e83
3 changed files with 189 additions and 11 deletions
|
|
@ -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 =
|
||||
|
|
|
|||
|
|
@ -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 ----
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue