From 21744bdbfa90ab80a39efa6e8fbcc63200e65f4e Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 17 Apr 2026 11:26:22 +0100 Subject: [PATCH] feat(ingestion): add Ruby kind-aware MRO strategy and ancestry views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds kind-aware MRO infrastructure for Ruby's prepend/include/extend semantics. The heritage map now preserves declaration kind and exposes split views for instance vs singleton dispatch; `lookupMethodByOwnerWithMRO` gets a new `'ruby-mixin'` MroStrategy that walks prepend providers before the direct owner lookup, honoring Ruby's rule that prepended modules beat the class's own method of the same name. Changes: - gitnexus-shared: `MroStrategy` union gains `'ruby-mixin'` with doc block - model/heritage-map: `directParents` internal shape changes from `Set` to ordered `ParentEntry[]` with `{ parentId, kind }`. Public `getParents`/`getAncestors` contract preserved (still returns flat string[] via dedup). New methods: `getParentEntries`, `getInstanceAncestry`, `getSingletonAncestry`. Insertion order mirrors tree-sitter match order, which for Ruby matches source declaration order. - model/resolve: new `ruby-mixin` branch walks prepend parents first (reverse declaration — last-prepended wins), then direct owner, then extends + include parents (reverse declaration). Non-Ruby strategies (`first-wins`, `c3`, `leftmost-base`, `implements-split`, `qualified-syntax`) unchanged — they still do direct-owner-first short-circuit. Walker also accepts optional `ancestryOverride` for singleton dispatch. - languages/ruby: sets `mroStrategy: 'ruby-mixin'` Known limitation (documented in test TODO): Ruby bare-identifier calls inside methods flow through `resolveFreeCall` today, not owner-scoped `resolveMemberCall`. That means `ruby-mixin`'s shadow-name behavior (prepend > self) is not yet observable in the `call_serialize → serialize` case; it resolves via global lookup. The strategy is correctly wired and will apply as soon as Ruby self-call inference is added (threading bare calls as `self.method` with `receiverTypeName = enclosing class`). The `prepended_marker` test (non-shadowed method only reachable via the prepend provider) keeps working as the narrow-guard: it proves the prepend parent enters the ancestry, even though the MRO ordering effect isn't reachable yet from free-call paths. No regressions: 840 tests pass across Ruby, Python (C3), Rust (qualified), Java, Kotlin, heritage-extractor-wiring, resolve-enclosing-owner. --- gitnexus-shared/src/mro-strategy.ts | 11 +- gitnexus/src/core/ingestion/languages/ruby.ts | 4 + .../src/core/ingestion/model/heritage-map.ts | 125 ++++++++++++++++-- gitnexus/src/core/ingestion/model/resolve.ts | 83 +++++++++++- .../resolvers/ruby-sequential-mixin.test.ts | 31 +++-- 5 files changed, 235 insertions(+), 19 deletions(-) diff --git a/gitnexus-shared/src/mro-strategy.ts b/gitnexus-shared/src/mro-strategy.ts index 6168c67c0..609246085 100644 --- a/gitnexus-shared/src/mro-strategy.ts +++ b/gitnexus-shared/src/mro-strategy.ts @@ -14,10 +14,19 @@ * - `implements-split`: BFS walk, first match wins (Java/C#/Kotlin) — full * interface-default ambiguity is handled at graph level. * - `qualified-syntax`: No auto-resolution (Rust — requires `::m`). + * - `ruby-mixin`: Kind-aware walk (Ruby). Walks `prepend` parents first + * (reverse declaration order — last-prepended wins), + * then the direct owner's own methods, then `extends` + * and `include` parents (reverse declaration order). + * This is the only strategy that does NOT do a + * direct-owner-first short-circuit, because Ruby + * `prepend` must beat the class's own method of the + * same name. */ export type MroStrategy = | 'first-wins' | 'c3' | 'leftmost-base' | 'implements-split' - | 'qualified-syntax'; + | 'qualified-syntax' + | 'ruby-mixin'; diff --git a/gitnexus/src/core/ingestion/languages/ruby.ts b/gitnexus/src/core/ingestion/languages/ruby.ts index fa5c284a8..102a66e95 100644 --- a/gitnexus/src/core/ingestion/languages/ruby.ts +++ b/gitnexus/src/core/ingestion/languages/ruby.ts @@ -158,5 +158,9 @@ export const rubyProvider = defineLanguage({ classExtractor: createClassExtractor(rubyClassConfig), heritageExtractor: createHeritageExtractor(rubyHeritageConfig), labelOverride: rubyLabelOverride, + // Ruby MRO is kind-aware: prepend providers beat the class's own method, + // which in turn beats include providers. See `lookupMethodByOwnerWithMRO` + // in `model/resolve.ts` for the walk order. + mroStrategy: 'ruby-mixin', builtInNames: BUILT_INS, }); diff --git a/gitnexus/src/core/ingestion/model/heritage-map.ts b/gitnexus/src/core/ingestion/model/heritage-map.ts index 16ecf3305..44d419951 100644 --- a/gitnexus/src/core/ingestion/model/heritage-map.ts +++ b/gitnexus/src/core/ingestion/model/heritage-map.ts @@ -87,11 +87,47 @@ export const resolveExtendsType = ( /** Maximum ancestor chain depth to prevent runaway traversal. */ const MAX_ANCESTOR_DEPTH = 32; +/** + * Direct parent entry with the heritage kind that produced it. Preserved + * so kind-aware consumers (Ruby MRO, see `lookupMethodByOwnerWithMRO`) can + * walk prepend/include providers in the correct order. Flat-string consumers + * use `getParents` / `getAncestors` and see only the parent nodeIds. + */ +export interface ParentEntry { + readonly parentId: string; + /** 'extends' | 'implements' | 'trait-impl' | 'include' | 'extend' | 'prepend' */ + readonly kind: string; +} + export interface HeritageMap { /** Direct parents of `childNodeId` (extends + implements + trait-impl). */ getParents(childNodeId: string): string[]; /** Full ancestor chain (BFS, bounded depth, cycle-safe). */ getAncestors(childNodeId: string): string[]; + /** + * Direct parents with heritage kind preserved, insertion-ordered. Used by + * kind-aware consumers (Ruby MRO) that need to distinguish prepend / + * include / extend / extends for walk-order decisions. + * + * Insertion order mirrors the order `ExtractedHeritage` records were fed + * into `buildHeritageMap`, which in turn mirrors tree-sitter match order. + * For Ruby, this matches source declaration order for `prepend` / `include` + * statements — the MRO walk reverses this (last-declared-first) at the + * consumer side. + */ + getParentEntries(childNodeId: string): readonly ParentEntry[]; + /** + * Ordered ancestry for instance method dispatch (Ruby-aware): includes + * `extends`, `implements`, `trait-impl`, `include`, `prepend` kinds. + * Excludes `extend` (singleton-only). Order is caller-determined in Unit 3. + * For non-Ruby callers (first-wins, c3, etc.), this matches `getAncestors`. + */ + getInstanceAncestry(childNodeId: string): readonly ParentEntry[]; + /** + * Ordered ancestry for singleton / class-method dispatch (Ruby-aware): + * only `extend` kind parents. For non-Ruby languages this is always empty. + */ + getSingletonAncestry(childNodeId: string): readonly ParentEntry[]; /** * File paths of classes that directly implement or extend-as-interface the * given interface/abstract-class **name**. Replaces the standalone @@ -130,8 +166,12 @@ export const buildHeritageMap = ( ctx: ResolutionContext, getHeritageStrategy?: HeritageStrategyLookup, ): HeritageMap => { - // childNodeId → Set (Set to deduplicate cross-chunk duplicates) - const directParents = new Map>(); + // childNodeId → insertion-ordered array of { parentId, kind }. + // Ordered array (not Set) because Ruby MRO walk depends on declaration + // order. A parallel `seen` map dedupes `(parentId, kind)` pairs without + // losing order. + const directParents = new Map(); + const seenParents = new Map>(); // interfaceName → Set (implementor lookup for interface dispatch) const implementorFiles = new Map>(); @@ -149,10 +189,23 @@ export const buildHeritageMap = ( let parents = directParents.get(child.nodeId); if (!parents) { - parents = new Set(); + parents = []; directParents.set(child.nodeId, parents); } - parents.add(parent.nodeId); + let seen = seenParents.get(child.nodeId); + if (!seen) { + seen = new Set(); + seenParents.set(child.nodeId, seen); + } + // Dedup by `parentId + kind` so the same parent under two different + // kinds (e.g. a module that is both included and prepended — legal + // Ruby though unusual) is recorded twice; the consumer needs both + // kinds in the walk. A single (parent, kind) pair is deduped. + const key = `${parent.nodeId}|${h.kind}`; + if (!seen.has(key)) { + seen.add(key); + parents.push({ parentId: parent.nodeId, kind: h.kind }); + } } } } @@ -191,9 +244,30 @@ export const buildHeritageMap = ( // --- Public API --------------------------------------------------- + /** Internal helper: return the entries array (may be undefined). */ + const entriesFor = (nodeId: string): readonly ParentEntry[] | undefined => + directParents.get(nodeId); + + const getParentEntries = (childNodeId: string): readonly ParentEntry[] => { + const entries = entriesFor(childNodeId); + return entries ?? []; + }; + const getParents = (childNodeId: string): string[] => { - const parents = directParents.get(childNodeId); - return parents ? [...parents] : []; + const entries = entriesFor(childNodeId); + if (!entries) return []; + // Deduplicate parent ids across kinds so the flat-string contract + // (used by non-Ruby MRO strategies and by the C3 linearizer) stays + // identical to its pre-kind-awareness behavior. + const out: string[] = []; + const seen = new Set(); + for (const e of entries) { + if (!seen.has(e.parentId)) { + seen.add(e.parentId); + out.push(e.parentId); + } + } + return out; }; const getAncestors = (childNodeId: string): string[] => { @@ -212,10 +286,13 @@ export const buildHeritageMap = ( visited.add(parentId); result.push(parentId); // Expand parent's own parents for next level - const grandparents = directParents.get(parentId); + const grandparents = entriesFor(parentId); if (grandparents) { + const gpSeen = new Set(); for (const gp of grandparents) { - if (!visited.has(gp)) nextFrontier.push(gp); + if (gpSeen.has(gp.parentId)) continue; + gpSeen.add(gp.parentId); + if (!visited.has(gp.parentId)) nextFrontier.push(gp.parentId); } } } @@ -226,9 +303,39 @@ export const buildHeritageMap = ( return result; }; + /** + * Instance-dispatch ancestry walk. Excludes `extend` (singleton-only). + * For kind-aware consumers (Ruby MRO): walks parents in source-insertion + * order. The consumer is responsible for interleaving self / reversing + * prepend order / etc. This method preserves raw declaration order. + */ + const getInstanceAncestry = (childNodeId: string): readonly ParentEntry[] => { + const entries = entriesFor(childNodeId); + if (!entries) return []; + return entries.filter((e) => e.kind !== 'extend'); + }; + + /** + * Singleton-dispatch ancestry walk. Only `extend` parents. For non-Ruby + * languages this is always empty (no language currently produces `extend` + * heritage records outside Ruby). + */ + const getSingletonAncestry = (childNodeId: string): readonly ParentEntry[] => { + const entries = entriesFor(childNodeId); + if (!entries) return []; + return entries.filter((e) => e.kind === 'extend'); + }; + const getImplementorFiles = (interfaceName: string): ReadonlySet => { return implementorFiles.get(interfaceName) ?? EMPTY_SET; }; - return { getParents, getAncestors, getImplementorFiles }; + return { + getParents, + getAncestors, + getParentEntries, + getInstanceAncestry, + getSingletonAncestry, + getImplementorFiles, + }; }; diff --git a/gitnexus/src/core/ingestion/model/resolve.ts b/gitnexus/src/core/ingestion/model/resolve.ts index 746a2b2de..659a7f9a8 100644 --- a/gitnexus/src/core/ingestion/model/resolve.ts +++ b/gitnexus/src/core/ingestion/model/resolve.ts @@ -313,7 +313,86 @@ export const lookupMethodByOwnerWithMRO = ( model: SemanticModel, strategy: MroStrategy, argCount?: number, + /** + * Optional pre-computed ancestry list. When provided, overrides the default + * per-strategy ancestry source. Primarily used by Ruby singleton dispatch: + * the caller supplies `heritageMap.getSingletonAncestry(ownerNodeId)` as + * node-id array so this walker resolves against `extend` providers only. + * + * For `ruby-mixin` strategy, passing an override switches the walker into + * a no-prepend-no-self linear scan (the caller has already decided the + * order), which is the correct semantics for singleton dispatch. + */ + ancestryOverride?: readonly string[], ): SymbolDefinition | undefined => { + // ── Ruby mixin strategy ─────────────────────────────────────────── + // + // Kind-aware walk that does NOT short-circuit on the direct owner first. + // Ruby MRO for instance dispatch: + // 1. Walk `prepend` providers (reverse declaration — last-prepended first) + // 2. Direct owner lookup (the class's own methods) + // 3. Walk `extends` and `include` providers (reverse declaration) + // + // For singleton dispatch (`ClassName.foo`), the caller passes + // `ancestryOverride = heritageMap.getSingletonAncestry(owner).map(...)`. + // The walker then does a simple left-to-right scan of that override and + // skips the prepend/direct/extends-include partition. + if (strategy === 'ruby-mixin') { + if (ancestryOverride) { + // Singleton dispatch: scan the pre-computed ancestry only. + // argCount still narrows overloaded methods. + for (const ancestorId of ancestryOverride) { + const method = model.methods.lookupMethodByOwner(ancestorId, methodName, argCount); + if (method) return method; + } + return undefined; + } + + // Instance dispatch — kind-aware walk. + const instanceEntries = heritageMap.getInstanceAncestry(ownerNodeId); + // Partition into prepend parents vs other parents (extends / include / + // implements / trait-impl), preserving declaration order within each. + const prependParents: string[] = []; + const otherParents: string[] = []; + for (const e of instanceEntries) { + if (e.kind === 'prepend') prependParents.push(e.parentId); + else otherParents.push(e.parentId); + } + + // 1. Walk prepend parents in REVERSE declaration order (last-prepended wins). + for (let i = prependParents.length - 1; i >= 0; i--) { + const method = model.methods.lookupMethodByOwner(prependParents[i], methodName, argCount); + if (method) return method; + } + + // 2. Direct owner lookup (the class's own method). + const direct = model.methods.lookupMethodByOwner(ownerNodeId, methodName, argCount); + if (direct) return direct; + + // 3. Walk extends + include parents in REVERSE declaration order. + // (Ruby `include A; include B` puts B ahead of A in MRO.) + for (let i = otherParents.length - 1; i >= 0; i--) { + const method = model.methods.lookupMethodByOwner(otherParents[i], methodName, argCount); + if (method) return method; + } + + // 4. Transitive ancestors (a mixin that itself mixes in another module). + // Fall back to the BFS ancestor walk for depth > 1. Order is best-effort; + // Ruby's actual MRO for transitive mixins is rare and this under-spec is + // documented in plan 003's Deferred to Separate Tasks. + for (const ancestorId of heritageMap.getAncestors(ownerNodeId)) { + // Skip direct parents we already walked above. + if (prependParents.includes(ancestorId)) continue; + if (otherParents.includes(ancestorId)) continue; + if (ancestorId === ownerNodeId) continue; + const method = model.methods.lookupMethodByOwner(ancestorId, methodName, argCount); + if (method) return method; + } + return undefined; + } + + // ── Non-Ruby strategies: direct-owner-first short-circuit ───────── + // 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. @@ -326,7 +405,9 @@ export const lookupMethodByOwnerWithMRO = ( // Determine ancestor walk order based on MRO strategy. // readonly to accept the cached (frozen) c3 linearization without copying. let ancestors: readonly string[]; - if (strategy === 'c3') { + if (ancestryOverride) { + ancestors = ancestryOverride; + } else if (strategy === 'c3') { // C3 linearization (memoized per HeritageMap // so repeated calls for the same owner within an ingestion run reuse the // linearization instead of rebuilding the parent map and re-running C3). diff --git a/gitnexus/test/integration/resolvers/ruby-sequential-mixin.test.ts b/gitnexus/test/integration/resolvers/ruby-sequential-mixin.test.ts index 156aead5c..357ef85e9 100644 --- a/gitnexus/test/integration/resolvers/ruby-sequential-mixin.test.ts +++ b/gitnexus/test/integration/resolvers/ruby-sequential-mixin.test.ts @@ -22,6 +22,14 @@ * plan 001) makes the test fail with a clear owner-mismatch instead * of passing trivially on `Account`'s own method. * + * Plan 003 adds the `'ruby-mixin'` MroStrategy and kind-aware ancestry + * (prepend / include / extend split). The infrastructure is wired in + * `lookupMethodByOwnerWithMRO` and applies whenever Ruby calls flow through + * the owner-scoped `resolveMemberCall` path. Shadow-name assertion for + * `call_serialize → PrependedOverride#serialize` is TODO-marked below because + * Ruby bare-identifier calls inside methods (self-calls) currently take the + * `resolveFreeCall` path which doesn't do MRO. See the TODO comment for detail. + * * Known guard limitation (documented residual): reverting plan 001 Unit 1 * alone (the sequential prepass extractFromCall) does NOT make these tests * fail, because `processCalls` independently extracts call-based heritage @@ -143,18 +151,25 @@ describe('Ruby mixin heritage: sequential vs worker parity', () => { it('sequential mode resolves prepend-only method: call_prepended_marker → PrependedOverride#prepended_marker', () => { // `prepended_marker` is defined ONLY on PrependedOverride — not on // Account, Greetable, or LoggerMixin. A resolver that fails to enter - // the prepend provider into the MRO (i.e., a regression in plan 001 - // Unit 1's sequential prepass OR Unit 2's module relabel) would not - // find this method at all, and the owner list would be empty. - // - // We deliberately do NOT assert `call_serialize → PrependedOverride#serialize` - // here because `Account` also defines `serialize`; kind-aware MRO ordering - // (prepend wins over the class's own method) is a separate concern deferred - // by plan 001. + // the prepend provider into the MRO (regression in plan 001 Unit 1's + // sequential prepass OR Unit 2's module relabel) would not find this + // method at all, and the owner list would be empty. const owners = resolvedMethodOwners(sequential, 'call_prepended_marker', 'prepended_marker'); expect(owners).toContain('PrependedOverride'); }); + // TODO(plan-003-followup): assert that prepend shadows self for + // `call_serialize → PrependedOverride#serialize`. Blocked on Ruby bare-call + // self-inference: bare identifier calls like `serialize` inside `Account#call_serialize` + // currently flow through `resolveFreeCall` (global name lookup), not + // `resolveMemberCall` (owner-scoped + MRO walk). The `'ruby-mixin'` MroStrategy + // added by plan 003 is correctly wired and will apply as soon as Ruby bare calls + // are threaded as `self.method` with receiverTypeName = enclosing class. Until then, + // shadow-name resolution lands on `Account#serialize` regardless of prepend MRO. + // + // The `prepended_marker` test above is the narrower guard that works today + // (non-shadowed method only reachable via the prepend provider). + it('sequential mode emits IMPLEMENTS edges for all three mixin kinds', () => { // Ruby mixins (include / extend / prepend) flow through the IMPLEMENTS // branch of processHeritageFromExtracted with the mixin kind recorded in