fix(cpp): thread call-site types into qualified member lookup (#1632)

[prajapatisparsh]

Summary

Closes #1632. Part of #1564 (LANG-cpp).

Thread call-site arity and argument types through the resolveQualifiedReceiverMember contract so narrowOverloadCandidates can disambiguate inline-namespace children with distinct signatures (e.g. foo(int) vs foo(double)).

Changes

• gitnexus-shared/.../types.ts — Widened Callsite: arity made optional, argumentTypes added
• scope-resolution/contract/scope-resolver.ts — Added optional callsite?: Callsite to resolveQualifiedReceiverMember
• receiver-bound-calls.ts — Passes site (ReferenceSite) through structural typing
• languages/cpp/scope-resolver.ts — Forwards callsite in the lambda
• languages/cpp/inline-namespaces.ts — Accepts callsite?, imports cppConversionRank, threads both into narrowOverloadCandidates
• test/integration/resolvers/cpp.test.ts — Flipped foo(int) vs foo(double) fixture from 0→1 edge (verified by startLine: 4 for v1::foo), added normalization-ambiguity fixture (int vs long, stays at 0 edges)
• test/fixtures/.../cpp-inline-namespace-ambiguous-normalized/ — New fixture

Behavior

Scenario	Before	After
outer::foo(42) — v1 has foo(int), v2 has foo(double)	0 edges (conservative suppress)	1 edge to v1::foo
outer::foo() — identical sigs in both children	0 edges	0 edges (preserved)
outer::foo(42) — foo(int) vs foo(long) (normalize to same type)	0 edges	0 edges (new negative test)
ADL using-import path	pass-through	unchanged (callsite undefined)

Testing

✓ 241/241 tests pass (registry-primary C++ resolver)
✓ pre-commit hooks (eslint, prettier, typechecking) passed

Notes

conversionRankFn is imported directly in inline-namespaces.ts (not threaded through the contract) — keeps the contract language-agnostic.

[vercel]

@prajapatisparsh is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Claude finished @magyargergo's task in 6m 37s —— View job

---

PR #1810 Review — C++ inline namespace call-site types

• Post initial comment
• Read repo docs (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, DoD.md)
• Inspect PR diff (git diff HEAD2..HEAD1)
• Search for legacy parity / stale expected-failure entries
• Search all resolveQualifiedReceiverMember call sites and implementers
• Search Callsite usages for arity optional-ness risk
• Read changed source files in detail
• Read test files and fixtures
• Analyze runtime path correctness
• Run targeted rg searches (legacy, narrowOverloadCandidates, cppConversionRank)
• Compile findings and post full review

---

Review bar

Per DoD.md §2.7, GUARDRAILS.md (no false positives without sign-off), and AGENTS.md (graph-safe under-approximation): mergeable only when the runtime path is fully wired, ambiguity suppression is preserved, the shared contract change doesn't break other resolvers, and legacy-parity infrastructure is kept consistent. The C++ resolver's stated posture is that false-positive CALLS edges are worse than false negatives.

---

Current PR state

Branch: fix/1632-inline-namespace-callsite-types
Commits: 2 (fix(cpp): thread call-site types… + merge-back from main)
Diff stat: 8 files, +71/-24

Could not run npm test or tsc --noEmit (no package manager access in sandbox). Evidence gathered from static analysis, source reading, and targeted rg searches.

---

Understanding of the change

The full runtime path for outer::foo(42) with v1::foo(int) / v2::foo(double):

1. Capture (captures.ts:233): inferCppCallArgTypes on the call_expression AST node infers 42 → ['int']. This is written into grouped['@reference.parameter-types'].
2. Extraction (scope-extractor.ts:943): extractArgumentTypes parses the JSON capture into argumentTypes: ['int']. The ReferenceSite carries { arity: 1, argumentTypes: ['int'] }.
3. Dispatch (receiver-bound-calls.ts:498): provider.resolveQualifiedReceiverMember is called with site (the full ReferenceSite) as the new callsite? argument. Structural typing satisfies Callsite since ReferenceSite has optional arity? and argumentTypes?.
4. Forwarding (cpp/scope-resolver.ts:278): callsite is threaded into resolveCppQualifiedNamespaceMember.
5. Collection (inline-namespaces.ts): findMemberInNamespaceTransitive collects both v1::foo(int) and v2::foo(double) → allHits.length === 2.
6. Narrowing: narrowOverloadCandidates([v1::foo, v2::foo], 1, ['int'], { conversionRankFn: cppConversionRank }):
   • Arity filter: both have parameterCount=1 → both survive.
   • Exact-type filter: 'int' === 'int' matches v1::foo(int), 'int' === 'double' fails → typed = [v1::foo], typed.length > 0 → result = [v1::foo].
7. narrowed.length === 1 → return allHits[0] → 1 CALLS edge to v1::foo(int) at line 4.

For outer::foo(42) with v1::foo(int) / v2::foo(long): normalizeCppParamType collapses long → 'int', so exact-type filter produces typed = [v1::foo, v2::foo]. typed.length > 1 → falls to isOverloadAmbiguousAfterNormalization → detects collision → return 'ambiguous' → suppressed. ✓

For outer::foo() (same sig both children): callsite is absent or arity=0, narrowing produces both candidates → isOverloadAmbiguousAfterNormalization → suppressed. ✓

---

Findings

Finding 1 — Stale LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES entry + missing new entry

Severity: BLOCKING

Evidence:

helpers.ts:339 (unchanged by this PR) still contains the old test name:

'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(double)',

PR #1810 renamed this test to:

'outer::foo(42) emits exactly 1 CALLS edge to v1::foo(int) when v1 declares foo(int) and v2 declares foo(double)'

cpp.test.ts:19 uses const it = createResolverParityIt('cpp'), so every it(...) is wrapped by isLegacyResolverParityExpectedFailure. Under REGISTRY_PRIMARY_CPP=0:

• The old entry string matches no test → stale dead entry
• The new test name is not in the skip list → test runs in legacy mode
• The legacy DAG has no inline-namespace resolver and no call-site type threading → it would emit 0 edges (or an arbitrary 1 edge not guaranteed to be v1::foo at line 4)
• Test expects toHaveLength(1) AND startLine === 4 → FAILS in legacy mode

Additionally, the new normalized fixture test (cpp-inline-namespace-ambiguous-normalized) also has no entry in the expected-failure list:

'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int'

The legacy path would pick arbitrarily (1 edge) while the test expects 0 → also fails in legacy mode.

The PR description states ✓ 241/241 tests pass without mentioning REGISTRY_PRIMARY_CPP=0. The PR omitted the legacy parity run.

Why it matters: DoD §2.1 requires the real runtime path to be verified, and issue #1564's posture requires both modes to be consistent. A CI run under REGISTRY_PRIMARY_CPP=0 will fail, breaking parity test gating.

Recommended fix: In helpers.ts, inside LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp:

1. Remove the old stale entry (line 339)
2. Add both new entries:

// PR #1810: call-site type threading resolves diff-sigs case in registry-primary;
// legacy DAG has no inline-namespace resolver and no call-site type info.
'outer::foo(42) emits exactly 1 CALLS edge to v1::foo(int) when v1 declares foo(int) and v2 declares foo(double)',
// Normalized-signature suppression (int vs long) is scope-resolver-only;
// legacy path picks an arbitrary match.
'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int',

Blocks merge: yes

---

Finding 2 — Stale describe block name contradicts new test behavior

Severity: LOW

Evidence:

cpp.test.ts:3047:

describe('C++ inline namespace — ambiguous distinct signatures (conservative suppress)', () => {

This describes a "conservative suppress" behavior, but the describe block now contains a test asserting exactly 1 CALLS edge (not a suppress). The old name made sense when the test documented a limitation; after the fix it's a misnomer.

createResolverParityIt matches on it(...) names, not describe(...) names, so this has no correctness impact. But a reader will be confused by a describe named "conservative suppress" that contains a test asserting successful resolution.

Recommended fix: Rename the describe block to 'C++ inline namespace — distinct signatures resolved via call-site types'.

Blocks merge: no

---

Finding 3 — Callsite.arity optional-ness: runtime safe but silently weakens c/arity.ts

Severity: LOW

Evidence:

c/arity.ts:14:

if (!Number.isFinite(callsite.arity) || callsite.arity < 0) return 'unknown';

After this PR, callsite.arity is number | undefined. Number.isFinite(undefined) returns false, so the guard immediately returns 'unknown' when arity is undefined — correct at runtime. Lines 17–18 are only reached when arity is a finite non-negative number.

TypeScript does not narrow via Number.isFinite, so strictly speaking lines 17–18 operate on number | undefined. However gitnexus/tsconfig.json has strict: false, so this does not cause a compile error.

The arityCompatibility hook on ScopeResolver takes callsite: Callsite and is called from the Registry/RegistryProviders path (not from resolveQualifiedReceiverMember). The existing callers construct Callsite objects only when arity is known (e.g., { arity: argCount } after an !== undefined guard at overload-narrowing.ts:191–192). So in practice cArityCompatibility would not receive undefined arity in existing call sites. No behavior change for existing consumers.

Why it matters: Low risk, but weakening Callsite.arity to optional without a thorough audit of all Callsite consumers (including future ones) could allow silent degradation in other arityCompatibility implementations. The change is justified for the inline-namespace path but the comment scope on the Callsite interface is now accurate ("if available").

Recommended fix: None required, but consider adding a clarifying comment to cArityCompatibility noting that undefined arity is handled by the Number.isFinite guard.

Blocks merge: no

---

Finding 4 — cppConversionRank direct import: architecture is correct

Severity: NONE

inline-namespaces.ts is C++-specific code (languages/cpp/). The direct import of cppConversionRank from ./conversion-rank.js is architecturally correct — it does not leak into shared ingestion code and keeps the ScopeResolver contract language-agnostic. The contract accepts Callsite (shared, language-agnostic); conversion-rank scoring is applied inside the C++ implementation without affecting the contract shape.

No circular dependency: conversion-rank.ts is a pure utility with no upstream imports from the resolver. The conversionRankFn on ScopeResolver (PR #1606) is a different provider hook; inline-namespaces.ts correctly bypasses it because this code is already inside the C++ resolver. Tests use the full pipeline (not mocks), so no testability issue arises.

---

Finding 5 — callsite !== undefined guard appropriateness

Severity: NONE

callsite !== undefined ? { conversionRankFn: cppConversionRank } : undefined

When callsite is provided but callsite.argumentTypes is undefined (e.g., argument types couldn't be inferred from non-literal expressions), narrowOverloadCandidates skips the exact-type filter and attempts conversion-rank ranking with argTypes = undefined. In rankByConversion, when argTypes is undefined, the ranking does nothing useful. The multi-candidate result falls through to isOverloadAmbiguousAfterNormalization or the final return 'ambiguous' — the conservative suppress path. This is correct behavior per the graph-safety posture.

---

Finding 6 — ambiguous sentinel handling from #1600 is unaffected

Severity: NONE

The memberDef === 'ambiguous' check at receiver-bound-calls.ts:506 is strict-equality checked before if (memberDef !== undefined). The sentinel handling path is unchanged by this PR. handledSites is still marked to prevent fallback. The using-import/ADL recovery path still skips correctly because it only fires when the qualified-namespace hook returns undefined.

---

Required follow-up tests

The only blocking gap is the legacy parity infrastructure:

1. Must add: 'outer::foo(42) emits exactly 1 CALLS edge to v1::foo(int) when v1 declares foo(int) and v2 declares foo(double)' to LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp in helpers.ts.
2. Must add: 'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int' to the same list.
3. Must remove: the stale old entry at helpers.ts:339.
4. Must verify: run REGISTRY_PRIMARY_CPP=0 npx vitest run test/integration/resolvers/cpp.test.ts before merge.

Recommended but non-blocking:

• Rename the describe block (Finding 2) to remove "conservative suppress".
• Add a structured-resolution-outcome assertion alongside the positive test (matching the pattern used in cpp-inline-namespace-ambiguous's companion test that checks kind: 'suppressed'; here the outcome would be a resolved edge, not a suppression).

---

Merge recommendation

Request changes — one blocking finding.

The core logic (runtime wiring, narrowing, ambiguity preservation, contract change) is correct and well-designed. The positive test is mutation-resistant: removing site from the resolveQualifiedReceiverMember call degrades to the conservative-suppress path, causing the test to fail at toHaveLength(1). The ISO C++ semantics approximation is honest. The cppConversionRank direct import is architecturally appropriate.

The only thing blocking merge is: helpers.ts was not updated. The old expected-failure string is now dead, and both new test names run unguarded under REGISTRY_PRIMARY_CPP=0. This will produce a legacy-parity CI failure. The fix is three lines in helpers.ts plus a REGISTRY_PRIMARY_CPP=0 test run.

Fix this →

[magyargergo]

@prajapatisparsh could you please look into claude's findings?

[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
9710	9708	0	2	589s

✅ All 9708 tests passed

2 test(s) skipped — expand for details

• PHP pipeline benchmark > scales with file count (workers enabled)
• 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	80.09%	33083/41307	N/A%	—	🟢 ████████████████░░░░
Branches	68.62%	21168/30845	N/A%	—	🟢 █████████████░░░░░░░
Functions	84.9%	3430/4040	N/A%	—	🟢 ████████████████░░░░
Lines	83.48%	29785/35679	N/A%	—	🟢 ████████████████░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

Nice! 🔥

[magyargergo]

Thank you! Feel free to pickup more :)

[prajapatisparsh]

Thank you! Feel free to pickup more :)

Sure