fix(cpp): resolve cross-namespace same-tail inheritance bases bridge-held (#1993)

[magyargergo]

Stacked PR 3/5 — base fix/1995-cpp-union-anon-ns-collision (#2004). Auto-retargets when #2004 merges.

Summary

Follow-up to #1981 / #1982. PR #1981 landed a targeted bridge (tagNamespacePrefixes + a resolveDefGraphId namespace-prefixed retry) that fixes the within-namespace same-tail heritage case (NS::A::Inner vs NS::B::Inner, distinguished by the class chain). The residual: a cross-namespace same-tail base (NS1::A::Inner vs NS2::A::Inner) — both key the namespace-omitted A.Inner in the qualifiedNames resolution index, so resolveQualifiedInheritanceBase could not pick a winner and the deriving classes cross-wired (DB's EXTENDS bound to NS1's A::Inner).

Fixed bridge-held — without flipping the class-chain-only qualifiedName invariant (which would re-bite two-phase lookup / UDC / brace-init) and without re-keying the resolution index.

Fix

The namespacePrefix sidecar (set by tagNamespacePrefixes, deliberately separate from qualifiedName) already records each namespaced def's enclosing namespace. Two sidecar-only changes:

1. tagNamespacePrefixes now also tags defs declared directly in a namespace (e.g. the deriving NS1::DA), composed identically to the existing class-nested path. The original loop only reached class-nested defs (A::Inner).
2. resolveQualifiedInheritanceBase, on a same-tail tie at a key, breaks the tie by preferring the candidate whose namespacePrefix matches the deriving class's — the base in the same enclosing namespace. It still refuses when the sidecar can't pick a unique.

def.qualifiedName and the qualifiedNames index keys are untouched, so two-phase lookup, UDC ranking, brace-init, and file-local linkage are unaffected. The deferred root-cause work (making qualifiedName namespace-inclusive and removing the bridge) is unchanged and out of scope.

Tests

New cpp-cross-namespace-same-tail fixture + test (registry-primary, in the cpp parity expected-failures like the #1982 namespaced tests): NS1.DA EXTENDS NS1.A.Inner and NS2.DB EXTENDS NS2.A.Inner — positive base-identity by the resolved node's qualifiedName, which fails pre-fix (DB cross-wired to NS1.A.Inner).

Verification

• cpp resolver suite: registry-primary 287/287, legacy green — no regression to brace-init, UDC, two-phase lookup, ADL, file-local linkage, or the within-namespace (Qualified nested-type node identity — resolution-side follow-up (Ruby same-tail routed-props/mixins + Rust inherent impls) #1982) namespaced-collision tests.
• tsc --noEmit clean; prettier clean.

Closes #1993.

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	Jun 4, 2026 9:08am

[magyargergo]

Tri-review — approve-with-nits (1 blocker being addressed in-PR)

Method: per-PR multi-lens review → independent adversarial re-verification → static-analysis alignment (tsc/eslint/prettier, all clean) → cross-stack synthesis + critic gate.

Review: PR #2005 — C++ cross-namespace same-tail heritage tie-break (#1993)

Verdict: approve-with-nits. The fix is correct on the real runtime path, language-neutral, well-contained, and additive. The one thing I'd want before merge is a worker-path parity test, to match the precedent set by both predecessor lanes in this stack and the DoD's sequential≡worker requirement.

What's good

• Sound mechanism, minimal blast radius. Sidecar-only: def.qualifiedName and the qualifiedNames index keys are untouched, so two-phase lookup / UDC / brace-init / file-local linkage are unaffected. The deriving-side tag (namespace-direct → NS1) and base-side tag (class-nested → NS1) compose identically and compare equal; nsCount===1 selects, otherwise refuse-on-tie is preserved.
• Object-identity invariant holds. buildDefIndex/buildQualifiedNameIndex are built from file.localDefs, which share instances with Scope.ownedDefs (documented at walkers.ts:763), and the mutation runs in populateOwners before finalize — so the sidecar is visible via scopes.defs.
• Non-overlapping loops. Each def is owned by exactly one scope; the class-nested loop skips Namespace scopes and the new loop only processes them, plus the new loop guards namespacePrefix !== undefined before writing. No double-tag.
• Language-neutrality preserved. The shared walkers.ts change names no languages — it gates on kind==='Namespace'/type==='Namespace' (namespace-free languages are a no-op). C++ activation stays behind the populateOwners provider hook (AGENTS.md:42). ✅
• Positive identity assertion + dual parity legs. The test resolves the EXTENDS source by qualifiedName and asserts the target's qualifiedName (not dangle-only), and discriminates the cross-wire. createResolverParityIt('cpp') + the byte-matching expected-failures entry (helpers.ts:553 ↔ cpp.test.ts:4213) give both registry-primary and legacy legs. tsc clean; no CHANGELOG/golden/snapshot touched.

Findings

Major

• Missing worker-path parity test (cpp.test.ts:4200-4216). Only the sequential path is tested. The Qualified nested-type node identity — resolution-side follow-up (Ruby same-tail routed-props/mixins + Rust inherent impls) #1982 namespaced fix (cpp.test.ts:4148) and the fix(ingestion): C++ same-tail nested types in sibling unions / anonymous namespaces still merge into one node (#1978 fan-out U1 retro-fix) #1995 union/anon fixes (3961/4032) each shipped a worker-path describe with a usedWorkerPool===true guard + worker-path EXTENDS assertion. The DoD requires sequential≡worker parity, and the EXTENDS target node (NS1.A.Inner) is keyed worker-side. Please add a mirrored worker-path block (workerThresholdsForTest:{minFiles:1,minBytes:1}, workerPoolSize:2) and its name to the legacy expected-failures set.

Minor

• Narrative contradiction on the pre-fix failure mode (cpp-cross-namespace-same-tail/main.cpp:1-8). The fixture says "silent miss, not a miswire"; the PR summary + test comment say DB "cross-wired to NS1.A.Inner." Pre-fix, the count>1 branch returns undefined (DA untagged) → simple-name fall-through, whose outcome could be a miss or a mis-bind. Both fail the assertion pre-fix, so fail-pre/pass-post holds — but align the three descriptions.

Nits

• Widened tagging now sets namespacePrefix on all namespace-direct defs (functions/variables), changing resolveDefGraphId (ids.ts:152-156) node-id resolution — but only as a more-precise retry before the unchanged simpleKey fallback (improvement/no-op, never a regression), and it's covered by the 98 existing namespace cpp fixtures incl. the full ADL suite. No fix(cpp): scope-model def.qualifiedName drops the enclosing namespace (root cause behind #1982's targeted heritage bridge) #1993-owned test pins it.
• Untyped sidecar via as casts (walkers.ts:394-404) — consistent with the pre-existing Qualified nested-type node identity — resolution-side follow-up (Ruby same-tail routed-props/mixins + Rust inherent impls) #1982 convention; consider typing namespacePrefix? on SymbolDefinition in a follow-up to drop the casts repo-wide.

Nice, surgical fix that stays within the established sidecar bridge rather than flipping the qualifiedName invariant.

[magyargergo]

✅ Blocker resolved + narrative corrected (review follow-up)

Worker-path parity test added (355f08df). The #1993 describe now ships the missing parse-worker.ts sibling — mirrors the #1982/#1995 worker blocks (workerThresholdsForTest {minFiles:1,minBytes:1}, workerPoolSize:2, a usedWorkerPool===true guard, and the same NS1.DA→NS1.A.Inner / NS2.DB→NS2.A.Inner base assertions). Both worker test names are registered in LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES['cpp'] (registry-primary-only, like the sequential entry).
Verified: primary 3/3 (worker pool genuinely used + correct resolution), legacy 217 passed / 80 skipped (the two new worker its skip cleanly). DoD sequential≡worker now holds.

Narrative correction — the review had this backwards. The synthesis recommended changing the PR body from "cross-wire" to "silent miss". The empirical pre-fix run is a cross-wire, not a silent miss: baseQnOf('NS2.DB') returned 'NS1.A.Inner' — an existing EXTENDS edge to the wrong target (a true silent miss would have tripped the toBeDefined() guard first). Pre-fix, resolveQualifiedInheritanceBase refuses-on-tie and the scope-walk fallback first-wins to NS1's Inner, so DB binds NS1::A::Inner (DA resolves correctly only by that first-wins luck). So the PR body was already correct; I corrected the fixture/test comments (which wrongly said "silent miss") to match — same commit.

Re-stacked: #2006/#2007 rebased onto the updated #1993; chain stays linear, all PRs MERGEABLE.

[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
11134	11118	0	16	697s

✅ All 11118 tests passed

16 test(s) skipped — expand for details

• COBOL pipeline benchmark > scales with file count
• C++ ADL emit benchmark > emit phase scales sub-quadratically with co-scaled files and sites
• C++ pipeline benchmark > scales with file count
• C# pipeline benchmark > scales with file count — namespaces spread across the solution
• C# pipeline benchmark > scales with file count — all types in one (global) namespace bucket
• C# pipeline benchmark > scales with file count — all types in one (named) namespace bucket
• Go pipeline benchmark > scales with file count (workers enabled)
• Go pipeline benchmark — worker pool (issue Worker idle timeout kills long Go scope extraction and surfaces as Napi::Error during analyze #1848) > does not quarantine the large generated Go file on sub-batch idle timeout
• Go structural interface detection benchmark > scales linearly with interface × struct count
• Go structural interface detection split-phase benchmark > separates index-build and detection time
• PHP pipeline benchmark > scales with file count (workers enabled)
• Ruby pipeline benchmark > scales with file count (workers enabled)
• Rust pipeline benchmark > scales with file count (workers enabled)
• Vue pipeline benchmark > scales with component count
• run.cjs direct-exec entrypoint (fix(cli): steer docs, skills, and hooks through a CLI-neutral project-local runner (#1939) #1945) > resolves a .cmd shim via the Windows shell branch, passing args and exit code
• 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.38%	38911/48407	N/A%	—	🟢 ████████████████░░░░
Branches	68.96%	24752/35892	N/A%	—	🟢 █████████████░░░░░░░
Functions	85.6%	4049/4730	N/A%	—	🟢 █████████████████░░░
Lines	83.98%	35010/41687	N/A%	—	🟢 ████████████████░░░░

---

_{📋 View full run · Generated by CI}