fix(SM-11): Codex adversarial review corrections + deeper D0 fixes

Addresses the three high-severity findings from the Codex adversarial review of PR #744 (https://github.com/abhigyanpatwari/GitNexus/pull/744#issuecomment-4212075120), plus four deeper fixes discovered during regression triage. All discovered issues are now addressed end-to-end rather than papered over with tail-return fallbacks.

Codex review findings:

R1 (C++ diamond): The cpp-diamond-inheritance fixture used non-virtual inheritance, which is genuinely ambiguous in real C++ (two Base subobjects). Changed A and B to use 'virtual public Base' so there's a single shared Base subobject and d.method() is an unambiguous call that the leftmost-base MRO walk correctly resolves.

R2 (C# default-interface): The csharp-interface-default-method fixture called user.Validate() via a User-typed variable, but C# does not inherit default interface methods as callable class members — the call is only valid through an interface-typed variable. Changed App.cs to 'IValidator user = new User(...)' which is the idiomatic dispatch pattern.

R3 (resolveCallTarget tail-return): When D1-D4 receiver filtering produced zero file-matched and zero owner-matched candidates for a member call, the function fell through to the permissive single-candidate tail return — silently emitting CALLS edges for methods that don't belong to the receiver. Added an explicit null-route inside the D1-D4 block that fires only when both filters yielded 0.

R4 (Rust negative assertion): Added the c.trait_only() negative integration test in rust.test.ts demonstrating that direct member calls on Rust structs do not walk trait ancestry. The test now passes because of R3 (previously fell through to the tail return).

Regression triage discoveries:

1. D0 was dead code on the sequential pipeline. The sequential path sets overloadHints for every call regardless of whether the method is overloaded, and the original D0 skip condition '!overloadHints && !preComputedArgTypes' was therefore always false. The Java/C#/C++ SM-9/SM-10 inheritance tests were passing ONLY via the tail-return fallback. Fix: narrow the skip to 'overloadHints && filteredCandidates.length > 1' — skip D0 only when there are actually multiple candidates that need overload disambiguation.

2. lookupMethodByOwner couldn't disambiguate arity-differing overloads (e.g. C++ greet() vs greet(string)). With D0 now firing on the sequential path, same-name/different-arity overloads would collapse to an arbitrary first pick. Fix: added an optional argCount parameter to lookupMethodByOwner + lookupMethodByOwnerWithMRO that filters the overload set by parameterCount/requiredParameterCount before the returnType dedup.

3. Python and Rust class methods are captured as Function nodes (not Method) with ownerId set to the class. The methodByOwner index only accepted 'Method' and 'Constructor' types, so Python class methods and Rust trait methods were invisible to D0. Fix: extended the methodByOwner indexing condition to include 'Function' when ownerId is set. This also unlocks the Rust trait-method negative assertion by ensuring the qualified-syntax MRO strategy has something to return null for.

4. D0 was being skipped when a local variable shadowed an imported module name (Python 'from models.c import C; c = C()' creates both a module alias 'c → models/c.py' AND a typed local 'c'). Fix: the D0 skip now gates on 'aliasNarrowed' (a new boolean tracking whether the alias block actually narrowed filteredCandidates) instead of 'hasActiveModuleAlias'. If the method isn't in the aliased module, the receiver is a typed local variable and D0 should run.

5. PHP trait walk missed the HasTimestamps trait because lookupClassByName did not include 'Trait' type. buildHeritageMap uses lookupClassByName to resolve parent names, so 'BaseModel use HasTimestamps' was failing to register an ancestor edge for BaseModel → HasTimestamps. Fix: added 'Trait' to CLASS_TYPES. The trait is now a valid class-like type for heritage resolution (PHP use, Rust impl Trait for Struct, Scala traits).

Test updates:

- Updated the 'no heritageMap' unit test in call-processor.test.ts to assert the correct null-route behavior instead of the old tail-return fallback.

- Added a new unit test asserting Trait inclusion in the class set.

- Updated the 'does NOT include other type-like labels' test to remove Trait from its rejection set.

Verification:

- tsc --noEmit: clean

- vitest run test/unit/: 3016 passed (+1 new Trait inclusion test)

- vitest run test/integration/resolvers/: 1764 passed (+1 new Rust negative assertion)

- Zero regressions

gitnexus/src/core/ingestion/call-processor.ts

@@ -1416,15 +1416,31 @@ const resolveCallTarget = (
     //         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.
-    const hasActiveModuleAlias =
-      !!call.receiverName && ctx.moduleAliasMap?.get(currentFile)?.has(call.receiverName) === true;
-    if (!overloadHints && !preComputedArgTypes && !hasActiveModuleAlias) {
+    // 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;
     }
@@ -1475,6 +1491,23 @@ const resolveCallTarget = (
         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
+      // fuzzy fallback 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;
+      }
     }
   }
 
