From 66003dcea14faf3c6211429080d7634c9746a066 Mon Sep 17 00:00:00 2001 From: Greg Scher <59206+schergr@users.noreply.github.com> Date: Mon, 9 Nov 2026 16:30:43 -0500 Subject: [PATCH] fix: resolve Python absolute imports when the import root is a subdirectory In a repo like apps/api/app/core/db.py imported as app.core.db, the module graph had no edges and every module was listed as isolated. resolveImport only matches when the module path is the shorter side, but here the import's leading part is the tail of the module path. Add a Python case to langResolver, modelled on the Java one. It indexes each .py file under every trailing sub-path of its directory and of its own path, resolves a dotted import by its longest matching leading part, and picks the closest module when several match. A single segment import only matches a package (a directory with an __init__.py), so import logging does not resolve to a repo file called logging.py. Relative imports and anything unmatched still go to the generic resolver, so this only adds edges. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 3 ++ internal/graph/modresolve.go | 71 ++++++++++++++++++++++++++++++- internal/graph/modresolve_test.go | 54 +++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be520a9..2043168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ Follows the first real-agent A/B (bench/agent-ab, pull request #77), which found ### Changed - The MCP handshake instructions and `stacklit guide` use the routed wording instead of "source of truth, do not crawl" (about 100 tokens, was about 75). `STACKLIT_STEERING=d` keeps the earlier text for comparison. +### Fixed +- Python absolute imports resolve when the import root is below the scanned root, for example `app.core.db` for `apps/api/app/core/db.py`. Before, such a repo had no module edges and every module was listed as isolated. A single segment import only matches a package, so `import logging` does not resolve to a repo file called `logging.py`. + ### Documentation - `docs/SESSIONS.md` and `docs/USAGE.md` report what the first A/B measured. `--crawl-guard` and steering `d` measured worse than no Stacklit (cost 1.12x, correct 82.7% against 94.0%): off by default, not recommended. Added a "When Stacklit helps" section: large repos and locate or add questions, not exact-name lookups or small repos. diff --git a/internal/graph/modresolve.go b/internal/graph/modresolve.go index bb63337..84cd3cb 100644 --- a/internal/graph/modresolve.go +++ b/internal/graph/modresolve.go @@ -12,13 +12,16 @@ import ( ) // langResolver maps language-specific import strings (Rust paths, Java -// packages, Ruby requires) to module names. The generic resolver only knows +// packages, Python absolute imports, Ruby requires) to module names. The generic resolver only knows // directory-style paths, which these languages never use. type langResolver struct { root string fileMod map[string]string // slash path -> module of that file dirMod map[string]string // slash dir -> module of files in that dir javaPkgs map[string][]string // trailing dir path -> sorted unique modules + pyPkgs map[string][]string // trailing dir path of a .py file -> sorted unique modules + pyFiles map[string][]string // trailing path of a .py file without .py -> sorted unique modules + pyTop map[string]bool // directory names that hold an __init__.py (importable packages) rubyLibs map[string][]string // require path without .rb -> sorted unique file paths crates map[string]string // Rust crate identifier -> its source dir seenSrc map[string]bool @@ -32,11 +35,15 @@ func newLangResolver(files []*parser.FileInfo, root string, modOf map[string]str fileMod: map[string]string{}, dirMod: map[string]string{}, javaPkgs: map[string][]string{}, + pyPkgs: map[string][]string{}, + pyFiles: map[string][]string{}, + pyTop: map[string]bool{}, rubyLibs: map[string][]string{}, crates: map[string]string{}, seenSrc: map[string]bool{}, } javaSeen := map[string]map[string]bool{} + pySeen := map[string]bool{} for _, f := range files { p := filepath.ToSlash(f.Path) mod := modOf[f.Path] @@ -54,6 +61,8 @@ func newLangResolver(files []*parser.FileInfo, root string, modOf map[string]str r.javaPkgs[suf] = append(r.javaPkgs[suf], mod) } } + case ".py": + r.notePython(p, dir, mod, pySeen) case ".rb": noExt := strings.TrimSuffix(p, path.Ext(p)) for _, suf := range dirSuffixes(noExt) { @@ -66,6 +75,12 @@ func newLangResolver(files []*parser.FileInfo, root string, modOf map[string]str for k := range r.javaPkgs { sort.Strings(r.javaPkgs[k]) } + for k := range r.pyPkgs { + sort.Strings(r.pyPkgs[k]) + } + for k := range r.pyFiles { + sort.Strings(r.pyFiles[k]) + } for k := range r.rubyLibs { sort.Strings(r.rubyLibs[k]) } @@ -138,6 +153,15 @@ func (r *langResolver) resolve(f *parser.FileInfo, imp string) (mod string, hand return r.rust(p, imp), true case ".java": return r.java(p, imp), true + case ".py": + // Relative imports and anything unmatched stay with the generic resolver. + if strings.HasPrefix(imp, ".") { + return "", false + } + if m := r.python(p, imp); m != "" { + return m, true + } + return "", false case ".rb": return r.ruby(p, imp), true } @@ -250,6 +274,51 @@ func (r *langResolver) java(p, imp string) string { return "" } +// ---- Python ---- + +// notePython indexes a .py file under every trailing sub-path of its directory +// and of its own path, so `app.core.db` can find apps/api/app/core/db.py when +// the import root (apps/api) is a subdirectory of the scanned root. +func (r *langResolver) notePython(p, dir, mod string, seen map[string]bool) { + add := func(idx map[string][]string, kind, suf string) { + k := kind + "|" + suf + "|" + mod + if seen[k] { + return + } + seen[k] = true + idx[suf] = append(idx[suf], mod) + } + if path.Base(p) == "__init__.py" { + r.pyTop[path.Base(dir)] = true + } else { + for _, suf := range dirSuffixes(strings.TrimSuffix(p, ".py")) { + add(r.pyFiles, "f", suf) + } + } + for _, suf := range dirSuffixes(dir) { + add(r.pyPkgs, "d", suf) + } +} + +// python resolves an absolute dotted import (`app.core.db`, `app.core`) to a +// module by the longest leading part of the import that is the tail of a file +// or directory path. A single-segment import only matches a real package (a +// directory with an __init__.py), so `import logging` does not land on a +// repo file called logging.py. +func (r *langResolver) python(p, imp string) string { + segs := strings.Split(imp, ".") + for n := len(segs); n >= 1; n-- { + key := strings.Join(segs[:n], "/") + if mods := r.pyFiles[key]; n >= 2 && len(mods) > 0 { + return closestModule(p, mods) + } + if mods := r.pyPkgs[key]; len(mods) > 0 && (n >= 2 || r.pyTop[segs[0]]) { + return closestModule(p, mods) + } + } + return "" +} + // closestModule picks the candidate sharing the longest path prefix with p. func closestModule(p string, mods []string) string { best, bestScore := mods[0], -1 diff --git a/internal/graph/modresolve_test.go b/internal/graph/modresolve_test.go index 34f917c..090f30a 100644 --- a/internal/graph/modresolve_test.go +++ b/internal/graph/modresolve_test.go @@ -150,3 +150,57 @@ func TestModuleRankingPrefersRichFiles(t *testing.T) { t.Errorf("ExportsByRank = %v", m.ExportsByRank) } } + +// Absolute imports are written from the Python import root (apps/api), which +// is a subdirectory of the scanned root, so `app.core.db` has to match the +// tail of apps/api/app/core/db.py. +func TestPythonAbsoluteImportsFromSubdirectoryRoot(t *testing.T) { + files := []*parser.FileInfo{ + fi("apps/api/app/__init__.py"), + fi("apps/api/app/main.py", "app.core", "app.domains.spend.router", "fastapi"), + fi("apps/api/app/core/__init__.py"), + fi("apps/api/app/core/db.py", "alembic", "sqlalchemy"), + fi("apps/api/app/core/logging.py"), + fi("apps/api/app/domains/spend/__init__.py"), + fi("apps/api/app/domains/spend/router.py", "app.core.db", "app.domains.spend.service", "logging"), + fi("apps/api/app/domains/spend/service.py", "app.core.db.Session", ".router"), + fi("apps/api/alembic/env.py"), + } + g := Build(files, BuildOptions{MaxDepth: 6}) + + core := "apps/api/app/core" + spend := "apps/api/app/domains/spend" + if got, want := dependedBy(g, core), []string{"apps/api/app", spend}; !reflect.DeepEqual(got, want) { + t.Errorf("core depended_by = %v, want %v", got, want) + } + // main.py reaches spend through app.domains.spend.router (a file, not a package). + if got, want := dependedBy(g, spend), []string{"apps/api/app"}; !reflect.DeepEqual(got, want) { + t.Errorf("spend depended_by = %v, want %v", got, want) + } + // `logging` is the standard library and `alembic` is a third-party package, even though + // the repo has a logging.py and an alembic/ directory without an __init__.py. + if got := dependedBy(g, "apps/api/alembic"); len(got) != 0 { + t.Errorf("alembic depended_by = %v, want none", got) + } + for _, m := range g.Module(core).DependsOn { + t.Errorf("core should depend on nothing, got %s (stdlib logging or third-party alembic matched a repo path)", m) + } +} + +func TestPythonSingleSegmentImportNeedsAPackage(t *testing.T) { + files := []*parser.FileInfo{ + fi("svc/app/__init__.py"), + fi("svc/app/models.py"), + fi("svc/worker/run.py", "app", "app.models"), + fi("svc/util/helpers.py"), + fi("svc/worker/jobs.py", "helpers"), + } + g := Build(files, BuildOptions{}) + if got, want := dependedBy(g, "svc/app"), []string{"svc/worker"}; !reflect.DeepEqual(got, want) { + t.Errorf("app depended_by = %v, want %v", got, want) + } + // helpers is a plain file with no package around it, so a bare `import helpers` stays unresolved. + if got := dependedBy(g, "svc/util"); len(got) != 0 { + t.Errorf("util depended_by = %v, want none", got) + } +}