diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/reconcile-ownership.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/reconcile-ownership.ts new file mode 100644 index 000000000..5c0962848 --- /dev/null +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/reconcile-ownership.ts @@ -0,0 +1,147 @@ +/** + * Reconcile scope-resolution's ownership view into the SemanticModel. + * + * For migrated languages (Python in particular) the legacy `parse` phase + * emits class-body callables without `ownerId` because + * `parsing-processor`'s `resolveEnclosingOwner` is language-dependent and + * not every extractor carries the enclosing-class info at parse time. + * Scope-resolution later calls `provider.populateOwners(parsed)`, which + * stamps the correct `ownerId` onto `parsed.localDefs[i]`. This pass + * mirrors those corrections into `model.methods` and `model.fields` so + * downstream passes can consult `SemanticModel` as the single + * authoritative owner-keyed index — no parallel scope-resolution + * registry is needed. + * + * ## Single-source-of-truth invariant (I9) + * + * After this pass runs, every `def in parsed.localDefs` with a non- + * undefined `ownerId` is reachable via either: + * - `model.methods.lookupAllByOwner(ownerId, simpleName)` — if the + * def is a Method / Function / Constructor, OR + * - `model.fields.lookupFieldByOwner(ownerId, simpleName)` — if the + * def is a Property / Variable. + * + * This invariant is the foundation of Contract Invariant I9 + * (`contract/scope-resolver.ts`): scope-resolution passes MUST read + * symbol-keyed lookups exclusively from `SemanticModel`. + * + * ## Idempotency + * + * The pass skips registration when `(ownerId, simpleName)` already + * contains a def with matching `nodeId`. Safe to call multiple times + * or after a language whose legacy extractor does populate `ownerId` + * (C#) — no duplicates are introduced. + * + * ## Transitional shim + * + * This reconciliation pass is an explicit shim. The architectural end + * state is for the legacy extractor to emit the correct `ownerId` for + * every language at parse time, removing the need for a second pass. + * See ARCHITECTURE.md § "Semantic-model source of truth" for the + * follow-up plan. + */ + +import type { ParsedFile } from 'gitnexus-shared'; +import type { MutableSemanticModel, SemanticModel } from '../../model/semantic-model.js'; +import { simpleQualifiedName } from '../graph-bridge/ids.js'; + +export interface ReconcileStats { + /** Method/Function/Constructor defs registered into MethodRegistry. */ + readonly methodsRegistered: number; + /** Property/Variable defs registered into FieldRegistry. */ + readonly fieldsRegistered: number; + /** Defs already present (idempotent skip). */ + readonly skippedAlreadyPresent: number; +} + +export function reconcileOwnership( + parsedFiles: readonly ParsedFile[], + model: MutableSemanticModel, +): ReconcileStats { + let methodsRegistered = 0; + let fieldsRegistered = 0; + let skippedAlreadyPresent = 0; + + for (const parsed of parsedFiles) { + for (const def of parsed.localDefs) { + const ownerId = (def as { ownerId?: string }).ownerId; + if (ownerId === undefined) continue; + const simple = simpleQualifiedName(def); + if (simple === undefined) continue; + + if (def.type === 'Method' || def.type === 'Function' || def.type === 'Constructor') { + const existing = model.methods.lookupAllByOwner(ownerId, simple); + if (existing.some((e) => e.nodeId === def.nodeId)) { + skippedAlreadyPresent++; + continue; + } + model.methods.register(ownerId, simple, def); + methodsRegistered++; + } else if (def.type === 'Property' || def.type === 'Variable') { + const existing = model.fields.lookupFieldByOwner(ownerId, simple); + if (existing !== undefined && existing.nodeId === def.nodeId) { + skippedAlreadyPresent++; + continue; + } + model.fields.register(ownerId, simple, def); + fieldsRegistered++; + } + } + } + + return { methodsRegistered, fieldsRegistered, skippedAlreadyPresent }; +} + +/** + * Debug-mode parity validator. Runs only when + * `VALIDATE_SEMANTIC_MODEL !== '0'` AND `NODE_ENV !== 'production'`. + * + * Iterates every def in `parsedFiles[i].localDefs` with an `ownerId` + * and asserts it is reachable via `model.methods.lookupAllByOwner` or + * `model.fields.lookupFieldByOwner`. On mismatch: emits a warning via + * `onWarn` — never throws, mirroring the pipeline's soft-fail posture. + * + * This is the enforcement of Contract Invariant I9 at runtime. In + * production it is a no-op; in development it surfaces drift between + * `parsed.localDefs` and `SemanticModel` that would otherwise silently + * produce wrong edges. + */ +export function validateOwnershipParity( + parsedFiles: readonly ParsedFile[], + model: SemanticModel, + onWarn: (message: string) => void, +): number { + if (process.env.NODE_ENV === 'production') return 0; + if (process.env.VALIDATE_SEMANTIC_MODEL === '0') return 0; + + let mismatches = 0; + for (const parsed of parsedFiles) { + for (const def of parsed.localDefs) { + const ownerId = (def as { ownerId?: string }).ownerId; + if (ownerId === undefined) continue; + const simple = simpleQualifiedName(def); + if (simple === undefined) continue; + + if (def.type === 'Method' || def.type === 'Function' || def.type === 'Constructor') { + const found = model.methods.lookupAllByOwner(ownerId, simple); + if (!found.some((d) => d.nodeId === def.nodeId)) { + onWarn( + `semantic-model parity: ${def.type} ${def.nodeId} (${parsed.filePath}) ` + + `owned by ${ownerId} as "${simple}" not in MethodRegistry`, + ); + mismatches++; + } + } else if (def.type === 'Property' || def.type === 'Variable') { + const found = model.fields.lookupFieldByOwner(ownerId, simple); + if (found === undefined || found.nodeId !== def.nodeId) { + onWarn( + `semantic-model parity: ${def.type} ${def.nodeId} (${parsed.filePath}) ` + + `owned by ${ownerId} as "${simple}" not in FieldRegistry`, + ); + mismatches++; + } + } + } + } + return mismatches; +} diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index 151f421fd..c548597c9 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -26,7 +26,7 @@ import type { ParsedFile, RegistryProviders } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../../../graph/types.js'; import type { MutableSemanticModel } from '../../model/semantic-model.js'; -import { simpleQualifiedName } from '../graph-bridge/ids.js'; +import { reconcileOwnership, validateOwnershipParity } from './reconcile-ownership.js'; import { extractParsedFile } from '../../scope-extractor-bridge.js'; import { finalizeScopeModel } from '../../finalize-orchestrator.js'; import { resolveReferenceSites, type ResolveStats } from '../../resolve-references.js'; @@ -104,35 +104,11 @@ export function runScopeResolution( } // Reconcile scope-resolution's ownership view into the SemanticModel. - // For migrated languages (Python in particular) the legacy extractor - // emits class-body members without `ownerId` — - // `provider.populateOwners(parsed)` above stamps the correct ownerId - // on `parsed.localDefs[i]`. Without this pass those defs would be - // invisible to `model.methods.lookupMethodByOwner` / - // `model.fields.lookupFieldByOwner`, forcing scope-resolution to - // maintain a parallel owner-keyed index. The pass is idempotent: we - // skip defs already present under `(ownerId, simpleName)` by nodeId, - // so re-running it (or running after a language whose legacy - // extractor does populate ownerId, e.g. C#) doesn't introduce - // duplicates. - for (const parsed of parsedFiles) { - for (const def of parsed.localDefs) { - const ownerId = (def as { ownerId?: string }).ownerId; - if (ownerId === undefined) continue; - const simple = simpleQualifiedName(def); - if (simple === undefined) continue; - - if (def.type === 'Method' || def.type === 'Function' || def.type === 'Constructor') { - const existing = input.model.methods.lookupAllByOwner(ownerId, simple); - if (existing.some((e) => e.nodeId === def.nodeId)) continue; - input.model.methods.register(ownerId, simple, def); - } else if (def.type === 'Property' || def.type === 'Variable') { - const existing = input.model.fields.lookupFieldByOwner(ownerId, simple); - if (existing !== undefined && existing.nodeId === def.nodeId) continue; - input.model.fields.register(ownerId, simple, def); - } - } - } + // See `reconcile-ownership.ts` for the full rationale (Contract + // Invariant I9). Debug-mode validator runs immediately after to + // catch drift between `parsed.localDefs` and the registries. + reconcileOwnership(parsedFiles, input.model); + validateOwnershipParity(parsedFiles, input.model, onWarn); if (parsedFiles.length === 0) { return { diff --git a/gitnexus/test/unit/scope-resolution/reconcile-ownership.test.ts b/gitnexus/test/unit/scope-resolution/reconcile-ownership.test.ts new file mode 100644 index 000000000..bc8cd5a71 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/reconcile-ownership.test.ts @@ -0,0 +1,297 @@ +/** + * Unit tests for the reconciliation pass and parity validator that + * bridge scope-resolution's post-`populateOwners` ownership view into + * `SemanticModel` (Contract Invariant I9). + * + * The reconciliation pass is the load-bearing shim that lets scope- + * resolution passes consume `SemanticModel` as the single authoritative + * owner-keyed index even when the legacy parse phase emitted class-body + * callables without `ownerId` (e.g. Python). + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import type { ParsedFile, SymbolDefinition } from 'gitnexus-shared'; +import { createSemanticModel } from '../../../src/core/ingestion/model/semantic-model.js'; +import { + reconcileOwnership, + validateOwnershipParity, +} from '../../../src/core/ingestion/scope-resolution/pipeline/reconcile-ownership.js'; + +// ─── Fixture helpers ──────────────────────────────────────────────────────── + +const mkFile = (filePath: string, localDefs: readonly SymbolDefinition[]): ParsedFile => ({ + filePath, + moduleScope: `scope:${filePath}#module`, + scopes: [], + parsedImports: [], + localDefs, + referenceSites: [], +}); + +const mkMethod = (opts: { + nodeId: string; + filePath: string; + name: string; + ownerId?: string; + type?: 'Method' | 'Function' | 'Constructor'; +}): SymbolDefinition => ({ + nodeId: opts.nodeId, + filePath: opts.filePath, + type: opts.type ?? 'Method', + qualifiedName: opts.ownerId ? `${opts.ownerId.replace('def:', '')}.${opts.name}` : opts.name, + ...(opts.ownerId !== undefined ? { ownerId: opts.ownerId } : {}), +}); + +const mkProperty = (opts: { + nodeId: string; + filePath: string; + name: string; + ownerId: string; + type?: 'Property' | 'Variable'; +}): SymbolDefinition => ({ + nodeId: opts.nodeId, + filePath: opts.filePath, + type: opts.type ?? 'Property', + qualifiedName: `${opts.ownerId.replace('def:', '')}.${opts.name}`, + ownerId: opts.ownerId, +}); + +// ─── reconcileOwnership ──────────────────────────────────────────────────── + +describe('reconcileOwnership', () => { + it('registers a method with ownerId that the legacy extractor missed', () => { + const model = createSemanticModel(); + const save = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [save]); + + const stats = reconcileOwnership([file], model); + + expect(stats.methodsRegistered).toBe(1); + expect(stats.fieldsRegistered).toBe(0); + expect(stats.skippedAlreadyPresent).toBe(0); + expect(model.methods.lookupAllByOwner('def:User', 'save')).toHaveLength(1); + expect(model.methods.lookupAllByOwner('def:User', 'save')[0]).toBe(save); + }); + + it('registers a property under FieldRegistry', () => { + const model = createSemanticModel(); + const nameProp = mkProperty({ + nodeId: 'def:User.name', + filePath: 'models.py', + name: 'name', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [nameProp]); + + const stats = reconcileOwnership([file], model); + + expect(stats.fieldsRegistered).toBe(1); + expect(stats.methodsRegistered).toBe(0); + expect(model.fields.lookupFieldByOwner('def:User', 'name')).toBe(nameProp); + }); + + it('registers a Variable type as a field (Python class-body assignments)', () => { + const model = createSemanticModel(); + const attr = mkProperty({ + nodeId: 'def:User.tag', + filePath: 'models.py', + name: 'tag', + ownerId: 'def:User', + type: 'Variable', + }); + const file = mkFile('models.py', [attr]); + + reconcileOwnership([file], model); + + expect(model.fields.lookupFieldByOwner('def:User', 'tag')).toBe(attr); + }); + + it('skips defs without ownerId (top-level functions)', () => { + const model = createSemanticModel(); + const topLevel = mkMethod({ + nodeId: 'def:helper', + filePath: 'utils.py', + name: 'helper', + type: 'Function', + }); + const file = mkFile('utils.py', [topLevel]); + + const stats = reconcileOwnership([file], model); + + expect(stats.methodsRegistered).toBe(0); + expect(model.methods.lookupAllByOwner('def:something', 'helper')).toEqual([]); + }); + + it('is idempotent — re-running skips defs already registered', () => { + const model = createSemanticModel(); + const save = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [save]); + + const first = reconcileOwnership([file], model); + const second = reconcileOwnership([file], model); + + expect(first.methodsRegistered).toBe(1); + expect(second.methodsRegistered).toBe(0); + expect(second.skippedAlreadyPresent).toBe(1); + // Registry still contains exactly one entry, not two. + expect(model.methods.lookupAllByOwner('def:User', 'save')).toHaveLength(1); + }); + + it('coexists with pre-registered defs (legacy extractor already set ownerId)', () => { + const model = createSemanticModel(); + // Simulate the legacy path: register via SymbolTable.add, which + // fans out to MethodRegistry via the dispatch table. + const save = model.symbols.add('models.cs', 'save', 'def:User.save', 'Method', { + ownerId: 'def:User', + qualifiedName: 'User.save', + }); + const file = mkFile('models.cs', [save]); + + const stats = reconcileOwnership([file], model); + + expect(stats.methodsRegistered).toBe(0); + expect(stats.skippedAlreadyPresent).toBe(1); + expect(model.methods.lookupAllByOwner('def:User', 'save')).toHaveLength(1); + }); + + it('registers multiple overloads under the same (owner, name)', () => { + const model = createSemanticModel(); + const log1 = mkMethod({ + nodeId: 'def:Logger.log#1', + filePath: 'log.cs', + name: 'log', + ownerId: 'def:Logger', + }); + const log2 = mkMethod({ + nodeId: 'def:Logger.log#2', + filePath: 'log.cs', + name: 'log', + ownerId: 'def:Logger', + }); + const file = mkFile('log.cs', [log1, log2]); + + reconcileOwnership([file], model); + + const overloads = model.methods.lookupAllByOwner('def:Logger', 'log'); + expect(overloads).toHaveLength(2); + expect(overloads.map((d) => d.nodeId).sort()).toEqual(['def:Logger.log#1', 'def:Logger.log#2']); + }); +}); + +// ─── validateOwnershipParity ─────────────────────────────────────────────── + +describe('validateOwnershipParity', () => { + const originalNodeEnv = process.env.NODE_ENV; + const originalGate = process.env.VALIDATE_SEMANTIC_MODEL; + + afterEach(() => { + if (originalNodeEnv === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = originalNodeEnv; + if (originalGate === undefined) delete process.env.VALIDATE_SEMANTIC_MODEL; + else process.env.VALIDATE_SEMANTIC_MODEL = originalGate; + }); + + it('emits no warnings when reconciliation has populated all owner-keyed defs', () => { + process.env.NODE_ENV = 'development'; + const model = createSemanticModel(); + const save = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [save]); + reconcileOwnership([file], model); + + const onWarn = vi.fn(); + const mismatches = validateOwnershipParity([file], model, onWarn); + + expect(mismatches).toBe(0); + expect(onWarn).not.toHaveBeenCalled(); + }); + + it('warns when a def with ownerId is not registered in the model', () => { + process.env.NODE_ENV = 'development'; + const model = createSemanticModel(); + const orphan = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [orphan]); + // Intentionally skip reconciliation to simulate the drift. + + const onWarn = vi.fn(); + const mismatches = validateOwnershipParity([file], model, onWarn); + + expect(mismatches).toBe(1); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0][0]).toMatch(/semantic-model parity/); + expect(onWarn.mock.calls[0][0]).toMatch(/MethodRegistry/); + }); + + it('is a no-op when NODE_ENV=production', () => { + process.env.NODE_ENV = 'production'; + const model = createSemanticModel(); + const orphan = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [orphan]); + + const onWarn = vi.fn(); + const mismatches = validateOwnershipParity([file], model, onWarn); + + expect(mismatches).toBe(0); + expect(onWarn).not.toHaveBeenCalled(); + }); + + it('is a no-op when VALIDATE_SEMANTIC_MODEL=0', () => { + process.env.NODE_ENV = 'development'; + process.env.VALIDATE_SEMANTIC_MODEL = '0'; + const model = createSemanticModel(); + const orphan = mkMethod({ + nodeId: 'def:User.save', + filePath: 'models.py', + name: 'save', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [orphan]); + + const onWarn = vi.fn(); + validateOwnershipParity([file], model, onWarn); + + expect(onWarn).not.toHaveBeenCalled(); + }); + + it('warns on missing Property just like missing Method', () => { + process.env.NODE_ENV = 'development'; + const model = createSemanticModel(); + const orphan = mkProperty({ + nodeId: 'def:User.name', + filePath: 'models.py', + name: 'name', + ownerId: 'def:User', + }); + const file = mkFile('models.py', [orphan]); + + const onWarn = vi.fn(); + const mismatches = validateOwnershipParity([file], model, onWarn); + + expect(mismatches).toBe(1); + expect(onWarn.mock.calls[0][0]).toMatch(/FieldRegistry/); + }); +});