fix(ingestion): qualify Ruby same-tail nested mixin modules + route IMPLEMENTS by scope (#1991) (#2006)

* fix(ingestion): qualify Ruby same-tail nested mixin modules + route IMPLEMENTS by scope (#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>

* refactor(ingestion): single-source the Ruby Trait scope-label predicate; regen ruby bench baseline (#1991)

F5 follow-up to #1991: replace the four hardcoded `nodeLabel === 'Trait'` checks
(two each in the sequential parsing-processor.ts and worker parse-worker.ts
definition paths) with a single isQualifiableScopeLabel() in ast-helpers.ts so the
lockstep paths can't drift. Value-identical predicate — no behavior change.

Also regenerate the ruby scope-capture bench baseline: #1991 added the
ruby-nested-mixin-tail-collision fixture (and updated the ruby captures-golden),
but the bench baseline was never regenerated, so the order-independent fingerprint
drifts (bf6b13a -> f0d9b4c6, fixture_count 85 -> 86). Pure fixture-corpus drift.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar 2026-06-04 11:38:41 +01:00 committed by GitHub
parent 083aedbc41
commit 560291ad6e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 222 additions and 21 deletions

View file

@ -39,10 +39,10 @@
"_rebaselined": "#1956: heritage-bearing scale source (class extends Base + use trait); both forms gated at scale; linear (~1.04)."
},
"ruby": {
"fingerprint": "61d6e5f049e5e6c4871c210d28d15348f2396345751a98ccfff2f4b54f727aff",
"fingerprint": "b5ea93bb3d0469c3821a8c70f5d5991c6f326e41097c119ad691154301dcc753",
"scaling_budget": 1.5,
"_rebaselined": "#1956 synth-widening: + ruby-qualified-base fixture; synth now reduces a scope_resolution superclass (class C < Mod::Super) to its trailing constant (matching the #1940 legacy leg), at parity. Linear (~1.03). (Earlier #1956: heritage-bearing scale source.) | #942: scope-resolution-only cleanup reworded fixture comments; capture byte-positions shift, capture LOGIC unchanged.",
"_note": "F62: + scope_resolution class/module declaration captures \u2014 fixture count 78\u219281, fingerprint drift expected. #1975: + ruby-tail-collision fixture (Foo::Bar vs Baz::Bar stay distinct nodes) \u2014 pure fixture-corpus drift, scope-extractor captures unchanged; 81\u219282."
"_note": "F62: + scope_resolution class/module declaration captures \u2014 fixture count 78\u219281, fingerprint drift expected. #1975: + ruby-tail-collision fixture (Foo::Bar vs Baz::Bar stay distinct nodes) \u2014 pure fixture-corpus drift, scope-extractor captures unchanged; 81\u219282. #1991: + ruby-nested-mixin-tail-collision fixture (85\u219286). Recomputed on the #942 merge (fixture-comment rewording shifts capture byte-positions, capture LOGIC unchanged): bf6b13a -> b5ea93bb."
},
"swift": {
"fingerprint": "53325c6345161c5a495f997297af5a24fb718fd3e6647040160f8ab2a2c8e4c0",

View file

@ -164,6 +164,14 @@ export function createClassExtractor(config: ClassExtractionConfig): ClassExtrac
return extract(node, { name: simpleName })?.qualifiedName ?? null;
},
// #1991: qualify a non-typeDeclaration scope node (e.g. a Ruby `module` → Trait)
// by the same ancestor-scope walk the node-id path uses, so two same-tail nested
// mixin modules stay distinct. extract()/extractQualifiedName cannot be reused —
// they bail on non-typeDeclarations (a module is not in typeDeclarationNodes).
qualifyScopeName(node: SyntaxNode, simpleName: string): string {
return buildQualifiedName(node, simpleName);
},
shouldSkipClassCapture(context): boolean {
return config.shouldSkipClassCapture?.(context) ?? false;
},

View file

@ -44,6 +44,14 @@ export interface ClassExtractor {
},
): ExtractedClassSymbol | null;
extractQualifiedName(node: SyntaxNode, simpleName: string): string | null;
/**
* #1991: qualify a scope-defining node that maps to a class-like registry label
* (e.g. a Ruby `module` Trait) but is NOT a typeDeclaration, so it cannot go
* through extract()/extractQualifiedName (which bail on non-typeDeclarations).
* Walks the same ancestor scopes as the node-id path. Optional only providers
* that materialize such nodes implement it.
*/
qualifyScopeName?(node: SyntaxNode, simpleName: string): string;
shouldSkipClassCapture?(
context: ClassCaptureContext & { nodeLabel: ClassLikeNodeLabel },
): boolean;

View file

@ -13,21 +13,42 @@ import { generateId } from '../../../../lib/utils.js';
const HERITAGE_PREFIX = '__heritage__:';
const PROPERTY_PREFIX = '__property__:';
/**
* #1991: resolve a BARE mixin reference (`include Loggable`) to a nested module by
* the INCLUDING class's lexical scope Ruby looks up a constant in the innermost
* enclosing scope first. For owner `App.S`, try `App.Loggable`, then walk outward.
* Returns undefined if no enclosing-scope-qualified module exists.
*/
function qualifyMixinByOwnerScope(
mixinName: string,
ownerName: string,
graphIdByName: ReadonlyMap<string, string>,
): string | undefined {
let prefix = ownerName;
let dot = prefix.lastIndexOf('.');
while (dot !== -1) {
prefix = prefix.slice(0, dot);
const g = graphIdByName.get(`${prefix}.${mixinName}`);
if (g !== undefined) return g;
dot = prefix.lastIndexOf('.');
}
return undefined;
}
function emitRubyMixinEdges(
graph: KnowledgeGraph,
parsedFiles: readonly ParsedFile[],
nodeLookup: GraphNodeLookup,
): void {
const graphIdByName = new Map<string, string>();
// Secondary tail -> graphId map (first-wins). The `__heritage__` marker carries
// the mixin TARGET as the bare written name (`arg.text`, e.g. `Loggable`), not
// its full qualifiedName, so a nested mixin module included by its short name
// (`include Loggable` where it is `App::Loggable`) misses the full-qn map and
// its IMPLEMENTS edge is silently dropped (#1982 follow-up). The tail fallback
// recovers it. OWNER (`className`) lookups stay full-qn only, preserving
// same-tail owner disambiguation; only the under-qualified mixin reference
// falls back, and a genuine same-tail mixin tie there resolves first-wins.
const graphIdByTail = new Map<string, string>();
// Secondary tail -> graphId map. The `__heritage__` marker carries the mixin
// TARGET as the bare written name (`arg.text`, e.g. `Loggable`), not its full
// qualifiedName, so a nested mixin module included by its short name misses the
// full-qn map. We first resolve it lexically by the including class's enclosing
// scope (`qualifyMixinByOwnerScope`); this tail map is the last resort. A genuine
// same-tail collision is mapped to `null` so we REFUSE to guess (#1991) rather
// than the old first-wins, which cross-wired App::Loggable / Web::Loggable.
const graphIdByTail = new Map<string, string | null>();
for (const parsed of parsedFiles) {
for (const def of parsed.localDefs) {
if (!isClassLike(def.type)) continue;
@ -43,7 +64,12 @@ function emitRubyMixinEdges(
graphIdByName.set(fullName, graphId);
const dot = fullName.lastIndexOf('.');
const tail = dot === -1 ? fullName : fullName.slice(dot + 1);
if (tail.length > 0 && !graphIdByTail.has(tail)) graphIdByTail.set(tail, graphId);
if (tail.length > 0) {
const existingTail = graphIdByTail.get(tail);
if (existingTail === undefined) graphIdByTail.set(tail, graphId);
else if (existingTail !== null && existingTail !== graphId)
graphIdByTail.set(tail, null); // same-tail collision — refuse to guess
}
}
}
}
@ -64,9 +90,15 @@ function emitRubyMixinEdges(
if (parts.length < 3) continue;
const [kind, mixinName, className] = parts;
const classGraphId = graphIdByName.get(className!);
// Owner stays full-qn; the mixin target may be written by short name and
// miss the full-qn map, so fall back to the simple-tail map (#1982).
const mixinGraphId = graphIdByName.get(mixinName!) ?? graphIdByTail.get(mixinName!);
// Owner stays full-qn. The mixin target may be written by short name and miss
// the full-qn map; resolve it lexically by the including class's enclosing
// scope (`App::S` + `Loggable` -> `App::Loggable`), then fall back to the tail
// map ONLY when unambiguous — never first-wins on a collision (#1982/#1991).
const mixinGraphId =
graphIdByName.get(mixinName!) ??
qualifyMixinByOwnerScope(mixinName!, className!, graphIdByName) ??
graphIdByTail.get(mixinName!) ??
undefined;
if (classGraphId === undefined || mixinGraphId === undefined) continue;
const edgeKey = `${classGraphId}->${mixinGraphId}:${kind}`;
if (emitted.has(edgeKey)) continue;

View file

@ -18,6 +18,7 @@ import {
findObjectLiteralBindingInfo,
getLabelFromCaptures,
isSuppressedConcreteTypedefDuplicate,
isQualifiableScopeLabel,
qualifyRustImplTargetByModScope,
CLASS_CONTAINER_TYPES,
type SyntaxNode,
@ -612,7 +613,13 @@ const processParsingSequential = async (
const getQualifiedOwnerName =
provider.classExtractor?.qualifiedNodeId === true
? (node: SyntaxNode, simpleName: string): string | null =>
provider.classExtractor!.extractQualifiedName(node, simpleName)
// #1991: a Ruby `module` owner is not a typeDeclaration, so
// extractQualifiedName returns null; fall back to the scope walk so a
// method inside a nested module owns through the SAME qualified Trait
// id its node uses (App.Loggable), not a dangling bare id.
provider.classExtractor!.extractQualifiedName(node, simpleName) ??
provider.classExtractor!.qualifyScopeName?.(node, simpleName) ??
null
: undefined;
const enclosingClassInfo = needsOwner
? cachedFindEnclosingClassInfo(
@ -639,7 +646,16 @@ const processParsingSequential = async (
extractedClassSymbol?.qualifiedName ??
(classNodeForSymbol && provider.classExtractor?.isTypeDeclaration(classNodeForSymbol)
? (provider.classExtractor.extractQualifiedName(classNodeForSymbol, nodeName) ?? nodeName)
: undefined);
: // #1991: a Ruby `module` maps to Trait (class-like registry) but is not a
// typeDeclaration, so extractQualifiedName bails. Qualify it via the scope
// walk so two same-tail nested mixin modules get distinct ids. Gated on
// qualifiedNodeId, so languages without the flag are unaffected.
isQualifiableScopeLabel(nodeLabel) &&
provider.classExtractor?.qualifiedNodeId === true &&
classNodeForSymbol
? (provider.classExtractor.qualifyScopeName?.(classNodeForSymbol, nodeName) ??
undefined)
: undefined);
// Qualify method/property IDs with enclosing class name to avoid collisions
// e.g. "Method:animal.dart:Animal.speak" vs "Method:animal.dart:Dog.speak".
@ -662,7 +678,10 @@ const processParsingSequential = async (
const qualifiedName =
rustImplQualifiedName !== undefined
? rustImplQualifiedName
: isClassLikeLabel &&
: // #1991: include Trait so a Ruby mixin module's qualified scope id keys
// the node, mirroring the class-like path (qualifiedTypeName is computed
// for Trait above).
(isClassLikeLabel || isQualifiableScopeLabel(nodeLabel)) &&
provider.classExtractor?.qualifiedNodeId === true &&
qualifiedTypeName !== undefined
? qualifiedTypeName

View file

@ -43,6 +43,18 @@ export const qualifyRustImplTargetByModScope = (
return [...modSegments, ...splitQualifiedName(rawTargetText)].filter(Boolean).join('.');
};
/**
* #1991: scope-label predicate that single-sources the `nodeLabel === 'Trait'`
* checks in parsing-processor.ts / parse-worker.ts. A Ruby `module` maps to the
* `Trait` registry label but is NOT a typeDeclaration, so `extractQualifiedName`
* bails on it; these node labels are instead qualified via the scope walk
* (`qualifyScopeName`) so same-tail nested modules get distinct ids. Keeping the
* literal in one place stops the four hand-maintained copies (two each in the
* sequential and worker definition paths) from drifting apart. Pure predicate
* value-identical to the inlined `nodeLabel === 'Trait'`.
*/
export const isQualifiableScopeLabel = (nodeLabel: string): boolean => nodeLabel === 'Trait';
/**
* Ordered list of definition capture keys for tree-sitter query matches.
* Used to extract the definition node from a capture map.

View file

@ -66,6 +66,7 @@ import {
genericFuncName,
inferFunctionLabel,
isSuppressedConcreteTypedefDuplicate,
isQualifiableScopeLabel,
qualifyRustImplTargetByModScope,
CLASS_CONTAINER_TYPES,
type SyntaxNode,
@ -1749,7 +1750,13 @@ const processFileGroup = (
const getQualifiedOwnerName =
provider.classExtractor?.qualifiedNodeId === true
? (node: SyntaxNode, simpleName: string): string | null =>
provider.classExtractor!.extractQualifiedName(node, simpleName)
// #1991: LOCKSTEP — a Ruby `module` owner is not a typeDeclaration, so
// extractQualifiedName returns null; fall back to the scope walk so a
// method inside a nested module owns through the SAME qualified Trait
// id its node uses on the worker path too.
provider.classExtractor!.extractQualifiedName(node, simpleName) ??
provider.classExtractor!.qualifyScopeName?.(node, simpleName) ??
null
: undefined;
const enclosingClassInfo = needsOwner
? cachedFindEnclosingClassInfo(
@ -1774,7 +1781,15 @@ const processFileGroup = (
extractedClassSymbol?.qualifiedName ??
(classNodeForSymbol && provider.classExtractor?.isTypeDeclaration(classNodeForSymbol)
? (provider.classExtractor.extractQualifiedName(classNodeForSymbol, nodeName) ?? nodeName)
: undefined);
: // #1991: LOCKSTEP with parsing-processor.ts — qualify a Ruby `module`
// (Trait) via the scope walk so same-tail nested mixin modules get
// distinct ids on the worker path too. Gated on qualifiedNodeId.
isQualifiableScopeLabel(nodeLabel) &&
provider.classExtractor?.qualifiedNodeId === true &&
classNodeForSymbol
? (provider.classExtractor.qualifyScopeName?.(classNodeForSymbol, nodeName) ??
undefined)
: undefined);
// Qualify method/property IDs with enclosing class name to avoid collisions.
// Class-like nodes use their own fully-qualified path as the id key when the
@ -1792,7 +1807,9 @@ const processFileGroup = (
const qualifiedName =
rustImplQualifiedName !== undefined
? rustImplQualifiedName
: isClassLikeLabel &&
: // #1991: LOCKSTEP — include Trait so a Ruby mixin module's qualified
// scope id keys the worker-path node, matching the sequential path.
(isClassLikeLabel || isQualifiableScopeLabel(nodeLabel)) &&
provider.classExtractor?.qualifiedNodeId === true &&
qualifiedTypeName !== undefined
? qualifiedTypeName

View file

@ -0,0 +1,25 @@
# Two same-tail NESTED mixin modules (App::Loggable + Web::Loggable), each included
# by a sibling class in the same enclosing module (#1991). The structure phase never
# qualified `module` (Trait) node ids, so both collapsed onto one Trait:app.rb:Loggable
# node and the bare-name mixin reference cross-wired IMPLEMENTS (first-wins tail).
# Single-file on purpose: the bare node id embeds file.path, so a cross-file split
# would not collide. S must IMPLEMENTS App::Loggable only; T → Web::Loggable only.
module App
module Loggable
def log; end
end
class S
include Loggable
end
end
module Web
module Loggable
def warn; end
end
class T
include Loggable
end
end

View file

@ -211,6 +211,10 @@
"captureGroups": 11,
"digest": "dfa494facc56b5e07a12befc77cd1e3788f0494f1373960cd9fe88715750e590"
},
"ruby-nested-mixin-tail-collision/app.rb": {
"captureGroups": 21,
"digest": "b42c38446b3e5307cd79eec888d5faf9d9683d79bacdd9738f64871d2a1e8bbc"
},
"ruby-nested-tail-collision/nested.rb": {
"captureGroups": 31,
"digest": "c48ebe5516a0faf50effbad0a19fe29be70c371b50ba9d6fa6ae3f6f708b3a4e"

View file

@ -1636,6 +1636,82 @@ describe('Ruby inline module-nested same-tail collision — worker path parity (
});
});
// ---------------------------------------------------------------------------
// Same-tail NESTED mixin MODULE collision — distinct Trait nodes (issue #1991)
//
// `module App; module Loggable; class S; include Loggable; end; end` +
// `module Web; module Loggable; class T; include Loggable; end; end`. The
// structure phase never qualified `module` (Trait) node ids, so both Loggable
// modules collapsed onto one Trait:app.rb:Loggable node and the bare-name mixin
// reference cross-wired IMPLEMENTS (first-wins tail). Asserts two distinct Trait
// nodes and each class IMPLEMENTS its OWN module (positive target identity), not
// just dangle-free. The IMPLEMENTS routing is registry-primary.
// ---------------------------------------------------------------------------
describe('Ruby same-tail nested mixin-module collision — distinct Trait nodes (issue #1991)', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'ruby-nested-mixin-tail-collision'),
() => {},
);
}, 60000);
it('materializes App.Loggable and Web.Loggable as two distinct Trait nodes', () => {
const qns = getNodesByLabelFull(result, 'Trait')
.map((n) => n.properties.qualifiedName)
.filter((q) => q === 'App.Loggable' || q === 'Web.Loggable')
.sort();
expect(qns).toEqual(['App.Loggable', 'Web.Loggable']);
});
it('routes S -> App.Loggable and T -> Web.Loggable (no cross-wire, R2)', () => {
expect(findDanglingEdges(result, ['IMPLEMENTS', 'HAS_METHOD'])).toEqual([]);
const impl = getRelationships(result, 'IMPLEMENTS');
const targetQnOf = (className: string) => {
const e = impl.find((x) => x.source === className && x.target === 'Loggable');
expect(e, `IMPLEMENTS from ${className}`).toBeDefined();
return result.graph.getNode(e!.rel.targetId)?.properties.qualifiedName;
};
expect(targetQnOf('S')).toBe('App.Loggable');
expect(targetQnOf('T')).toBe('Web.Loggable');
expect(impl.filter((x) => x.source === 'S')).toHaveLength(1);
expect(impl.filter((x) => x.source === 'T')).toHaveLength(1);
});
});
// Same fixture through the WORKER pool — the __heritage__ marker owner + the
// qualified module node id must survive worker serialization (#1991 R2/R15).
describe('Ruby same-tail nested mixin-module collision — worker path parity (issue #1991)', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'ruby-nested-mixin-tail-collision'),
() => {},
{ workerThresholdsForTest: { minFiles: 1, minBytes: 1 }, workerPoolSize: 2 },
);
}, 120000);
it('genuinely used the worker pool for the same-tail mixin-module fixture', () => {
expect(result.usedWorkerPool).toBe(true);
});
it('routes S -> App.Loggable and T -> Web.Loggable on the worker path (no cross-wire)', () => {
const impl = getRelationships(result, 'IMPLEMENTS');
const targetQnOf = (className: string) => {
const e = impl.find((x) => x.source === className && x.target === 'Loggable');
expect(e, `IMPLEMENTS from ${className}`).toBeDefined();
return result.graph.getNode(e!.rel.targetId)?.properties.qualifiedName;
};
expect(targetQnOf('S')).toBe('App.Loggable');
expect(targetQnOf('T')).toBe('Web.Loggable');
expect(impl.filter((x) => x.source === 'S')).toHaveLength(1);
expect(impl.filter((x) => x.source === 'T')).toHaveLength(1);
});
});
// ---------------------------------------------------------------------------
// Nested mixin included by SHORT name — IMPLEMENTS edge must not drop (#1982).
//