@@ -1704,6 +1737,7 @@ const resolveMethodByOwner = (
   filePath: string,
   ctx: ResolutionContext,
   heritageMap?: HeritageMap,
+  argCount?: number,
 ): { def: SymbolDefinition; tier: ResolutionTier } | undefined => {
   const typeResolved = ctx.resolve(receiverTypeName, filePath);
   if (!typeResolved) return undefined;
@@ -1722,13 +1756,24 @@ const resolveMethodByOwner = (
   //   firstDef === undefined → owner-scoped resolution found nothing
   //   firstDef && !ambiguous → unambiguous answer
   //   ambiguous              → genuine homonym ambiguity — refuse to pick
+  //
+  // argCount is threaded through so arity-differing overloads
+  // (e.g. C++ `greet()` vs `greet(string)`) are disambiguated inside the
+  // owner-scoped lookup rather than collapsing to an arbitrary first pick.
   let firstDef: SymbolDefinition | undefined;
   let ambiguous = false;
   for (const candidate of typeResolved.candidates) {
     if (!CLASS_LIKE_TYPES.has(candidate.type)) continue;
     const def = canWalkMRO
-      ? lookupMethodByOwnerWithMRO(candidate.nodeId, methodName, heritageMap, ctx.symbols, language)
-      : ctx.symbols.lookupMethodByOwner(candidate.nodeId, methodName);
+      ? lookupMethodByOwnerWithMRO(
+          candidate.nodeId,
+          methodName,
+          heritageMap,
+          ctx.symbols,
+          language,
+          argCount,
+        )
+      : ctx.symbols.lookupMethodByOwner(candidate.nodeId, methodName, argCount);
     if (!def) continue;
     if (!firstDef) {
       firstDef = def;
@@ -1784,8 +1829,16 @@ export const resolveMemberCall = (
   currentFile: string,
   ctx: ResolutionContext,
   heritageMap?: HeritageMap,
+  argCount?: number,
 ): ResolveResult | null => {
-  const resolved = resolveMethodByOwner(ownerType, methodName, currentFile, ctx, heritageMap);
+  const resolved = resolveMethodByOwner(
+    ownerType,
+    methodName,
+    currentFile,
+    ctx,
+    heritageMap,
+    argCount,
+  );
   if (!resolved) return null;
   return toResolveResult(resolved.def, resolved.tier);
 };
@@ -1880,9 +1933,12 @@ export const lookupMethodByOwnerWithMRO = (
   heritageMap: HeritageMap,
   symbols: SymbolTable,
   language: SupportedLanguages,
+  argCount?: number,
 ): SymbolDefinition | undefined => {
-  // Direct lookup first (child override — no walk needed)
-  const direct = symbols.lookupMethodByOwner(ownerNodeId, methodName);
+  // Direct lookup first (child override — no walk needed).
+  // argCount is threaded through so arity-differing overloads on the direct
+  // owner can be disambiguated before the MRO walk starts.
+  const direct = symbols.lookupMethodByOwner(ownerNodeId, methodName, argCount);
   if (direct) return direct;
 
   const strategy = getProvider(language).mroStrategy;
@@ -1909,9 +1965,10 @@ export const lookupMethodByOwnerWithMRO = (
     ancestors = heritageMap.getAncestors(ownerNodeId);
   }
 
-  // Walk ancestors in MRO order — first match wins
+  // Walk ancestors in MRO order — first match wins.
+  // argCount narrows overloaded ancestors the same way as the direct lookup.
   for (const ancestorId of ancestors) {
-    const method = symbols.lookupMethodByOwner(ancestorId, methodName);
+    const method = symbols.lookupMethodByOwner(ancestorId, methodName, argCount);
     if (method) return method;
   }
 

gitnexus/src/core/ingestion/symbol-table.ts

@@ -1,6 +1,18 @@
 import type { NodeLabel } from 'gitnexus-shared';
 
-export const CLASS_TYPES = new Set(['Class', 'Struct', 'Interface', 'Enum', 'Record']);
+export const CLASS_TYPES = new Set([
+  'Class',
+  'Struct',
+  'Interface',
+  'Enum',
+  'Record',
+  // Traits are class-like for heritage resolution: PHP `use Trait;`, Rust
+  // `impl Trait for Struct`, and Scala traits all contribute methods to the
+  // hierarchy of their using/implementing type. Including Trait here lets
+  // buildHeritageMap resolve `h.parentName` to a Trait nodeId so the MRO
+  // walker can visit the trait and find its methods.
+  'Trait',
+]);
 
 export interface SymbolDefinition {
   nodeId: string;
@@ -93,7 +105,24 @@ export interface SymbolTable {
    * overloads share the same returnType, undefined when return types differ (ambiguous).
    * Used by walkMixedChain for deterministic cross-class chain resolution.
    */
-  lookupMethodByOwner: (ownerNodeId: string, methodName: string) => SymbolDefinition | undefined;
+  /**
+   * Lookup a method by owner class + name, optionally filtered by arity.
+   *
+   * When `argCount` is provided, overloads whose parameter count doesn't
+   * accommodate the call's argument count are filtered out before the
+   * returnType dedup runs. This lets D0 (`resolveMemberCall`) disambiguate
+   * arity-differing overloads (e.g. C++ `greet()` vs `greet(string)`) that
+   * would otherwise collide on the shared `ownerId + methodName` key.
+   *
+   * Same-arity, same-returnType overloads (e.g. `save(int)` vs `save(String)`,
+   * both returning `void`) still collapse to the first match — callers must
+   * gate D0 on overload concern before invoking this function for that case.
+   */
+  lookupMethodByOwner: (
+    ownerNodeId: string,
+    methodName: string,
+    argCount?: number,
+  ) => SymbolDefinition | undefined;
 
   /**
    * Look up class-like definitions (Class, Struct, Interface, Enum, Record) by name.
@@ -225,9 +254,16 @@ export const createSymbolTable = (): SymbolTable => {
     }
     globalIndex.get(name)!.push(def);
 
-    // C2. Methods and constructors with ownerId go to methodByOwner index
-    // (in addition to globalIndex).
-    if ((type === 'Method' || type === 'Constructor') && metadata?.ownerId) {
+    // C2. Methods, constructors, and ownerId-bound Functions go to
+    // methodByOwner index (in addition to globalIndex).
+    //
+    // Some language extractors emit class methods as `Function` with an
+    // `ownerId` — notably Python (`def method(self):` inside a class body),
+    // Rust trait methods, and Kotlin object/companion methods. Treating
+    // `Function` with ownerId the same as `Method` here makes D0
+    // (`resolveMemberCall`) work uniformly across all supported languages
+    // instead of silently falling through to D1-D4 fuzzy widening.
+    if ((type === 'Method' || type === 'Constructor' || type === 'Function') && metadata?.ownerId) {
       const key = `${metadata.ownerId}\0${name}`;
       const existing = methodByOwner.get(key);
       if (existing) {
@@ -303,18 +339,42 @@ export const createSymbolTable = (): SymbolTable => {
   const lookupMethodByOwner = (
     ownerNodeId: string,
     methodName: string,
+    argCount?: number,
   ): SymbolDefinition | undefined => {
     const defs = methodByOwner.get(`${ownerNodeId}\0${methodName}`);
     if (!defs || defs.length === 0) return undefined;
-    if (defs.length === 1) return defs[0];
-    // Multiple overloads: return first if all share the same defined returnType (safe for chain resolution).
-    // Return undefined if return types differ or are absent (truly ambiguous — can't determine which overload).
-    const firstReturnType = defs[0].returnType;
-    if (firstReturnType === undefined) return undefined;
-    for (let i = 1; i < defs.length; i++) {
-      if (defs[i].returnType !== firstReturnType) return undefined;
+
+    // Arity narrowing: when an argCount is provided and there are multiple
+    // overloads, keep only those whose parameterCount can accommodate the
+    // call. This resolves arity-differing overloads (e.g. C++ `greet()` vs
+    // `greet(string)`) that share the same `ownerId + methodName` key.
+    //
+    // Candidates with `parameterCount === undefined` (extractor didn't
+    // populate the count — typically variadic or unknown) are retained
+    // conservatively so that legitimate variadic matches still resolve.
+    let pool = defs;
+    if (argCount !== undefined && defs.length > 1) {
+      const arityMatched = defs.filter((d) => {
+        if (d.parameterCount === undefined) return true;
+        const min = d.requiredParameterCount ?? d.parameterCount;
+        return argCount >= min && argCount <= d.parameterCount;
+      });
+      // Only adopt the arity-narrowed pool when it found matches; if arity
+      // rules out every candidate, fall back to the unfiltered set so the
+      // caller's fuzzy path still has something to work with.
+      if (arityMatched.length > 0) pool = arityMatched;
     }
-    return defs[0];
+
+    if (pool.length === 1) return pool[0];
+    // Multiple overloads after arity narrowing: return first if all share
+    // the same defined returnType (safe for chain resolution), undefined if
+    // return types differ (truly ambiguous — can't determine which overload).
+    const firstReturnType = pool[0].returnType;
+    if (firstReturnType === undefined) return undefined;
+    for (let i = 1; i < pool.length; i++) {
+      if (pool[i].returnType !== firstReturnType) return undefined;
+    }
+    return pool[0];
   };
 
   const lookupClassByName = (name: string): SymbolDefinition[] => {

gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/A.h

@@ -1,5 +1,10 @@
 #pragma once
 #include "Base.h"
 
-class A : public Base {
+// Virtual inheritance: together with `B : virtual public Base`, this creates
+// a single shared `Base` subobject under `Derived`, so `d.method()` is an
+// unambiguous call in real C++. Without the `virtual` keyword, a non-virtual
+// diamond would produce two separate `Base` subobjects and the call would
+// be ambiguous, requiring `d.A::method()` or `d.B::method()` to disambiguate.
+class A : virtual public Base {
 };

gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/B.h

@@ -1,5 +1,7 @@
 #pragma once
 #include "Base.h"
 
-class B : public Base {
+// See the comment in A.h — both sides of the diamond use virtual inheritance
+// so there is exactly one `Base` subobject under `Derived`.
+class B : virtual public Base {
 };

gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/App.cs

@@ -4,7 +4,12 @@ public class App
 {
     public static void Run()
     {
-        User user = new User("alice");
+        // Default interface methods in C# 8.0+ are reachable ONLY through
+        // the interface type, not as inherited class members. Declaring the
+        // variable as IValidator is the idiomatic way to invoke Validate().
+        // `User user = new User(...); user.Validate();` would be a compile
+        // error because User does not expose Validate as a class member.
+        IValidator user = new User("alice");
         user.Validate();
     }
 }

gitnexus/test/integration/resolvers/rust.test.ts

@@ -1859,23 +1859,26 @@ describe('Rust abstract dispatch (Repository trait)', () => {
 });
 
 // ---------------------------------------------------------------------------
-// SM-11: Rust Child struct — direct impl method resolution via D0
+// SM-11: Rust Child extends Parent — qualified-syntax MRO
 //
 // Companion integration test for the unit-level Rust qualified-syntax tests
-// in symbol-table.test.ts. Validates end-to-end that Rust direct-impl methods
-// resolve through the owner-scoped D0 path (`resolveMemberCall`).
+// in symbol-table.test.ts. Validates end-to-end that:
 //
-// NOTE on trait-inherited methods: Rust's qualified-syntax MRO strategy in
-// `lookupMethodByOwnerWithMRO` correctly returns null for trait-inherited
-// methods at the unit level. However, in the current pipeline, Rust trait
-// default methods are captured as `Function` nodes (not `Method` with
-// ownerId), so the owner-scoped index does not contain them. This means
-// direct `obj.trait_method()` calls currently fall through to D1-D4 fuzzy
-// widening rather than being correctly null-routed. Rust trait capture as
-// Method-with-ownerId is a Phase 5 (SM-16) fix — out of SM-11 scope.
+//   1. Direct `impl` methods on a struct resolve through the D0 owner-scoped
+//      path (`resolveMemberCall`) — the positive control.
+//
+//   2. Trait-inherited default methods are NOT reachable via direct
+//      `obj.trait_method()` syntax. Rust requires the trait to be in scope
+//      and uses qualified syntax for trait dispatch; the resolver correctly
+//      treats direct member calls as opaque to trait ancestry.
+//
+//      Previously this case emitted a false-positive CALLS edge via the
+//      permissive tail-return in resolveCallTarget — Codex review finding
+//      R3 (PR #744). The tail-return is now null-routed when D1-D4 receiver
+//      filtering produces zero matches on both file and owner dimensions.
 // ---------------------------------------------------------------------------
 
-describe('Rust Child direct-impl method resolution (SM-11)', () => {
+describe('Rust Child extends Parent — qualified-syntax MRO (SM-11)', () => {
   let result: PipelineResult;
 
   beforeAll(async () => {
@@ -1899,4 +1902,21 @@ describe('Rust Child direct-impl method resolution (SM-11)', () => {
     );
     expect(ownCall).toBeDefined();
   });
+
+  it('does NOT resolve c.trait_only() to Parent::trait_only via direct member call', () => {
+    // Qualified-syntax MRO: direct member calls on structs do not walk trait
+    // ancestry. `c.trait_only()` must null-route because `trait_only` is
+    // defined on the trait, not on the Child struct.
+    //
+    // The resolveCallTarget tail-return tightening (R3) is what makes this
+    // assertion testable: before the fix, resolveCallTarget would fall
+    // through D1-D4 (zero file matches, zero owner matches) and silently
+    // pick the single fuzzy candidate as a false-positive edge.
+    const calls = getRelationships(result, 'CALLS');
+    const traitCall = calls.find(
+      (c) =>
+        c.target === 'trait_only' && c.source === 'run' && c.targetFilePath.includes('parent.rs'),
+    );
+    expect(traitCall).toBeUndefined();
+  });
 });

gitnexus/test/unit/call-processor.test.ts

@@ -1716,7 +1716,24 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => {
     expect(doWorkCalls).toHaveLength(1);
   });
 
-  it('D0 skipped: same scenario still resolves via D1-D4 when heritageMap is undefined', async () => {
+  it('no heritageMap: inherited methods are unresolvable (null-routed, not false-positive)', async () => {
+    // Without a HeritageMap, the resolver cannot know that Parent.parentMethod
+    // belongs to Child's ancestry. The old D1-D4 tail-return would silently
+    // pick the lone fuzzy candidate and emit a CALLS edge — but that was an
+    // accidental match that happened to line up because `parentMethod`
+    // was unique in the global index.
+    //
+    // After the R3 tail-return tightening (PR #744 Codex review), member
+    // calls whose D1-D4 narrowing produces zero file-matched and zero
+    // owner-matched candidates null-route instead of falling through.
+    // The test now asserts the honest answer: without heritage information,
+    // we cannot attribute `c.parentMethod()` to `Parent` and therefore
+    // emit no edge.
+    //
+    // In the real ingestion pipeline, heritageMap is always threaded
+    // through, so this scenario is only reachable in tests that explicitly
+    // omit it. Keeping the test confirms the null-route behavior and
+    // documents the invariant "no heritage → no inherited-method edges".
     const { parentMethodId, appFile, parentFile, childFile } = setupChildParent();
 
     await processCalls(
@@ -1739,13 +1756,14 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => {
       ],
       createASTCache(),
       ctx,
-      // no heritageMap — D0 fast path must be skipped, D1-D4 must still resolve
+      // no heritageMap — D0 MRO walk is unavailable, D1-D4 receiver filtering
+      // also cannot link c.parentMethod() to Parent, so no edge is emitted.
     );
 
     const parentMethodCalls = graph.relationships.filter(
       (r) => r.type === 'CALLS' && r.targetId === parentMethodId,
     );
-    expect(parentMethodCalls).toHaveLength(1);
+    expect(parentMethodCalls).toHaveLength(0);
   });
 
   it('overloadHints guard: D0 skipped so literal-inferred overload disambiguation picks the right overload', async () => {

gitnexus/test/unit/symbol-table.test.ts

@@ -798,8 +798,19 @@ describe('SymbolTable', () => {
       expect(table.lookupClassByName('Qux')).toEqual([]);
     });
 
+    it('includes Trait in the class set (PHP use, Rust impl, Scala traits)', () => {
+      // Traits are class-like for heritage resolution — they contribute
+      // methods to the using/implementing type's hierarchy. buildHeritageMap
+      // relies on this to resolve `use Trait;` edges in PHP, `impl Trait for
+      // Struct` in Rust, etc. Added as part of PR #744 (SM-11 Codex review
+      // fixes) after the PHP HasTimestamps trait walk gap was discovered.
+      table.add('src/a.rs', 'Writer', 'trait:Writer', 'Trait');
+      const results = table.lookupClassByName('Writer');
+      expect(results).toHaveLength(1);
+      expect(results[0].nodeId).toBe('trait:Writer');
+    });
+
     it('does NOT include other type-like labels outside the allowed class set', () => {
-      table.add('src/a.rs', 'User', 'trait:User', 'Trait');
       table.add('src/a.ts', 'User', 'type:User', 'Type');
       expect(table.lookupClassByName('User')).toEqual([]);
     });