diff --git a/gitnexus/bench/receiver-resolution/BASELINE.md b/gitnexus/bench/receiver-resolution/BASELINE.md index 38d82dd37..badb4c316 100644 --- a/gitnexus/bench/receiver-resolution/BASELINE.md +++ b/gitnexus/bench/receiver-resolution/BASELINE.md @@ -9,7 +9,19 @@ test asserts this outcome. CI run 36319863343 at `4034cee` measured one addition Python call drop (113 to 114; all-kind total 159 to 160), classified as in-program with no receiver-shape annotation. No shape-arm result or performance threshold changed. The renamed bound-receiver correction preserves the existing method's -effective arity; the exact-head CI gate must still confirm these counts. +effective arity. The measurements below supersede this earlier snapshot. + +The final #3390 head did not retain that snapshot: its full corpus measured 125 +call drops, including 12 in the new mixin fixture. At #3393 head, C3 resolves +`order_hook` to `OrderX.order_hook`, removing that fixture's one ambiguous drop. +The full corpus now measures 124 call drops (16 Python, 54 in-program), with 11 +from the mixin fixture. The ten fixture outcomes missing from the old baseline +are three `helper()` calls with valid targets and an unproven variadic sibling, +field shadowing, three incompatible argument shapes, private-name lookup, an +abstract declaration, and duplicate definitions. The integration test pins +their exact call sites; none of these ten is the C3 `order_hook` call. These +counts track conservative unresolved coverage, including partial fan-out, not +only calls with no emitted edge. The Python capture fingerprint is also intentionally regenerated: ordinary call captures now include statically known argument counts, the corpus includes eight diff --git a/gitnexus/bench/receiver-resolution/baseline.json b/gitnexus/bench/receiver-resolution/baseline.json index 6656262dd..b234edc46 100644 --- a/gitnexus/bench/receiver-resolution/baseline.json +++ b/gitnexus/bench/receiver-resolution/baseline.json @@ -199,21 +199,21 @@ } }, "countArm": { - "callDrops": 114, - "totalDropsAllKinds": 160, + "callDrops": 124, + "totalDropsAllKinds": 170, "bySiteKind": { - "call": 114, + "call": 124, "read": 27, "write": 19 }, "callDropsByExtension": { ".java": 49, + ".py": 16, ".zig": 11, ".cs": 8, ".ts": 7, ".cpp": 7, ".tsx": 6, - ".py": 6, ".go": 5, ".php": 4, ".kt": 4, @@ -226,13 +226,13 @@ "chain-field": 60, "chain-call": 27, "no-chain": 23, + "<>": 11, "chain-mixed": 2, - "chain-unwrap": 1, - "<>": 1 + "chain-unwrap": 1 }, "callDropsByOrigin": { + "in-program": 54, "external": 44, - "in-program": 44, "unknown": 26 } } diff --git a/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts index 965cb69e8..0d5bbe1fe 100644 --- a/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts @@ -4,7 +4,7 @@ * * The provider is a thin wiring object — Python's specific bits * (super recognizer, LEGB merge precedence, Python's relative-import - * resolver, the simplified MRO walk) plug into `runScopeResolution`. + * resolver, C3 method resolution) plug into `runScopeResolution`. * * Migration reference: when bringing up the next language * (TypeScript / Java / Kotlin / Ruby), copy this file's structure — @@ -14,7 +14,7 @@ import type { ParsedFile, ReferenceSite, SymbolDefinition, TypeRef } from 'gitnexus-shared'; import { SupportedLanguages } from 'gitnexus-shared'; -import { buildMro, defaultLinearize } from '../../scope-resolution/passes/mro.js'; +import { buildMro, c3LinearizeStrategy } from '../../scope-resolution/passes/mro.js'; import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers.js'; import type { ArityVerdict, @@ -122,7 +122,7 @@ const pythonScopeResolver: ScopeResolver = { arityCompatibility: (callsite, def) => pythonArityCompatibility(def, callsite), buildMro: (graph, parsedFiles, nodeLookup) => - buildMro(graph, parsedFiles, nodeLookup, defaultLinearize), + buildMro(graph, parsedFiles, nodeLookup, c3LinearizeStrategy), populateOwners: (parsed: ParsedFile) => populateClassOwnedMembers(parsed), diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index 772acad52..c8adbbef1 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -528,11 +528,9 @@ export interface ScopeResolver { /** * Compute the method-dispatch order for every Class def in the - * workspace. Python uses depth-first first-seen via - * `pythonLinearize`; future languages may use C3 (Ruby, Python's - * real MRO when we go beyond the simplified walk), single- - * inheritance only (Java), or empty-map (languages without - * inheritance). + * workspace. Python passes `c3LinearizeStrategy` (CPython's MRO). + * Single-inheritance languages pass `defaultLinearize`. Languages + * without inheritance return an empty map. */ buildMro( graph: KnowledgeGraph, diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/mro.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/mro.ts index ab6842771..a95a7956c 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/mro.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/mro.ts @@ -7,9 +7,8 @@ * into `MethodDispatchIndex` via `buildPopulatedMethodDispatch`. * * **Why a strategy hook:** linearization differs across languages. - * - Python (depth-first first-seen, single inheritance): trivially - * correct; multi-inheritance falls back to BFS dedup. Real C3 - * would handle diamond hierarchies — defer until we hit one. + * - Python: C3 (`c3LinearizeStrategy`), the order CPython binds. + * Single inheritance matches the BFS walk. A diamond does not. * - Java (single-inheritance only): walk one parent. * - C++ (multiple inheritance): C3-like or BFS depending on how * strict the consumer needs to be. @@ -23,6 +22,7 @@ import type { ParsedFile } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../../../graph/types.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import type { LinearizeStrategy } from '../contract/scope-resolver.js'; +import { c3Linearize } from '../../model/resolve.js'; import { resolveDefGraphId } from '../graph-bridge/ids.js'; import { isClassLike } from '../scope/walkers.js'; @@ -89,10 +89,9 @@ export function buildMro( } /** - * Default linearization: depth-first BFS-with-visited, first-seen - * wins. Correct for single-inheritance languages and for Python's - * simplified MRO. Multi-inheritance diamond hierarchies need a real - * C3 implementation; per-language overrides land here. + * Default linearization: breadth-first, first-seen wins. Correct for + * single inheritance. A diamond visits a direct base before the deeper + * base C3 would rank first. Python does not use this. */ export const defaultLinearize: LinearizeStrategy = (_classDefId, directParents, parentsByDefId) => { const ancestors: string[] = []; @@ -108,3 +107,35 @@ export const defaultLinearize: LinearizeStrategy = (_classDefId, directParents, } return ancestors; }; + +/** + * CPython C3 order, excluding the class itself. + * + * The parent map and merge cache are reused for every class in one + * `buildMro` call. An inconsistent or cyclic hierarchy has no CPython + * order; those classes get an empty ancestor list rather than a + * breadth-first order that would name the wrong method. + */ +const c3SlotByParents = new WeakMap< + ReadonlyMap, + { parentMap: Map; cache: Map } +>(); + +export const c3LinearizeStrategy: LinearizeStrategy = ( + classDefId, + directParents, + parentsByDefId, +) => { + let slot = c3SlotByParents.get(parentsByDefId); + if (slot === undefined) { + const parentMap = new Map(); + for (const [id, parents] of parentsByDefId) parentMap.set(id, [...parents]); + slot = { parentMap, cache: new Map() }; + c3SlotByParents.set(parentsByDefId, slot); + } + if (!slot.parentMap.has(classDefId)) { + slot.parentMap.set(classDefId, [...directParents]); + } + const linearized = c3Linearize(classDefId, slot.parentMap, slot.cache); + return linearized ?? []; +}; diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 52af76330..d203b5304 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -410,6 +410,9 @@ export function emitReceiverBoundCalls( * incompatible instantiation. Absent ⇒ every heritage instantiation reads * as unknown ⇒ the pre-#2912 fan-out, unchanged. */ readonly heritageTypeArguments?: HeritageTypeArguments; + /** Classes whose written inheritance includes a base the graph could not + * prove. A later inherited member cannot be selected across that gap. */ + readonly unresolvedInheritanceByClass?: ReadonlySet; } = {}, ): ReceiverBoundResult { let emitted = 0; @@ -2340,6 +2343,7 @@ export function emitReceiverBoundCalls( const subtypeTargets = new Map(); const ambiguousCandidateIds = new Set(); const unknownCompatibilityCandidateIds = new Set(); + const incompleteInheritanceSubtypeIds = new Set(); const visitedSubtypeIds = new Set([ownerDef.nodeId]); const subtypeQueue = [ownerDef.nodeId]; let subtypeHead = 0; @@ -2352,27 +2356,32 @@ export function emitReceiverBoundCalls( subtypeQueue.push(subtype.nodeId); // Prefer a concrete override owned by this subtype. Otherwise - // accept exactly one inherited provider. The generic MRO is a - // BFS approximation rather than Python C3, so selecting the - // first of multiple inherited owners would fabricate order. - // A class-body field of the same name also blocks descriptor - // lookup and must suppress a later method candidate. + // take the first inherited provider in MRO order. Python's + // MRO is C3, so that first provider is the method CPython + // binds. A later base that also defines the name is hidden, + // the same way a class-body field hides a method. // // class Worker(HookMixin, Helpers): ... // // `Helpers` is not itself a subtype of HookMixin, so the - // subtype closure cannot discover it. The already-built MRO - // supplies the inherited owner set; the conservative rule - // above deliberately does not trust its approximate order. + // subtype closure cannot discover it. The MRO is the bridge. let subtypeAmbiguous = false; let picked: SymbolDefinition | undefined; const effectiveOwners = [ subtype.nodeId, ...scopes.methodDispatch.mroFor(subtype.nodeId), ]; - const inheritedCandidates = new Map(); - for (let ownerIndex = 0; ownerIndex < effectiveOwners.length; ownerIndex++) { - const effectiveOwnerId = effectiveOwners[ownerIndex]!; + let unresolvedBaseBeforeOwner = false; + for (const effectiveOwnerId of effectiveOwners) { + if (unresolvedBaseBeforeOwner) { + incompleteInheritanceSubtypeIds.add(subtype.nodeId); + break; + } + // A method on this owner still binds before its own bases. + // If no member binds here, an unresolved base may precede + // every later owner in the runtime MRO. + unresolvedBaseBeforeOwner = + options.unresolvedInheritanceByClass?.has(effectiveOwnerId) === true; const overloads = model.methods.lookupAllByOwner(effectiveOwnerId, memberName); const field = model.fields.lookupFieldByOwner(effectiveOwnerId, memberName); if (field !== undefined) { @@ -2400,7 +2409,6 @@ export function emitReceiverBoundCalls( // An abstract declaration still binds the name for this // owner. Do not expose a concrete method hidden in a base; // concrete descendants are visited as their own subtypes. - inheritedCandidates.clear(); ambiguousCandidateIds.add(candidate.nodeId); subtypeAmbiguous = true; break; @@ -2430,22 +2438,16 @@ export function emitReceiverBoundCalls( ) { continue; } - if (ownerIndex === 0) { - picked = candidate; - break; - } - inheritedCandidates.set(candidate.nodeId, candidate); + // First compatible definition in MRO order. Do not keep + // scanning: a later base is not the runtime target. + picked = candidate; + break; } } - if (!subtypeAmbiguous && picked === undefined) { - if (inheritedCandidates.size === 1) { - picked = inheritedCandidates.values().next().value; - } else if (inheritedCandidates.size > 1) { - for (const candidate of inheritedCandidates.values()) { - ambiguousCandidateIds.add(candidate.nodeId); - } - subtypeAmbiguous = true; - } + if (unresolvedBaseBeforeOwner && picked === undefined && !subtypeAmbiguous) { + // The final known owner can have a base absent from the + // indexed MRO, leaving this subtype's target unproven. + incompleteInheritanceSubtypeIds.add(subtype.nodeId); } if (subtypeAmbiguous || picked === undefined) continue; subtypeTargets.set(picked.nodeId, picked); @@ -2467,7 +2469,11 @@ export function emitReceiverBoundCalls( const allTargets = [...subtypeTargets.values()]; const coverage = prepareSubtypeDispatchCoverage( allTargets, - new Set([...ambiguousCandidateIds, ...unknownCompatibilityCandidateIds]), + new Set([ + ...ambiguousCandidateIds, + ...unknownCompatibilityCandidateIds, + ...incompleteInheritanceSubtypeIds, + ]), MAX_INTERFACE_DISPATCH_FANOUT, ); const targets = coverage.targets; diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index 27a96d599..bbb1db11f 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -223,6 +223,7 @@ function preEmitInheritanceEdges( scopes: ReturnType, nodeLookup: ReturnType, recordTypeArguments: HeritageTypeArgumentSink, + unresolvedInheritanceByClass: Set, ): Set { const handledSites = new Set(); const seen = new Set(); @@ -263,10 +264,23 @@ function preEmitInheritanceEdges( site.rawQualifiedName, callerClass, ); - if (targetDef === undefined) continue; + if (targetDef === undefined || targetDef.nodeId === callerClass.nodeId) { + // Static lookup can mistake the current declaration for an earlier + // same-named base. A self-edge is not inheritance evidence and would + // poison MRO construction; retain the uncertainty for dispatch. + unresolvedInheritanceByClass.add(callerClass.nodeId); + continue; + } const callerGraphId = resolveDefGraphId(callerClass.filePath, callerClass, nodeLookup); const targetGraphId = resolveDefGraphId(targetDef.filePath, targetDef, nodeLookup); - if (callerGraphId === undefined || targetGraphId === undefined) continue; + if ( + callerGraphId === undefined || + targetGraphId === undefined || + callerGraphId === targetGraphId + ) { + unresolvedInheritanceByClass.add(callerClass.nodeId); + continue; + } // Discriminate EXTENDS vs IMPLEMENTS by the resolved target's symbol kind: // conforming to an interface OR mixing in a trait/protocol is IMPLEMENTS, // deriving from a class-like is EXTENDS. The discriminator is purely @@ -827,9 +841,16 @@ export function runScopeResolution( const key = heritageTypeArgumentsKey(subtypeGraphId, supertypeGraphId); if (!heritageTypeArguments.has(key)) heritageTypeArguments.set(key, typeArguments); }; + const unresolvedInheritanceByClass = new Set(); const preEmittedInheritanceSites = callableFlowOnly ? new Set() - : preEmitInheritanceEdges(graph, finalized, nodeLookup, recordHeritageTypeArguments); + : preEmitInheritanceEdges( + graph, + finalized, + nodeLookup, + recordHeritageTypeArguments, + unresolvedInheritanceByClass, + ); // Call-based heritage hook (e.g., Ruby include/extend/prepend) — emits // IMPLEMENTS edges that `preEmitInheritanceEdges` cannot produce because // the heritage declarations are syntactic method calls, not grammar-level @@ -1125,6 +1146,7 @@ export function runScopeResolution( // interface-dispatch fan-out can refuse an incompatible instantiation // (#2912). Empty under `callableFlowOnly`, which emits no dispatch. heritageTypeArguments, + unresolvedInheritanceByClass, }, ); let receiverExtras = receiverBound.emitted; @@ -1216,6 +1238,7 @@ export function runScopeResolution( calleeIdSink: calleeIdAccumulator, isBuiltInName: provider.languageProvider.isBuiltInName, heritageTypeArguments, + unresolvedInheritanceByClass, }, ); receiverExtras += replayedReceiverBound.emitted; diff --git a/gitnexus/test/integration/resolvers/python.test.ts b/gitnexus/test/integration/resolvers/python.test.ts index 6c61a1fe9..76d3687e6 100644 --- a/gitnexus/test/integration/resolvers/python.test.ts +++ b/gitnexus/test/integration/resolvers/python.test.ts @@ -1200,19 +1200,41 @@ describe('Python mixin self-dispatch', () => { ).toBe(true); }); - it('suppresses inherited providers when the simplified MRO cannot prove Python order', () => { + it('records only the expected mixin dispatch gaps and partial coverage', () => { + const unresolvedSites = getResolutionOutcomes(result) + .filter( + (outcome) => + outcome.kind === 'suppressed' && + outcome.reason === 'receiver-unresolved' && + outcome.filePath === 'mixins.py', + ) + .map((outcome) => `${outcome.range.startLine}:${outcome.name}`) + .sort(); + expect(unresolvedSites).toEqual( + [ + '3:helper', + '6:helper', + '9:helper', + '12:missing_target', + '52:shadow_hook', + '75:keyword_only_target', + '78:positional_only_target', + '81:required_keyword_target', + '94:__private_hook', + '99:abstract_hook', + '104:duplicate_hook', + ].sort(), + ); + }); + + it('resolves a diamond mixin call to the C3 method, not the breadth-first base', () => { const orderCalls = getRelationships(result, 'CALLS').filter( (call) => call.source === 'dispatch_order' && call.target === 'order_hook', ); - expect(orderCalls).toEqual([]); - expect( - getResolutionOutcomes(result).some( - (outcome) => - outcome.kind === 'suppressed' && - outcome.name === 'order_hook' && - outcome.reason === 'member-lookup-ambiguous', - ), - ).toBe(true); + expect(orderCalls).toHaveLength(1); + // OrderX.order_hook is the CPython target. OrderB.order_hook is the + // breadth-first hit and must not be the edge. + expect(result.graph.getNode(orderCalls[0]!.rel.targetId)?.properties.startLine).toBe(40); }); it('honors inherited field shadowing instead of skipping to a later method', () => { @@ -1327,6 +1349,132 @@ describe('Python mixin self-dispatch', () => { }); }); +// --------------------------------------------------------------------------- +// Incomplete Python inheritance must not invent an MRO binding +// --------------------------------------------------------------------------- + +describe('Python incomplete inheritance', () => { + it('ignores a self-named external base without losing the class for its children', async () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-self-parent-')); + try { + writeFixtureRepo(repoDir, { + 'case.py': `import unittest + +class TestCase(unittest.TestCase): + def project_helper(self): + return 1 + +class Child(TestCase): + def call(self): + return self.project_helper() +`, + }); + const result = await runPipelineFromRepo(repoDir, () => {}); + const extendsEdges = getRelationships(result, 'EXTENDS'); + expect(extendsEdges.some((edge) => edge.rel.sourceId === edge.rel.targetId)).toBe(false); + expect(extendsEdges.map((edge) => `${edge.source}->${edge.target}`)).toEqual([ + 'Child->TestCase', + ]); + expect( + getRelationships(result, 'CALLS').filter( + (edge) => edge.source === 'call' && edge.target === 'project_helper', + ), + ).toHaveLength(1); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }, 60000); + + it('keeps an unindexed earlier base unresolved while retaining direct overrides', async () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-unknown-base-')); + try { + writeFixtureRepo(repoDir, { + 'case.py': `import external + +class HookMixin: + def dispatch(self): + return self.hook() + +class First: + def hook(self): + return 1 + +class Second: + def hook(self): + return 2 + +class Worker(HookMixin, external.Parent, First, Second): + pass + +class DirectWorker(HookMixin, external.Parent, First, Second): + def hook(self): + return 3 +`, + }); + const result = await runPipelineFromRepo(repoDir, () => {}); + const calls = getRelationships(result, 'CALLS').filter( + (edge) => edge.source === 'dispatch' && edge.target === 'hook', + ); + expect(calls.map((edge) => edge.rel.targetId)).toEqual([ + expect.stringContaining('DirectWorker.hook'), + ]); + // The external parent may supply hook at runtime, so First and Second + // are not proven targets. DirectWorker's own method still binds first. + expect( + getResolutionOutcomes(result).some( + (outcome) => + outcome.name === 'hook' && + outcome.reason === 'receiver-unresolved' && + outcome.candidateIds.some((id) => id.endsWith(':Class:Worker')), + ), + ).toBe(true); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }, 60000); + + it('records partial coverage when the final known MRO owner has an unindexed base', async () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-unknown-tail-')); + try { + writeFixtureRepo(repoDir, { + 'case.py': `import external + +class HookMixin: + def dispatch(self): + return self.hook() + +class Tail(external.Parent): + pass + +class Worker(HookMixin, Tail): + pass + +class DirectWorker(HookMixin): + def hook(self): + return 1 +`, + }); + const result = await runPipelineFromRepo(repoDir, () => {}); + const calls = getRelationships(result, 'CALLS').filter( + (edge) => edge.source === 'dispatch' && edge.target === 'hook', + ); + expect(calls.map((edge) => edge.rel.targetId)).toEqual([ + expect.stringContaining('DirectWorker.hook'), + ]); + expect( + getResolutionOutcomes(result).some( + (outcome) => + outcome.name === 'hook' && + outcome.reason === 'receiver-unresolved' && + outcome.candidateIds.some((id) => id.endsWith(':Class:Worker')), + ), + ).toBe(true); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }, 60000); +}); + // --------------------------------------------------------------------------- // Parent class resolution: EXTENDS edge // --------------------------------------------------------------------------- diff --git a/gitnexus/test/unit/scope-resolution/c3-linearize-strategy.test.ts b/gitnexus/test/unit/scope-resolution/c3-linearize-strategy.test.ts new file mode 100644 index 000000000..259338b66 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/c3-linearize-strategy.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest'; +import { + c3LinearizeStrategy, + defaultLinearize, +} from '../../../src/core/ingestion/scope-resolution/passes/mro.js'; + +describe('c3LinearizeStrategy', () => { + const parents = new Map([ + ['OrderedWorker', ['MroOrderMixin', 'OrderA', 'OrderB']], + ['OrderA', ['OrderX']], + ['MroOrderMixin', []], + ['OrderX', []], + ['OrderB', []], + ]); + + it('ranks the deeper CPython base ahead of the direct breadth-first base', () => { + expect(c3LinearizeStrategy('OrderedWorker', parents.get('OrderedWorker')!, parents)).toEqual([ + 'MroOrderMixin', + 'OrderA', + 'OrderX', + 'OrderB', + ]); + }); + + it('does not match breadth-first order on that hierarchy', () => { + expect(defaultLinearize('OrderedWorker', parents.get('OrderedWorker')!, parents)).toEqual([ + 'MroOrderMixin', + 'OrderA', + 'OrderB', + 'OrderX', + ]); + }); +});