feat(cpp): resolve template-body this-> + using ns::name calls in scope resolver

[Copilot]

Template class bodies had two positive-lookup gaps: explicit this->member() calls were not reliably resolving through class ownership/MRO, and free calls introduced via using ns::name could be missed in template contexts. This PR closes those gaps while preserving existing two-phase dependent-base suppression behavior and keeping legacy-parity CI stable.

• Scope resolution: explicit this receiver path (C++-only)

  • Added an explicit receiver-bound branch for receiverName === 'this' in receiver-bound-calls, gated behind resolveThisViaEnclosingClass and enabled only for C++.
  • Resolves against the enclosing class and its MRO, enabling:
    • this->f() → most-derived member
    • this->base_method() → inherited member via EXTENDS chain
  • Uses full overload-set narrowing + ambiguity suppression (no first-overload bias), and documents C++ name-hiding behavior for arity-mismatch cases.

• Scope resolution: using ns::name recovery for template bodies

  • Extended C++ resolver fallback to recover unresolved free calls from ParsedImport named-using entries (kind: 'named').
  • Resolves localName back to namespace member via qualified namespace resolver.
  • Applies arity/argument narrowing before returning candidates.
  • Treats multiple candidates as ambiguous (returns 'ambiguous') to avoid non-deterministic binding.

• Callable lookup hardening correction

  • Removed the previously added broad same-file module-scope fallback from shared findCallableBindingInScope to avoid cross-language scope-boundary regressions.

• Integration fixtures + assertions

  • Updated fixtures and C++ resolver integration tests to add/strengthen guards for:
    • this->f()
    • this->base_method()
    • utils::ns_helper()
    • ns_helper_2() after using utils::ns_helper_2;
  • Added paired two-phase fixture coverage (f() suppressed + this->f() resolved in the same class body).
  • Added name-hiding arity regression coverage and positive derived-overload coverage (this->f(42)).
  • Strengthened assertions with concrete targetFilePath checks.

• Parity CI stability

  • Marked new scope-resolver-only C++ template/this-> assertions as expected legacy-DAG parity skips, so registry-primary behavior is tested while legacy parity remains green.

template<class T>
struct Derived : Base<T> {
  void g() { this->f(); }             // resolves to Derived/Base member chain
  void k() { this->base_method(); }   // resolves via EXTENDS chain
};

using utils::ns_helper_2;
template<class T>
struct D : Base<T> {
  void g() {
    utils::ns_helper();  // qualified namespace call resolves
    ns_helper_2();       // named-using free call resolves
  }
};

[!WARNING]

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	May 14, 2026 4:57pm

[github-actions]

Claude finished @magyargergo's task in 7m 47s —— View job

---

PR #1590 — Production-Readiness Review

Tasks:

• Read repo context files (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
• Read all changed implementation files
• Read related/supporting files (MIGRATED_LANGUAGES, CI workflows, fixture files)
• Hygiene checks (branch diff, Unicode scan, firewall warning)
• Compile review

---

Review bar for this PR

DoD.md §2.1 (correctness), §2.7 (tests), and §6 (not-done signals) plus the PR-specific bar: prefer a missed binding over a misleading binding unless the new binding is proven deterministic, language-scoped, and arity-safe. C++ is in MIGRATED_LANGUAGES — the scope-resolution path owns CALLS edges in production.

---

Current PR state

Open. 4 commits: 95ea800 (initial plan), 17fb1ad (implementation), 2d8a142 (ambiguity fix), 8f7d318 (merge-from-main). CI checks exist but conclusions not verified here (re-running state reported). The merge-from-main commit brings in 586dbf7 (template specialization disambiguation from #1587).

---

Branch hygiene assessment

Classification: merge-from-main commit present but harmless and merge-safe.

8f7d318 merges main (which advanced to include #1587 — template specialization disambiguation). The diff from that merge touches 21 files, all in C++ scope resolution or the cpp.test.ts suite — related work, no unrelated workflow/Docker/release churn. The merge is topically coherent with this PR.

---

Understanding of the change

Three independent mechanisms added:

1. Case 0.5 in emitReceiverBoundCalls (receiver-bound-calls.ts:328–370): When receiverName === 'this', walk findEnclosingClassDef + MRO, use findOwnedMember to locate the member, apply arity check via narrowOverloadCandidates, emit edge.

2. Named-using recovery in resolveAdlCandidates (scope-resolver.ts:221–248): Before falling through to pickCppAdlCandidates, scan callerParsed.parsedImports for kind === 'named' entries whose localName === site.name, resolve the imported namespace member, treat multiple distinct hits as 'ambiguous'.

3. Module-scope fallback in findCallableBindingInScope (walkers.ts:258–283): After the lexical scope chain walk finds nothing, scan scopes.scopeTree.byId.values() for a Module scope with scope.filePath === startFilePath and check its bindings.

---

Findings

---

Finding 1: this branch fires for ALL languages, preempts type-binding path (Case 4)

Severity: major
File: gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts:328–370

Evidence:
Case 0.5 (receiverName === 'this') is inserted before Case 1 (namespace), Case 2 (class-name), and Case 4 (type binding). The branch has no language guard. Languages that write this.Method() — C# (this.Save() confirmed in csharp-self-this-resolution/src/Models/User.cs:6), JavaScript, TypeScript, Java, Kotlin, PHP — all have explicitReceiver.name === 'this' in their reference sites. Case 0.5 will fire for all of them and, if it succeeds, sets handledSites.add(siteKey) and continue — permanently bypassing Case 4.

Case 4 uses pickOverload which correctly handles OVERLOAD_AMBIGUOUS and returns the sentinel when arity normalization collapses distinct overloads (see pickOverload → isOverloadAmbiguousAfterNormalization). Case 0.5 uses findOwnedMember (first overload only) + narrowOverloadCandidates([singleDef], ...). For a class with two same-arity overloads (Save(string) / Save(int)), Case 4 would return OVERLOAD_AMBIGUOUS and suppress; Case 0.5 would feed the first-registered overload and pass it through if arity matches — false positive edge.

The existing csharp-self-this-resolution test covers only the simple single-overload shape and would not catch this regression.

Production risk: For any OO language with this.Method() calls where the class has overloaded methods, Case 0.5 can emit a false positive that Case 4 would have suppressed. Because C#, TypeScript, and Java are all in MIGRATED_LANGUAGES, this is live in production paths.

Recommended fix: Language-gate Case 0.5 to C++ via a flag on ReceiverBoundProviderSubset (e.g., resolveThisViaEnclosingClass?: boolean), or prove with existing cross-language tests that OVERLOAD_AMBIGUOUS suppression still fires correctly for all migrated languages. Fix this →

Blocks merge: yes

---

Finding 2: findOwnedMember feeds a single overload to narrowOverloadCandidates in the this-> path

Severity: major
File: gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts:333–344

Evidence:

memberDef = findOwnedMember(ownerId, memberName, model);
// findOwnedMember = model.methods.lookupAllByOwner(ownerDefId, memberName)[0]
if (memberDef !== undefined) {
  if (
    site.kind === 'call' &&
    narrowOverloadCandidates([memberDef], site.arity, site.argumentTypes).length === 0
  ) {
    memberDef = undefined;
    break;
  }
  break;
}

findOwnedMember returns lookupAllByOwner(...)[0] — the first registered overload. narrowOverloadCandidates is called with that single candidate. Two failure modes:

A (false positive): Class has f(int) and f(string). First registered is f(string). Call is this->f(42) (arity 1). narrowOverloadCandidates([f_string], 1, ['int']) — if arity passes (both have arity 1) but type doesn't narrow further, returns [f_string] → emits edge to wrong overload.

B (false miss): First registered is f(int) (arity 1). Call is this->f() (arity 0). Narrowing rejects, break — never checks f() overload (arity 0) which would match.

Case 4's pickOverload correctly calls model.methods.lookupAllByOwner(ownerId, memberName) (all overloads) and handles the full narrowing + ambiguity path. The this-> path should mirror that.

Production risk: Any C++ class with overloaded methods called via this-> will produce either false positive (wrong overload selected) or false miss (correct overload skipped).

Recommended fix: Replace findOwnedMember + narrowOverloadCandidates([single]) with pickOverload(ownerId, memberName, site, model) (already defined in the same file at line 665). This correctly handles the full overload set and returns OVERLOAD_AMBIGUOUS when warranted. Fix this →

Blocks merge: yes

---

Finding 3: Module-scope fallback in findCallableBindingInScope is too broad and not language-gated

Severity: major
File: gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts:258–283

Evidence:

// After lexical chain walk finds nothing:
if (startFilePath !== undefined) {
  for (const scope of scopes.scopeTree.byId.values()) {
    if (scope.kind !== 'Module' || scope.filePath !== startFilePath) continue;
    // checks scope.bindings and lookupBindingsAt(scope.id, ...)
    return b.def; // first callable match wins
  }
}

Problems:

Scope boundary violation: When a template-body's lexical chain ends at a Class scope (the known bug this addresses), the fallback bypasses the scope boundary and returns any same-file module-level callable with the matching name. If a file has both a template method calling helper() and an unrelated module-level function named helper, the fallback would silently resolve to the module-level helper even if it shouldn't be visible.

Not language-gated: findCallableBindingInScope is used by emitFreeCallFallback for all languages. For TypeScript, Go, Python, and C# files where a scope chain ends early (any legitimate scope-chain gap), this fallback fires and performs a file-wide callable scan.

Non-deterministic iteration: scopes.scopeTree.byId.values() iterates a Map by insertion order, which is deterministic within a single run but ordering depends on parse traversal order. If a file has multiple top-level Module scopes (an edge case), which one wins is not guaranteed by contract.

No ambiguity detection: If two Module-scope callables with the same name exist (e.g., two overloads both registered at module scope), the fallback returns the first and ignores the second — potential false positive.

Production risk: For any large C++ file with template-heavy shapes, any unqualified call within a template body can now resolve to any same-file module-level callable with the same name. Given that C++ headers commonly declare many utility functions at module scope, this could systematically produce false CALLS edges throughout C++ projects.

Recommended fix: Narrow the fallback to be C++-specific (add a flag or call-site guard), add shadowing/ambiguity detection before returning, and add a regression test with a same-file unrelated callable and a template body call to prove it is NOT selected. Fix this →

Blocks merge: yes

---

Finding 4: No paired positive/negative test in the same fixture

Severity: major
File: gitnexus/test/integration/resolvers/cpp.test.ts:1952–1993, fixtures cpp-two-phase-dependent-base/derived.h, cpp-two-phase-this-qualified/derived.h

Evidence:
The negative test (cpp-two-phase-dependent-base) uses f() (unqualified) inside Derived<T>::g() — correctly expects 0 edges. The positive test (cpp-two-phase-this-qualified) uses this->f() inside a different Derived<T> — correctly expects 1 edge.

There is no single fixture that contains both forms side-by-side in the same class body. Without it, the test suite cannot prove that the same template body correctly handles:

• f() → unresolved (dependent base suppression intact)
• this->f() → resolved (new positive path)

The risk: if the new this-> path accidentally weakened the dependent-base suppression for unqualified calls (e.g., by seeding handledSites incorrectly), the existing tests would not catch it because they operate on separate fixtures.

Production risk: Dependent-base suppression regression could go undetected through CI.

Recommended fix: Add a third fixture (e.g., cpp-two-phase-paired) containing a template class with both f() and this->f() calls to the same base member, asserting f() emits 0 edges and this->f() emits exactly 1. Fix this →

Blocks merge: yes

---

Finding 5: this-> arity-mismatch breaks MRO walk (undocumented C++ name-hiding choice)

Severity: minor
File: receiver-bound-calls.ts:336–343

Evidence:

if (
  site.kind === 'call' &&
  narrowOverloadCandidates([memberDef], site.arity, site.argumentTypes).length === 0
) {
  memberDef = undefined;
  break;  // stops chain walk
}

If the derived class has void f(int) and this->f() is called (arity 0), the arity check fails and the chain walk stops — Base::f() (arity 0) is never tried. In C++, name hiding semantics (a derived-class name hides all base-class overloads of the same name) would justify this break. But:

1. The PR doesn't document this as an intentional name-hiding approximation.
2. There is no test proving this behavior is correct.
3. Inconsistency with Case 2 (class-name receiver) which has the same break with an explicit comment explaining the PHP rationale — no equivalent C++ rationale here.

Recommended fix: Document explicitly with a comment referencing C++ name-hiding semantics ([basic.lookup]). Add a test for this->f() with arity mismatch at derived class and a matching overload at base class.

Blocks merge: no

---

Finding 6: using ns::name recovery bypasses arity narrowing for overloaded namespace functions

Severity: minor
File: gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts:228–243

Evidence:

const member = resolveCppQualifiedNamespaceMember(imp.targetRaw, imp.importedName, ...);
if (seenUsing.has(member.nodeId)) continue;
seenUsing.add(member.nodeId);
usingNamedHits.push(member);

resolveCppQualifiedNamespaceMember returns one SymbolDefinition. If the namespace has ns_helper_2(int) and ns_helper_2() as separate overloads (separate nodeIds), both pass imp.importedName === 'ns_helper_2' and both get pushed → usingNamedHits.length === 2 → returns 'ambiguous' (conservative). This is safe.

If there is only one overload but its arity doesn't match the call site, the hit is returned anyway and arity is not checked here. The call will then propagate through and potentially emit a false positive if downstream narrowing doesn't catch it. This is a minor risk because resolveAdlCandidates is a last-resort path and the single-hit return is plausible.

Recommended fix: After collecting usingNamedHits, apply arity narrowing using site.arity/site.argumentTypes before returning, or document why it's acceptable to skip arity here.

Blocks merge: no

---

Finding 7: Test assertions use name-only matching without target file/node identity

Severity: minor
File: gitnexus/test/integration/resolvers/cpp.test.ts:1982–1992

Evidence:

it('Derived<T>::g() -> this->f() resolves to f (1 edge)', () => {
  const calls = getRelationships(result, 'CALLS');
  const thisCalls = calls.filter((c) => c.source === 'g' && c.target === 'f');
  expect(thisCalls.length).toBe(1);
});

The test asserts 1 edge from g to any function named f. It does NOT assert targetFilePath === 'base.h' or verify the target nodeId. If the fixture contained another f (e.g., a module-level helper or an unrelated function), the assertion would still pass while masking a resolution error. Compare to other tests in the file that do assert targetFilePath (e.g., cpp-two-phase-dependent-base-ns at line 2036–2039).

Recommended fix: Add expect(thisCalls[0].targetFilePath).toContain('base.h') (or equivalent) to both this-> positive tests. Similarly for ns_helper and ns_helper_2 tests.

Blocks merge: no

---

PR-specific assessment: C++ this-> receiver resolution

The new Case 0.5 correctly identifies the structural gap (template scope chain doesn't register a typeBinding for this, so Case 4 misses). The findEnclosingClassDef + MRO walk approach is architecturally sound. However:

• Not language-gated → preempts type-binding resolution for C#, TS, Java (Finding 1, blocker)
• Single-overload narrowing → false positives for overloaded methods (Finding 2, blocker)
• handledSites.add(siteKey) only fires after successful resolution (correct, not a bug)
• reason = 'global' for same-file is consistent with super-branch usage (acceptable)
• findEnclosingClassDef will find the innermost class scope — correct for methods, but could be wrong for lambdas-in-methods (existing limitation, not introduced by this PR)

---

PR-specific assessment: named using-declaration recovery

The using ns::name recovery in resolveAdlCandidates is C++-specific (lives in scope-resolver.ts), correctly typed to kind === 'named' (not wildcard), and handles the ambiguity case conservatively. The lookup via resolveCppQualifiedNamespaceMember with imp.targetRaw/imp.importedName is correct for the using utils::ns_helper_2 form. Arity narrowing gap (Finding 6) is minor. Ordering (before ADL) is correct C++ lookup semantics. This section is the most defensible part of the PR.

---

PR-specific assessment: module-scope fallback

This is the highest-risk change. The fallback is framed as targeted at "template-heavy C++ fixture shapes" but is implemented as a generic findCallableBindingInScope change that fires for all languages (Finding 3, blocker). The comment on line 259 says "some capture shapes in template-heavy C++ fixtures" but the implementation has no guard. The byId.values() iteration is also non-deterministic for multi-Module-scope edge cases, and the first-match return ignores ambiguity. Recommend constraining this to C++ or proving the fix at the root cause (why does the scope chain end at Class instead of Module?) rather than papering over it with a fallback scan.

---

PR-specific assessment: dependent-base suppression

The existing negative tests (cpp-two-phase-dependent-base, cpp-two-phase-dependent-base-ns) still run and assert toBe(0). The isCppDependentBaseMember gate in isCallableVisibleFromCaller is untouched. The this-> positive path fires on explicitReceiver !== undefined sites, which unqualified dependent-base calls are NOT (they have no explicit receiver). So the negative suppression is structurally independent of the new positive path. The gap is the absence of a paired test in one fixture (Finding 4, blocker).

---

PR-specific assessment: tests and CI

CI: C++ is in MIGRATED_LANGUAGES. ci-scope-parity.yml runs C++ resolver tests with both REGISTRY_PRIMARY_CPP=0 and REGISTRY_PRIMARY_CPP=1. ci-tests.yml cross-platform job runs npx vitest run on Windows and macOS (all tests, no filter). Coverage is structurally adequate, but the test content has the gaps identified above.

Missing tests:

• this->f(args) arity mismatch → no edge (verifies conservative narrowing)
• Class with two overloads of the member name → both via this->
• using a::x; using b::x; → expect 0 or ambiguous, not a resolved edge
• Same-file module callable with same name as template-body call → must NOT resolve via module fallback
• Cross-language: C# / TypeScript this.Save() with two same-arity overloads → must preserve OVERLOAD_AMBIGUOUS suppression

---

Hidden Unicode / hygiene

Firewall warning: The Copilot warning about extension.ladybugdb.com originates from gitnexus/scripts/install-duckdb-extension.mjs, which is a pre-existing file not modified by this PR. The garbled non-ASCII in the warning message is path-fragment noise in the firewall log, not actual binary content in repo files. Not a concern from this PR.

Unicode scan: All changed source files (.ts, .h) are ASCII-clean. No bidi controls detected in any executable or test code. The changed .h fixtures are minimal pure-ASCII C++ headers.

Diff hygiene: The merge commit content is topically related to C++ resolution. No unrelated formatting, Docker, release, or dependency changes introduced.

---

Final verdict

not production-ready

Three blockers prevent merge. Finding 1: The receiverName === 'this' branch at Case 0.5 is not language-gated and preempts the type-binding resolution path (Case 4) for C#, TypeScript, Java, and all other OO languages — for any class with overloaded methods, Case 0.5 can emit a false positive that Case 4's OVERLOAD_AMBIGUOUS sentinel would have suppressed. Finding 2: findOwnedMember feeds only the first registered overload to narrowOverloadCandidates, producing either false positive (wrong overload selected when first happens to pass arity) or false miss (correct overload skipped when first fails arity) — should use pickOverload which already handles the full overload set correctly. Finding 3: The module-scope fallback in findCallableBindingInScope is not language-gated and performs a broad file-wide callable scan whenever the lexical scope chain ends before Module — for any template body in a file with same-named module-level functions, this can systematically produce false CALLS edges. Together, these constitute a false-positive production risk in a shared infrastructure path, which the DoD §2.1 and §6 "not-done signals" bar prohibits given that CALLS edges from the scope-resolution path are consumed directly by impact analysis, RAG, and search with no downstream sanity filter.

[github-actions]

✨ PR Autofix

Found fixable formatting / unused-import issues across 49 changed lines. Comment /autofix on this PR to apply them, or run npm run lint:fix && npm run format locally.

{"schema":"gitnexus.pr-autofix/v2","state":"fixes-available","pr_number":1590,"changed_lines":49,"head_sha":"8f7d3181a34e98fa4be7e715c18c71a5ba42ef69","run_id":"25864449184","apply_command":"/autofix"}

[magyargergo]

/autofix

[github-actions]

✅ Applied autofix and pushed a commit. (apply run)

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Tests	success	unit tests, 3 platforms
✅ E2E	success	gitnexus-web changes only

Test Results

Tests	Passed	Failed	Skipped	Duration
9043	9042	0	1	450s

✅ All 9042 tests passed

1 test(s) skipped — expand for details

• buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	78.19%	28482/36425	N/A%	—	🟢 ███████████████░░░░░
Branches	66.57%	18062/27132	N/A%	—	🟢 █████████████░░░░░░░
Functions	83.02%	2857/3441	N/A%	—	🟢 ████████████████░░░░
Lines	81.48%	25726/31571	N/A%	—	🟢 ████████████████░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

Findings

Finding 1: this branch fires for ALL languages, preempts type-binding path (Case 4)

Severity: major File: gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts:328–370

Evidence: Case 0.5 (receiverName === 'this') is inserted before Case 1 (namespace), Case 2 (class-name), and Case 4 (type binding). The branch has no language guard. Languages that write this.Method() — C# (this.Save() confirmed in csharp-self-this-resolution/src/Models/User.cs:6), JavaScript, TypeScript, Java, Kotlin, PHP — all have explicitReceiver.name === 'this' in their reference sites. Case 0.5 will fire for all of them and, if it succeeds, sets handledSites.add(siteKey) and continue — permanently bypassing Case 4.

Case 4 uses pickOverload which correctly handles OVERLOAD_AMBIGUOUS and returns the sentinel when arity normalization collapses distinct overloads (see pickOverload → isOverloadAmbiguousAfterNormalization). Case 0.5 uses findOwnedMember (first overload only) + narrowOverloadCandidates([singleDef], ...). For a class with two same-arity overloads (Save(string) / Save(int)), Case 4 would return OVERLOAD_AMBIGUOUS and suppress; Case 0.5 would feed the first-registered overload and pass it through if arity matches — false positive edge.

The existing csharp-self-this-resolution test covers only the simple single-overload shape and would not catch this regression.

Production risk: For any OO language with this.Method() calls where the class has overloaded methods, Case 0.5 can emit a false positive that Case 4 would have suppressed. Because C#, TypeScript, and Java are all in MIGRATED_LANGUAGES, this is live in production paths.

Recommended fix: Language-gate Case 0.5 to C++ via a flag on ReceiverBoundProviderSubset (e.g., resolveThisViaEnclosingClass?: boolean), or prove with existing cross-language tests that OVERLOAD_AMBIGUOUS suppression still fires correctly for all migrated languages. Fix this →

Blocks merge: yes

Finding 2: findOwnedMember feeds a single overload to narrowOverloadCandidates in the this-> path

Severity: major File: gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts:333–344

Evidence:

memberDef = findOwnedMember(ownerId, memberName, model);
// findOwnedMember = model.methods.lookupAllByOwner(ownerDefId, memberName)[0]
if (memberDef !== undefined) {
  if (
    site.kind === 'call' &&
    narrowOverloadCandidates([memberDef], site.arity, site.argumentTypes).length === 0
  ) {
    memberDef = undefined;
    break;
  }
  break;
}

findOwnedMember returns lookupAllByOwner(...)[0] — the first registered overload. narrowOverloadCandidates is called with that single candidate. Two failure modes:

A (false positive): Class has f(int) and f(string). First registered is f(string). Call is this->f(42) (arity 1). narrowOverloadCandidates([f_string], 1, ['int']) — if arity passes (both have arity 1) but type doesn't narrow further, returns [f_string] → emits edge to wrong overload.

B (false miss): First registered is f(int) (arity 1). Call is this->f() (arity 0). Narrowing rejects, break — never checks f() overload (arity 0) which would match.

Case 4's pickOverload correctly calls model.methods.lookupAllByOwner(ownerId, memberName) (all overloads) and handles the full narrowing + ambiguity path. The this-> path should mirror that.

Production risk: Any C++ class with overloaded methods called via this-> will produce either false positive (wrong overload selected) or false miss (correct overload skipped).

Recommended fix: Replace findOwnedMember + narrowOverloadCandidates([single]) with pickOverload(ownerId, memberName, site, model) (already defined in the same file at line 665). This correctly handles the full overload set and returns OVERLOAD_AMBIGUOUS when warranted. Fix this →

Blocks merge: yes

Finding 3: Module-scope fallback in findCallableBindingInScope is too broad and not language-gated

Severity: major File: gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts:258–283

Evidence:

// After lexical chain walk finds nothing:
if (startFilePath !== undefined) {
  for (const scope of scopes.scopeTree.byId.values()) {
    if (scope.kind !== 'Module' || scope.filePath !== startFilePath) continue;
    // checks scope.bindings and lookupBindingsAt(scope.id, ...)
    return b.def; // first callable match wins
  }
}

Problems:

Scope boundary violation: When a template-body's lexical chain ends at a Class scope (the known bug this addresses), the fallback bypasses the scope boundary and returns any same-file module-level callable with the matching name. If a file has both a template method calling helper() and an unrelated module-level function named helper, the fallback would silently resolve to the module-level helper even if it shouldn't be visible.

Not language-gated: findCallableBindingInScope is used by emitFreeCallFallback for all languages. For TypeScript, Go, Python, and C# files where a scope chain ends early (any legitimate scope-chain gap), this fallback fires and performs a file-wide callable scan.

Non-deterministic iteration: scopes.scopeTree.byId.values() iterates a Map by insertion order, which is deterministic within a single run but ordering depends on parse traversal order. If a file has multiple top-level Module scopes (an edge case), which one wins is not guaranteed by contract.

No ambiguity detection: If two Module-scope callables with the same name exist (e.g., two overloads both registered at module scope), the fallback returns the first and ignores the second — potential false positive.

Production risk: For any large C++ file with template-heavy shapes, any unqualified call within a template body can now resolve to any same-file module-level callable with the same name. Given that C++ headers commonly declare many utility functions at module scope, this could systematically produce false CALLS edges throughout C++ projects.

Recommended fix: Narrow the fallback to be C++-specific (add a flag or call-site guard), add shadowing/ambiguity detection before returning, and add a regression test with a same-file unrelated callable and a template body call to prove it is NOT selected. Fix this →

Blocks merge: yes

Finding 4: No paired positive/negative test in the same fixture

Severity: major File: gitnexus/test/integration/resolvers/cpp.test.ts:1952–1993, fixtures cpp-two-phase-dependent-base/derived.h, cpp-two-phase-this-qualified/derived.h

Evidence: The negative test (cpp-two-phase-dependent-base) uses f() (unqualified) inside Derived<T>::g() — correctly expects 0 edges. The positive test (cpp-two-phase-this-qualified) uses this->f() inside a different Derived<T> — correctly expects 1 edge.

There is no single fixture that contains both forms side-by-side in the same class body. Without it, the test suite cannot prove that the same template body correctly handles:

• f() → unresolved (dependent base suppression intact)
• this->f() → resolved (new positive path)

The risk: if the new this-> path accidentally weakened the dependent-base suppression for unqualified calls (e.g., by seeding handledSites incorrectly), the existing tests would not catch it because they operate on separate fixtures.

Production risk: Dependent-base suppression regression could go undetected through CI.

Recommended fix: Add a third fixture (e.g., cpp-two-phase-paired) containing a template class with both f() and this->f() calls to the same base member, asserting f() emits 0 edges and this->f() emits exactly 1. Fix this →

Blocks merge: yes

Finding 5: this-> arity-mismatch breaks MRO walk (undocumented C++ name-hiding choice)

Severity: minor File: receiver-bound-calls.ts:336–343

Evidence:

if (
  site.kind === 'call' &&
  narrowOverloadCandidates([memberDef], site.arity, site.argumentTypes).length === 0
) {
  memberDef = undefined;
  break;  // stops chain walk
}

If the derived class has void f(int) and this->f() is called (arity 0), the arity check fails and the chain walk stops — Base::f() (arity 0) is never tried. In C++, name hiding semantics (a derived-class name hides all base-class overloads of the same name) would justify this break. But:

1. The PR doesn't document this as an intentional name-hiding approximation.
2. There is no test proving this behavior is correct.
3. Inconsistency with Case 2 (class-name receiver) which has the same break with an explicit comment explaining the PHP rationale — no equivalent C++ rationale here.

Recommended fix: Document explicitly with a comment referencing C++ name-hiding semantics ([basic.lookup]). Add a test for this->f() with arity mismatch at derived class and a matching overload at base class.

Blocks merge: no

Finding 6: using ns::name recovery bypasses arity narrowing for overloaded namespace functions

Severity: minor File: gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts:228–243

Evidence:

const member = resolveCppQualifiedNamespaceMember(imp.targetRaw, imp.importedName, ...);
if (seenUsing.has(member.nodeId)) continue;
seenUsing.add(member.nodeId);
usingNamedHits.push(member);

resolveCppQualifiedNamespaceMember returns one SymbolDefinition. If the namespace has ns_helper_2(int) and ns_helper_2() as separate overloads (separate nodeIds), both pass imp.importedName === 'ns_helper_2' and both get pushed → usingNamedHits.length === 2 → returns 'ambiguous' (conservative). This is safe.

If there is only one overload but its arity doesn't match the call site, the hit is returned anyway and arity is not checked here. The call will then propagate through and potentially emit a false positive if downstream narrowing doesn't catch it. This is a minor risk because resolveAdlCandidates is a last-resort path and the single-hit return is plausible.

Recommended fix: After collecting usingNamedHits, apply arity narrowing using site.arity/site.argumentTypes before returning, or document why it's acceptable to skip arity here.

Blocks merge: no

Finding 7: Test assertions use name-only matching without target file/node identity

Severity: minor File: gitnexus/test/integration/resolvers/cpp.test.ts:1982–1992

Evidence:

it('Derived<T>::g() -> this->f() resolves to f (1 edge)', () => {
  const calls = getRelationships(result, 'CALLS');
  const thisCalls = calls.filter((c) => c.source === 'g' && c.target === 'f');
  expect(thisCalls.length).toBe(1);
});

The test asserts 1 edge from g to any function named f. It does NOT assert targetFilePath === 'base.h' or verify the target nodeId. If the fixture contained another f (e.g., a module-level helper or an unrelated function), the assertion would still pass while masking a resolution error. Compare to other tests in the file that do assert targetFilePath (e.g., cpp-two-phase-dependent-base-ns at line 2036–2039).

Recommended fix: Add expect(thisCalls[0].targetFilePath).toContain('base.h') (or equivalent) to both this-> positive tests. Similarly for ns_helper and ns_helper_2 tests.

Blocks merge: no

@copilot address these findings and act as a senior c++ static analysis tool engineer and iso c++ compiler expert and follow DoD.md

[Copilot]

/autofix

Applied autofix updates in commits d681f19 and f466231 (C++-gated this path, overload/ambiguity handling, fallback removal, and expanded regression tests).

Findings

Finding 1: this branch fires for ALL languages, preempts type-binding path (Case 4)

Severity: major File: `gitnexus/src/core/ingestion/scope-resolution/passes/recei...

Addressed the blocking findings in d681f19 and f466231: Case 0.5 is now C++-gated, this resolution now uses full overload-set narrowing with ambiguity suppression and C++ name-hiding behavior, the shared module-scope callable fallback was removed, using ns::name recovery now applies arity narrowing, and paired/targeted C++ regressions were added (including positive this->f(42) and paired unqualified-vs-this checks).

[magyargergo]

@copilot make sure ci is healthy! the cpp parity test failure most likely comes from the fact that we need to skip the new tests from the legacy DAG

[Copilot]

@copilot make sure ci is healthy! the cpp parity test failure most likely comes from the fact that we need to skip the new tests from the legacy DAG

Done in 5d9f335: I added the new C++ template this->/paired two-phase assertions to the cpp legacy-parity expected-failure skip list (test/integration/resolvers/helpers.ts) so they run in registry-primary mode but are skipped under REGISTRY_PRIMARY_CPP=0. I also verified locally: REGISTRY_PRIMARY_CPP=0 npx vitest run test/integration/resolvers/cpp.test.ts passes, and default npx vitest run test/integration/resolvers/cpp.test.ts also passes.