From f03a5ec19536ad67534c5151bbe59d2afb396e8f Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sun, 19 Apr 2026 10:14:20 +0100 Subject: [PATCH] test(python): remove parallel scope-resolution integration test The new test/integration/python-scope-resolution.test.ts duplicated coverage the reviewer explicitly rejected. The existing test/integration/resolvers/python.test.ts (191 tests, driven by runPipelineFromRepo) is the source of truth for Ring 3 parity. Also document the IMPORTS-emission follow-up gap: wiring emitImportEdges in python-scope-emit.ts today regresses 10 IMPORTS-edge fixtures because the scope-extractor's ImportEdge coverage is narrower than legacy pythonImportConfig.importResolver. Tracked as a follow-up. Baseline with REGISTRY_PRIMARY_PYTHON=1 is unchanged: 109/191 pass. --- .../src/core/ingestion/python-scope-emit.ts | 14 +- .../python-scope-resolution.test.ts | 432 ------------------ 2 files changed, 8 insertions(+), 438 deletions(-) delete mode 100644 gitnexus/test/integration/python-scope-resolution.test.ts diff --git a/gitnexus/src/core/ingestion/python-scope-emit.ts b/gitnexus/src/core/ingestion/python-scope-emit.ts index 6459f1dea..ea7418f57 100644 --- a/gitnexus/src/core/ingestion/python-scope-emit.ts +++ b/gitnexus/src/core/ingestion/python-scope-emit.ts @@ -172,17 +172,19 @@ export function runPythonScopeResolution( // records' source + target via `nodeLookup` (built earlier alongside // the MRO map) before calling `graph.addRelationship`. This keeps // `emit-references.ts` untouched (it stays pure scope-resolution). - const { emitted, skipped } = emitReferencesViaLookup( - graph, - indexes, - referenceIndex, - nodeLookup, - ); + const { emitted, skipped } = emitReferencesViaLookup(graph, indexes, referenceIndex, nodeLookup); // IMPORTS edges are emitted by the legacy `processImports` path // (which heritage resolution depends on for `ctx.resolve` to find // imported symbols). We intentionally do NOT emit IMPORTS here to // avoid duplicates and to keep heritage's resolution chain intact. + // TODO (RFC #909 Ring 3 follow-up): migrate IMPORTS edge emission to + // this path once the scope-extractor's ImportEdge coverage matches + // the legacy `pythonImportConfig.importResolver`. The existing + // `emitImportEdges` helper below is ready; wiring it requires + // closing a coverage gap identified against 10 fixtures (see + // `python-django-import`, `python-named-imports`, + // `python-alias-imports`, etc.). Tracked as separate ticket. const importsEmitted = 0; return { diff --git a/gitnexus/test/integration/python-scope-resolution.test.ts b/gitnexus/test/integration/python-scope-resolution.test.ts deleted file mode 100644 index 7e4e7dd37..000000000 --- a/gitnexus/test/integration/python-scope-resolution.test.ts +++ /dev/null @@ -1,432 +0,0 @@ -/** - * Python scope-resolution INTEGRATION test (RFC #909 Ring 3). - * - * Exercises the full new code path end-to-end on a real multi-file - * Python repo: - * - * fs `.py` source - * │ pythonProvider.emitScopeCaptures() - * ▼ - * CaptureMatch[] - * │ scope-extractor (Pass 1-5) - * ▼ - * ParsedFile[] - * │ finalizeScopeModel( + Python hooks adapted to FinalizeHooks) - * ▼ - * ScopeResolutionIndexes ◀── assertions live here - * - * This is the integration story that mirrors the established - * `test/integration/resolvers/python.test.ts` pattern (multi-file, real - * sources, pipeline-driven, declarative assertions on the resolved - * graph) — adapted for the new scope-resolution code path that's not - * yet wired into `runPipelineFromRepo` (Ring 4 work). - * - * Coverage map: - * - * - Cross-file `from X import Y` linking - * - Cross-file `import X as Y` (namespace alias) - * - Re-export chains (`__init__.py` re-exporting from a submodule) - * - Wildcard `from X import *` expansion + lookup - * - Function-local `from X import Y` attaching at the function scope - * - Method receiver synthesis: `self`, `cls`, `@staticmethod` skip - * - Per-name decomposition of `from m import a, b` - * - `FinalizeStats` accuracy - * - PEP-328 relative imports (`from .user import …`) - * - LEGB precedence: function-local shadows module-level imports - */ - -import { describe, it, expect, beforeAll } from 'vitest'; -import path from 'path'; -import fs from 'fs'; -import type { - BindingRef, - ImportEdge, - ParsedFile, - ScopeId, - WorkspaceIndex, -} from 'gitnexus-shared'; -import { extractParsedFile } from '../../src/core/ingestion/scope-extractor-bridge.js'; -import { - finalizeScopeModel, - type FinalizeOrchestratorOptions, -} from '../../src/core/ingestion/finalize-orchestrator.js'; -import { pythonProvider } from '../../src/core/ingestion/languages/python.js'; -import { - pythonMergeBindings, - resolvePythonImportTarget, - type PythonResolveContext, -} from '../../src/core/ingestion/languages/python/index.js'; -import type { ScopeResolutionIndexes } from '../../src/core/ingestion/model/scope-resolution-indexes.js'; - -const FIXTURE_ROOT = path.resolve( - __dirname, - '..', - 'fixtures', - 'python-scope-integration', -); - -// ─── Test harness ────────────────────────────────────────────────────────── - -interface ResolvedRepo { - readonly indexes: ScopeResolutionIndexes; - readonly parsedByFile: ReadonlyMap; -} - -/** Walk the fixture root, parse every `.py` file via `pythonProvider`, - * then run `finalizeScopeModel` with Python hooks adapted to the - * `FinalizeHooks` shape the orchestrator expects. - * - * The adapter layer here is intentionally thin and lives in the test: - * the production glue between `LanguageProvider` and `FinalizeHooks` - * is the subject of follow-up work (Ring 4 / pipeline wiring), and - * inlining it here keeps the integration test honest about what - * contracts it exercises. */ -function buildResolvedRepo(rootDir: string): ResolvedRepo { - const pyFiles = collectPyFiles(rootDir).sort(); - const allFilePaths = new Set(pyFiles); - - const parsedByFile = new Map(); - for (const filePath of pyFiles) { - const src = fs.readFileSync(filePath, 'utf8'); - const parsed = extractParsedFile(pythonProvider, src, filePath, () => {}); - if (parsed === undefined) { - throw new Error(`extractParsedFile returned undefined for ${filePath}`); - } - parsedByFile.set(filePath, parsed); - } - - // Adapter: `(targetRaw, fromFile, ws)` → `string | null` - // The provider's `resolveImportTarget` takes `(ParsedImport, ws)`, - // but the central finalize hook is targetRaw-shaped because it's - // upstream of `interpretImport`. We synthesize the minimal - // `ParsedImport` shape needed by `resolvePythonImportTarget`. - const options: FinalizeOrchestratorOptions = { - workspaceIndex: undefined, // per-file ws is built inline below - hooks: { - resolveImportTarget: (targetRaw, fromFile) => { - const ws: PythonResolveContext = { fromFile, allFilePaths }; - return resolvePythonImportTarget( - { kind: 'named', localName: '_', importedName: '_', targetRaw }, - ws as unknown as WorkspaceIndex, - ); - }, - mergeBindings: (existing, incoming, scopeId) => { - // The provider's `pythonMergeBindings(scope, bindings)` expects a - // `Scope` object, not a `ScopeId`. We don't need scope.kind for - // LEGB tier merging (tier comes from `BindingRef.origin`), so a - // shape stub is sufficient here. This adapter mirrors what the - // production wiring will do in #921 (Ring 2 finalize integration). - const fakeScope = { id: scopeId } as unknown as Parameters[0]; - return pythonMergeBindings(fakeScope, [...existing, ...incoming]); - }, - // Wildcard expansion: scan the target module's local defs for - // public names. Mirrors the Python runtime behavior — anything not - // prefixed with `_` is exported (no `__all__` parsing here; the - // production hook should consult `__all__` when present, but the - // fixture deliberately doesn't set it so we test the default path). - expandsWildcardTo: (targetModuleScope) => { - for (const file of parsedByFile.values()) { - if (file.moduleScope !== targetModuleScope) continue; - return file.localDefs - .filter((d) => d.qualifiedName !== undefined && d.qualifiedName.length > 0) - .map((d) => d.qualifiedName!.split('.').pop()!) - .filter((name) => name.length > 0 && !name.startsWith('_')); - } - return []; - }, - }, - }; - - const indexes = finalizeScopeModel([...parsedByFile.values()], options); - return { indexes, parsedByFile }; -} - -function collectPyFiles(dir: string): string[] { - const out: string[] = []; - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const full = path.join(dir, entry.name); - if (entry.isDirectory()) out.push(...collectPyFiles(full)); - else if (entry.isFile() && entry.name.endsWith('.py')) out.push(full); - } - return out; -} - -/** Lookup helpers — mirror the style of `test/integration/resolvers/helpers.ts`. */ -function importEdgesIn(repo: ResolvedRepo, fileSuffix: string): readonly ImportEdge[] { - const file = findFile(repo, fileSuffix); - return repo.indexes.imports.get(file.moduleScope) ?? []; -} - -function findFile(repo: ResolvedRepo, fileSuffix: string): ParsedFile { - for (const [path_, file] of repo.parsedByFile) { - if (path_.endsWith(fileSuffix)) return file; - } - throw new Error(`fixture file not found: ${fileSuffix}`); -} - -function functionScopeBy(parsed: ParsedFile, fnName: string) { - return parsed.scopes.find( - (s) => - s.kind === 'Function' && - // Some defs name the scope after the qualified path; match the leaf. - (s.ownedDefs.some((d) => d.qualifiedName?.split('.').pop() === fnName) || - s.ownedDefs.some((d) => d.qualifiedName === fnName)), - ); -} - -// ─── Fixture-wide one-time parse ─────────────────────────────────────────── - -let repo: ResolvedRepo; - -beforeAll(() => { - repo = buildResolvedRepo(FIXTURE_ROOT); -}, 30_000); - -// ─── Cross-file linking ──────────────────────────────────────────────────── - -describe('Python scope-resolution integration: cross-file linking', () => { - it('links `from models.user import User as UserModel` to user.py', () => { - const auth = importEdgesIn(repo, 'services/auth.py'); - const userImport = auth.find((e) => e.localName === 'UserModel'); - expect(userImport).toBeDefined(); - expect(userImport!.kind).toBe('alias'); - expect(userImport!.targetExportedName).toBe('User'); - expect(userImport!.targetFile?.endsWith('models/user.py')).toBe(true); - expect(userImport!.linkStatus).toBeUndefined(); - }); - - it('decomposes `from utils.logger import log_info, log_error` into 2 separate edges', () => { - const auth = importEdgesIn(repo, 'services/auth.py'); - // Only the module-level `from … import log_info, log_error`. The - // function-local `from … import log_error as fail` decomposes too, - // but lives at a different scope conceptually — we filter it out - // here by `kind === 'named'` (the function-local one is `'alias'`). - const log = auth.filter( - (e) => e.targetExportedName.startsWith('log_') && e.kind === 'named', - ); - const names = log.map((e) => e.localName).sort(); - expect(names).toEqual(['log_error', 'log_info']); - for (const e of log) { - expect(e.kind).toBe('named'); - expect(e.targetFile?.endsWith('utils/logger.py')).toBe(true); - } - }); - - it('links namespace import `import models.user` to user.py', () => { - const auth = importEdgesIn(repo, 'services/auth.py'); - const ns = auth.find((e) => e.kind === 'namespace'); - expect(ns).toBeDefined(); - expect(ns!.targetFile?.endsWith('models/user.py')).toBe(true); - }); - - it('handles PEP-328 relative imports in __init__.py (`from .user import …`)', () => { - const initFile = importEdgesIn(repo, 'models/__init__.py'); - expect(initFile.length).toBeGreaterThanOrEqual(2); - for (const e of initFile) { - expect(e.targetFile?.endsWith('models/user.py')).toBe(true); - expect(e.linkStatus).toBeUndefined(); - } - const exported = initFile.map((e) => e.targetExportedName).sort(); - expect(exported).toEqual(['Admin', 'User']); - }); -}); - -// ─── Wildcard expansion ──────────────────────────────────────────────────── - -describe('Python scope-resolution integration: wildcard expansion', () => { - it('expands `from utils.logger import *` into one edge per public def', () => { - const notifier = importEdgesIn(repo, 'services/notifier.py'); - const expanded = notifier.filter((e) => e.kind === 'wildcard-expanded'); - const names = expanded.map((e) => e.targetExportedName).sort(); - expect(names).toEqual(['log_error', 'log_info', 'log_with_extras']); - for (const e of expanded) { - expect(e.targetFile?.endsWith('utils/logger.py')).toBe(true); - expect(e.linkStatus).toBeUndefined(); - } - }); -}); - -// ─── Function-local imports / LEGB ───────────────────────────────────────── - -describe('Python scope-resolution integration: function-local imports', () => { - it('captures the function-local `from utils.logger import log_error as fail` as a ParsedImport on the file', () => { - // NOTE: per-scope owning attribution (Scope.imports) flows from - // `pythonImportOwningScope` and is exercised by the unit suite. The - // current finalize orchestrator keys the resolved `ImportEdge` map at - // the module scope only — wiring that to per-scope is Ring 4 work - // (#922 follow-up). Until then we assert that the parsed import is - // present and resolvable; the placement-at-function-scope assertion - // moves with the wiring. - const auth = findFile(repo, 'services/auth.py'); - const failImport = auth.parsedImports.find( - (p) => p.kind === 'alias' && (p as { alias?: string }).alias === 'fail', - ); - expect(failImport).toBeDefined(); - expect(failImport!.targetRaw).toBe('utils.logger'); - }); -}); - -// ─── Receiver type bindings (self / cls / staticmethod) ──────────────────── - -describe('Python scope-resolution integration: implicit receivers', () => { - it('synthesizes `self: AuthService` for instance methods', () => { - const auth = findFile(repo, 'services/auth.py'); - const authenticate = functionScopeBy(auth, 'authenticate'); - expect(authenticate).toBeDefined(); - expect(authenticate!.typeBindings.get('self')?.rawName).toBe('AuthService'); - expect(authenticate!.typeBindings.get('self')?.source).toBe('self'); - }); - - it('synthesizes `cls: AuthService` for @classmethod-decorated methods', () => { - const auth = findFile(repo, 'services/auth.py'); - const fromEnv = functionScopeBy(auth, 'from_env'); - expect(fromEnv).toBeDefined(); - expect(fromEnv!.typeBindings.get('cls')?.rawName).toBe('AuthService'); - expect(fromEnv!.typeBindings.has('self')).toBe(false); - }); - - it('does NOT synthesize a receiver for @staticmethod-decorated methods', () => { - const auth = findFile(repo, 'services/auth.py'); - const hashToken = functionScopeBy(auth, 'hash_token'); - expect(hashToken).toBeDefined(); - expect(hashToken!.typeBindings.has('self')).toBe(false); - expect(hashToken!.typeBindings.has('cls')).toBe(false); - }); - - it('does NOT synthesize a receiver for free top-level functions', () => { - const logger = findFile(repo, 'utils/logger.py'); - const logInfo = functionScopeBy(logger, 'log_info'); - expect(logInfo).toBeDefined(); - expect(logInfo!.typeBindings.has('self')).toBe(false); - expect(logInfo!.typeBindings.has('cls')).toBe(false); - }); -}); - -// ─── Stats accuracy ──────────────────────────────────────────────────────── - -describe('Python scope-resolution integration: FinalizeStats', () => { - it('reports correct totals across the multi-file fixture', () => { - const stats = repo.indexes.stats; - // 5 .py files in the fixture (auth.py, notifier.py, __init__.py, user.py, logger.py). - expect(stats.totalFiles).toBe(5); - // Every import edge in this fixture is resolvable in-workspace. - expect(stats.unresolvedEdges).toBe(0); - expect(stats.linkedEdges).toBe(stats.totalEdges); - expect(stats.totalEdges).toBeGreaterThan(0); - }); - - it('produces a non-empty bindings index seeded with at least one entry per file', () => { - expect(repo.indexes.bindings.size).toBeGreaterThan(0); - for (const [, file] of repo.parsedByFile) { - const moduleBindings = repo.indexes.bindings.get(file.moduleScope); - expect(moduleBindings).toBeDefined(); - } - }); - - it('exposes module scopes via `moduleScopes` index for every parsed file', () => { - for (const [filePath, file] of repo.parsedByFile) { - const moduleScopeId = repo.indexes.moduleScopes.get(filePath); - expect(moduleScopeId).toBe(file.moduleScope); - } - }); -}); - -// ─── Definition discovery ───────────────────────────────────────────────── - -describe('Python scope-resolution integration: defs index', () => { - it('indexes every public class and function across the workspace', () => { - const allDefs: string[] = []; - for (const d of repo.indexes.defs.byId.values()) { - if (d.qualifiedName !== undefined && d.qualifiedName.length > 0) { - allDefs.push(d.qualifiedName.split('.').pop()!); - } - } - const sorted = allDefs.sort(); - // Membership check: shifts in this list mean a real surface change. - for (const expected of [ - 'Admin', - 'AuthService', - 'User', - 'authenticate', - 'can_delete', - 'display_name', - 'emit_all', - 'from_env', - 'hash_token', - 'log_error', - 'log_info', - 'log_with_extras', - ]) { - expect(sorted).toContain(expected); - } - }); -}); - -// ─── Defensive: BindingRef provenance ───────────────────────────────────── - -describe('Python scope-resolution integration: BindingRef provenance', () => { - it("tags imported bindings with origin='import' and a non-null `via` ImportEdge", () => { - const auth = findFile(repo, 'services/auth.py'); - const moduleBindings = repo.indexes.bindings.get(auth.moduleScope); - expect(moduleBindings).toBeDefined(); - const userBinding = moduleBindings!.get('UserModel'); - expect(userBinding).toBeDefined(); - expect(userBinding!.length).toBeGreaterThan(0); - const ref: BindingRef = userBinding![0]!; - // `alias`-kind imports use origin='import' (the Python provider does - // not yet emit 'alias' as a `BindingRef.origin` value — alias-ness - // lives on the `via` ImportEdge). - expect(['import', 'namespace']).toContain(ref.origin); - expect(ref.via).toBeDefined(); - expect(ref.via!.targetExportedName).toBe('User'); - }); - - it("tags wildcard-expanded bindings with origin='wildcard'", () => { - const notifier = findFile(repo, 'services/notifier.py'); - const moduleBindings = repo.indexes.bindings.get(notifier.moduleScope); - expect(moduleBindings).toBeDefined(); - const log = moduleBindings!.get('log_info'); - expect(log).toBeDefined(); - expect(log![0]!.origin).toBe('wildcard'); - }); -}); - -// ─── Module scope topology ──────────────────────────────────────────────── - -describe('Python scope-resolution integration: scope topology', () => { - it('produces exactly one Module scope per .py file', () => { - let moduleScopes = 0; - for (const s of repo.indexes.scopeTree.byId.values()) { - if (s.kind === 'Module') moduleScopes += 1; - } - expect(moduleScopes).toBe(repo.parsedByFile.size); - }); - - it('class scopes contain method scopes as descendants', () => { - const userFile = findFile(repo, 'models/user.py'); - const userClass = userFile.scopes.find( - (s) => s.kind === 'Class' && s.ownedDefs.some((d) => d.qualifiedName?.endsWith('User')), - ); - expect(userClass).toBeDefined(); - const initMethod = userFile.scopes.find( - (s) => - s.kind === 'Function' && - s.parent === userClass!.id && - s.ownedDefs.some((d) => d.qualifiedName?.endsWith('__init__')), - ); - expect(initMethod).toBeDefined(); - }); -}); - -// ─── Cross-language sanity (shouldn't process .ts/.js) ──────────────────── - -describe('Python scope-resolution integration: scope kinds emitted', () => { - it('emits only Module / Class / Function scope kinds (no block/expression scopes)', () => { - const seenKinds = new Set(); - for (const file of repo.parsedByFile.values()) { - for (const s of file.scopes) seenKinds.add(s.kind); - } - // Python has no block scope; we deliberately don't emit @scope.block. - expect([...seenKinds].sort()).toEqual(['Class', 'Function', 'Module']); - }); -});