diff --git a/tests/skill/understand/test_extract_import_map.test.mjs b/tests/skill/understand/test_extract_import_map.test.mjs index 5d55c4896..1176450bb 100644 --- a/tests/skill/understand/test_extract_import_map.test.mjs +++ b/tests/skill/understand/test_extract_import_map.test.mjs @@ -798,6 +798,66 @@ describe('extract-import-map.mjs — Python resolver', () => { expect(result.output.importMap['app.py']).toEqual(['local.py']); }); + // Regression for #621: the absolute-import walk-up offered every ancestor + // directory as a candidate sys.path root, including directories that hold + // an `__init__.py`. Those are packages, and Python puts a package's PARENT + // on sys.path — never the package itself. Offering one let a module inside + // the package shadow the stdlib module of the same name, and let a module + // resolve to itself. + it('does not treat a package directory as a sys.path root', () => { + projectRoot = setupTree({ + 'service/app/__init__.py': `# package\n`, + // `service/app/logging.py` shadows the stdlib `logging` module by name. + 'service/app/logging.py': `import logging\n`, + // `service/` has no __init__.py -> legitimately a sys.path root, so + // `from app.logging import ...` is still a real in-tree edge. + 'service/app/main.py': `from app.logging import get_logger\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'service/app/__init__.py', language: 'python', fileCategory: 'code' }, + { path: 'service/app/logging.py', language: 'python', fileCategory: 'code' }, + { path: 'service/app/main.py', language: 'python', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + // `import logging` inside the package is the stdlib module: the in-tree + // `service/app/logging.py` is not on sys.path, so there is no edge — and + // above all no self-edge. + expect(result.output.importMap['service/app/logging.py']).toEqual([]); + // The genuine edge survives: `service/` is still a candidate root. + expect(result.output.importMap['service/app/main.py']).toEqual([ + 'service/app/logging.py', + ]); + }); + + it('never resolves a python import to the importing file', () => { + projectRoot = setupTree({ + // No __init__.py anywhere — PEP 420 namespace packages throughout. + 'src/svc/main.py': `import svc.main\nimport helpers\n`, + 'src/svc/helpers.py': `def help(): pass\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/svc/main.py', language: 'python', fileCategory: 'code' }, + { path: 'src/svc/helpers.py', language: 'python', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + // `import svc.main` from `src/svc/main.py` resolves to itself under the + // `src/` root; a module is never its own import target. `import helpers` + // is the real sibling edge and must be kept. + expect(result.output.importMap['src/svc/main.py']).toEqual([ + 'src/svc/helpers.py', + ]); + }); + it('resolves absolute imports against the importers per-service root in multi-service repos', () => { // Mirrors microservices-demo: each service ships its own sibling files // under src//, and uses bare `import helpers` to reach them. diff --git a/understand-anything-plugin/skills/understand/extract-import-map.mjs b/understand-anything-plugin/skills/understand/extract-import-map.mjs index 84b3a6d7f..3b0b8ce32 100644 --- a/understand-anything-plugin/skills/understand/extract-import-map.mjs +++ b/understand-anything-plugin/skills/understand/extract-import-map.mjs @@ -948,7 +948,8 @@ function extractKotlinSources(content) { export function resolvePythonImport(rawImport, specifiers, file, ctx) { if (typeof rawImport !== 'string') return []; const src = rawImport; - const importerDir = dirOf(toPosix(file.path)); + const importerPath = toPosix(file.path); + const importerDir = dirOf(importerPath); // Count leading dots; the rest is a dotted module path let dots = 0; @@ -996,14 +997,20 @@ export function resolvePythonImport(rawImport, specifiers, file, ctx) { } // Absolute import. Walk up from the importer's directory and try every - // ancestor as a candidate Python root — the first one where probing - // resolves anything wins. This handles the multi-service / multi-package - // case where each service's directory acts as its own implicit + // ancestor that could be a sys.path root as a candidate — the first one + // where probing resolves anything wins. This handles the multi-service / + // multi-package case where each service's directory acts as its own implicit // sys.path entry (e.g. `import demo_pb2_grpc` from // `src/emailservice/email_server.py` should resolve to // `src/emailservice/demo_pb2_grpc.py`, NOT fail because the file isn't // at `/demo_pb2_grpc.py`). // + // Two candidate roots are ruled out below, because Python's import system + // does not treat them as sys.path entries: a directory carrying an + // `__init__.py` is a package (only its parent goes on sys.path), and the + // importing file is never the target of its own import. Both rules narrow + // the walk without changing the multi-service case above, whose service + // directories carry no `__init__.py`. // Importer-scope precedence (deepest ancestor first) means that when // the same module name exists in multiple services, each service's // file shadows the others — no cross-service edges. @@ -1016,8 +1023,25 @@ export function resolvePythonImport(rawImport, specifiers, file, ctx) { const importerParts = importerDir ? importerDir.split('/').filter(Boolean) : []; for (let i = importerParts.length; i >= 0; i--) { const rootParts = importerParts.slice(0, i); + // A directory holding an `__init__.py` is a PACKAGE, and Python puts a + // package's PARENT on sys.path — never the package itself. Offering one + // as a root makes any stdlib or third-party module whose name collides + // with a module inside the package (`logging`, `types`, `json`, `email`, + // `queue`, ...) resolve to the in-tree file, and lets a module resolve to + // itself. Its parent is still a candidate on the next iteration, so the + // real `from app.logging import ...` edge is unaffected. + if (rootParts.length > 0 && ctx.fileSet.has(`${rootParts.join('/')}/__init__.py`)) { + continue; + } const candidateModule = rootParts.concat(tailSegments); - const matches = resolvePythonProbe(candidateModule, specifiers, ctx); + // A module is never its own import target. `import pkg.mod` from + // `pkg/mod.py` reaches itself whenever an enclosing namespace directory + // is a candidate root; the real target is the stdlib / third-party module + // of that name. Filtering here (rather than at the call site) keeps the + // walk going, so a shallower root can still match. + const matches = resolvePythonProbe(candidateModule, specifiers, ctx).filter( + (m) => m !== importerPath, + ); if (matches.length > 0) return matches; } return [];