fix(ingestion): #1981 stack follow-ups — Rust method-id collision (F3) + C++ sidecar typing (F4) + Ruby Trait predicate (F5)#2022
Closed
magyargergo wants to merge 9 commits into
Conversation
…d-qualified Impl node (abhigyanpatwari#1992) A generic inherent-impl target (`impl<T> Inner<T>`) is a `generic_type` node, which the inherent-impl owner walk (findEnclosingClassInfo) did not match — so the walk returned null and the method got `File -> DEFINES` with NO HAS_METHOD edge (orphaned, and invisible to findDanglingEdges). The Impl node was already correctly mod-qualified (the @name capture drills into the inner type_identifier, tree-sitter-queries.ts), so this is an owner-walk-only fix: drill into the generic base and mirror the node gate so the owner id == the node id byte-for-byte. A scoped-generic target (`impl<T> a::Inner<T>`) materializes no Impl node and is left orphaned (deferred) rather than minting a phantom owner. The owner walk is shared by the sequential and worker paths. New fixture + tests assert positive HAS_METHOD ownership through distinct `a.Inner` / `b.Inner` nodes on both resolver legs and the worker path, plus a negative scoped-generic guard. rust-captures-golden regenerated additively for the new fixture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bhigyanpatwari#1995) `union_specifier` was missing from cppClassConfig.ancestorScopeNodeTypes, so a struct nested in `union U1` and one in `union U2` both qualified to the bare `Inner` and merged onto one Struct:...:Inner node — from_u1/from_u2 cross-wired (invisible to findDanglingEdges). Adding `union_specifier` lets buildQualifiedName pick up the named union's `name` segment, materializing distinct `U1.Inner` / `U2.Inner` nodes. Anonymous unions have no `name` child and correctly contribute nothing (members inject into the enclosing scope); the separate C config is untouched. New fixture + positive-identity tests (sequential + worker, both legs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es (abhigyanpatwari#1995) An anonymous `namespace { }` is a namespace_definition with no `name` child, so the scope walker dropped it (empty segment) and two `namespace { struct Inner {} }` blocks in one TU collapsed onto a single `Inner` node — from_anon_a/from_anon_b cross-wired. A C++ `extractScopeSegments` override (the first consumer of the existing config hook) gives each anonymous namespace a deterministic per-block discriminator from its start byte, keeping the nested types distinct. Named scopes (incl. `inline namespace`) and anonymous unions are unaffected. Deterministic across the sequential and worker full-file parses. New fixture + tests assert node DISTINCTNESS (count==2 / distinct owners), not the non-portable discriminator value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…held (abhigyanpatwari#1993) PR abhigyanpatwari#1981's bridge fixed within-namespace same-tail heritage (NS::A::Inner vs NS::B::Inner). 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 index, so resolveQualifiedInheritanceBase couldn't pick a winner and the deriving classes cross-wired (DB's EXTENDS bound to NS1's A::Inner). Fixed bridge-held via the existing `namespacePrefix` sidecar — no qualifiedName invariant flip, no resolution-index re-keying: (1) tagNamespacePrefixes also tags defs declared directly in a namespace (the deriving NS1::DA), composed identically to the class-nested path; (2) resolveQualifiedInheritanceBase breaks a same-tail tie by preferring the candidate whose namespacePrefix matches the deriving class's. Two-phase lookup, UDC, brace-init, file-local linkage untouched (def.qualifiedName + index keys unchanged). New cpp-cross-namespace-same-tail fixture + registry-primary test (in the cpp parity expected-failures). Verified: cpp suite 287/287 primary, 209 + 78 skips legacy — no regression; tsc + prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e tie-break + correct narrative Add the missing parse-worker.ts parity describe for the abhigyanpatwari#1993 cross-namespace same-tail heritage tie-break, mirroring the abhigyanpatwari#1982/abhigyanpatwari#1995 worker siblings (workerThresholdsForTest minFiles:1/minBytes:1, workerPoolSize:2, usedWorkerPool guard, and the same NS1.DA→NS1.A.Inner / NS2.DB→NS2.A.Inner base assertions), and register both worker test names in LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES['cpp'] (registry-primary-only, like the sequential entry). Closes the DoD sequential≡worker gap flagged in the tri-review of PR abhigyanpatwari#2005. Also correct the fixture/test narrative: the pre-fix failure is a CROSS-WIRE (DB's EXTENDS binds NS1::A::Inner via the refuse-on-tie scope-walk fallback), not a silent miss — the empirical pre-fix run shows the edge exists but points at the wrong target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…MPLEMENTS by scope (abhigyanpatwari#1991) A Ruby `module` maps to the Trait label but is not a typeDeclaration, so the structure phase never qualified its node id: two same-tail nested mixin modules (App::Loggable / Web::Loggable) collapsed onto one Trait:f.rb:Loggable node and the bare-name `include Loggable` cross-wired IMPLEMENTS (first-wins tail). Structure phase: expose buildQualifiedName as a `qualifyScopeName` ClassExtractor hook and thread it for Trait nodes in parsing-processor + parse-worker (lockstep), so a module node keys by its qualified scope path (App.Loggable). Not Option A — `Trait` is not in CLASS_LIKE_LABELS and the qualified-id selection gates it out; qualifyScopeName bypasses the typeDeclaration gate that makes extractQualifiedName bail on modules. getQualifiedOwnerName also falls back to qualifyScopeName so methods inside a nested module own through the same qualified Trait id (no dangling HAS_METHOD). Resolution: emitRubyMixinEdges resolves a bare mixin reference lexically by the including class's enclosing scope (`App::S` + `Loggable` -> `App::Loggable`), and the simple-tail fallback is now delete-on-collision (refuse to guess on a same-tail tie) instead of first-wins. New single-file fixture + tests: two distinct Trait nodes, S IMPLEMENTS App.Loggable only, T IMPLEMENTS Web.Loggable only, no dangling HAS_METHOD; both resolver legs + worker path. Module->Trait preserved; Trait NOT added to CLASS_LIKE_LABELS. ruby-captures-golden regenerated additively. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ers (Ruby + Dart) (abhigyanpatwari#1994) The Ruby and Dart heritage/property pipelines encoded side-effect facts as ':'-delimited synthetic-import marker strings, hand-constructed and hand-parsed at ~8 sites with the field layout kept in agreement only by a comment — the fragility behind the abhigyanpatwari#1981 edge-drop. Route every site through a single shared codec (utils/heritage-marker.ts: encodeMarker / decodeMarker / isHeritageMarker). encodeMarker throws on a colon-bearing field so the silent-drop class becomes a loud failure; the ':' wire format is preserved byte-for-byte (ruby-captures-golden unchanged). Language-neutral — keyed only on the literal shared prefixes. Dart already single-sources its prefix and is heritage-only, so its import-target guard is left untouched (no invented __property__ path). Pure refactor: no new edges or behavior. Verified: new codec unit test; ruby resolver + golden 155/155 (zero golden diff) and dart resolver 63/63 on registry-primary, both green on legacy; tsc + prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dec (abhigyanpatwari#1994) Alias DART_HERITAGE_PREFIX to HERITAGE_MARKER_PREFIX (utils/heritage-marker.ts) instead of re-declaring the '__heritage__:' literal, so the Dart import-target heritage guard cannot desync from the codec's encode/decode. Value-identical; gives the codec prefix a direct production consumer. Addresses the tri-review nit on PR abhigyanpatwari#2007. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…method-id collision, C++ sidecar typing, Ruby Trait predicate Follow-up to the abhigyanpatwari#1991–abhigyanpatwari#1995 stack (PRs abhigyanpatwari#2003–abhigyanpatwari#2007), landed on top of abhigyanpatwari#2007 so the deferred §5 polish items don't cascade a re-rebase through the lower PRs. F3 (abhigyanpatwari#1992) — fix a latent same-tail Rust method-node collision. Two same-tail generic inherent impls under sibling mods that ALSO share a method name (`mod a { impl Inner { fn m } }` + `mod b { impl Inner { fn m } }`) both keyed `Function:…:Inner.m#0` and collapsed onto one node (graph addNode is first-write-wins), silently dropping the second. The owner Impl `classId` was already mod-qualified, masking the collision behind distinct HAS_METHOD sources. Qualify `className` (`a.Inner` / `b.Inner`) in the bare inherent-impl arm so the method node id inherits the mod scope (`a.Inner.m` / `b.Inner.m`). Symmetric: the call-resolution fallback rebuilds `${className}.${name}` from the same enclosing-impl walk, so definition and call ids still agree, and the HAS_METHOD owner anchors on the (unchanged) qualified `classId`. New same-method-name fixture + sequential & worker-parity tests; holds on both resolver legs. F4 (abhigyanpatwari#1993) — declare `namespacePrefix` on `SymbolDefinition` (gitnexus-shared) and drop the six `as { namespacePrefix?: string }` casts in walkers.ts / ids.ts. Pure type-level: the casts erase at compile time, runtime is byte-identical, and the field stays a sidecar (no graph-node identity; qualifiedName index untouched). F5 (abhigyanpatwari#1991) — single-source the four hardcoded `nodeLabel === 'Trait'` checks behind `isQualifiableScopeLabel()` in ast-helpers.ts so the sequential and worker definition paths can't drift. Value-identical predicate. Verified: gitnexus-shared build + src tsc clean; rust resolver suite both legs + worker parity (178/178 default, 176+2 skip legacy) incl. new F3 tests; ruby+cpp registry-primary 446/446; rust-coverage + rust-scope 42/42; eslint 0 errors; prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@magyargergo is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
f2582ec to
6ec1ef3
Compare
Collaborator
Author
|
Closed as superseded — the three findings here were folded into their home PRs and merged via the stack: F3 (Rust same-tail method-id collision) → #2003, F4 ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the deferred §5 follow-ups from the #1981 stack tri-review (
docs/plans/2026-06-03-006-pr-1981-stack-tri-review-action-plan.md). Stacked on top of #2007 (fix/1994-shared-marker-codec) on purpose: the action plan parked these as post-merge tickets because applying them to the lower lanes (#2003/#2005/#2006) would cascade a re-rebase of every PR above them. Landing them here touches none of the stacked PRs.F3 (#1992) — latent same-tail Rust method-node collision · the one real bug
Two same-tail generic inherent impls under sibling mods that also share a method name collapsed onto one graph node:
The method node id keys
${className}.${name}; the bare inherent-impl arm setclassNameto the bare tail (Inner), so bothms keyedFunction:lib.rs:Inner.m#0and the second was silently dropped (graph.addNodeis first-write-wins). The owner ImplclassIdwas already mod-qualified, so the two HAS_METHOD sources stayed distinct — which is exactly what masked the collision (sourceId-only assertions passed while both edges pointed at the same surviving target node).Fix: qualify
className(a.Inner/b.Inner) in the baretype_identifierimpl arm so the method node id inherits the mod scope (a.Inner.m/b.Inner.m). Symmetric by construction — the call-resolution fallback (call-processor.ts,parse-worker.ts) rebuilds${className}.${name}from the same enclosing-impl walk, so definition and call ids still agree, and HAS_METHOD anchors on the unchanged qualifiedclassId. Thefor-target and scoped arms are deliberately untouched (theirclassNamemust stay bare/raw to match their node ids).Scope note: this is graph-view-only — registry-primary call resolution was already correct (it keys by mod-qualified
ownerId+ simple name), so no call was ever mis-resolved. Newrust-generic-impl-same-method-namefixture + sequential & worker-parity tests assert two distinct nodes survive.F4 (#1993) — type the
namespacePrefixsidecar, drop the castsDeclared
namespacePrefix?: stringonSymbolDefinition(gitnexus-shared) and removed the sixas { namespacePrefix?: string }casts inwalkers.ts/graph-bridge/ids.ts. Pure type-level — theasassertions erase at compile time, emitted JS is byte-identical, and the field remains a sidecar (no graph-node identity; thequalifiedName-keyed index is untouched).F5 (#1991) — single-source the Ruby
Traitscope-label predicateReplaced the four hardcoded
nodeLabel === 'Trait'checks (two each in the sequentialparsing-processor.tsand workerparse-worker.tsdefinition paths) with oneisQualifiableScopeLabel()inast-helpers.ts, so the lockstep paths can't drift. Value-identical predicate.Not in this PR
impl Inner<T>collapse) #1992 language-neutrality) — moving the Rustimpl_itemowner walk behind a provider hook is not a clean move (the existingresolveEnclosingOwneris aSyntaxNoderemapper that can't carryEnclosingClassInfo; a correct version needs a newresolveEnclosingClassInfohook threaded through 8 call sites — 2 of which have no provider in scope — plus Go, which is hardcoded in the same function). Split out as Extract Rust/Go inherent-impl enclosing-owner resolution behind a LanguageProvider hook (#1992 follow-up, tri-review F1) #2021 for a dedicated, parity-tested refactor.rust.test.tsfix(ingestion): qualify Rust GENERIC inherent-impl target by mod scope (same-tailimpl Inner<T>collapse) #1992 worker block). No gap, no change needed.Verification
gitnexus-sharedbuild +gitnexussrctsc — clean (0 errors)rust-coverage+rust-scope: 42/42!style warnings only); prettier cleanPost-merge
F3 changes Rust inherent-impl method node ids (
Inner.m→a.Inner.m), so the embeddings re-key note from the action plan applies — fold into the same clean re-analyzethe stack already calls for.🤖 Generated with Claude Code