fix(scanner): Resolve Python relative imports by JordanCoin · Pull Request #181 · JordanCoin/codemap

JordanCoin · 2026-09-04T14:26:55Z

Fourth of the Graph accuracy milestone (#172). Fixes #136.

⚠️ Based on #171, not on main. #171 rewrites tryExactMatch and the file index this resolution calls into, so building on main would have meant hand-resolving conflicts in exactly the code where a bad merge means a wrong edge. The diff shown against main includes #171's commits. Review this alongside #171, or after it merges — I'll rebase and the diff will shrink to the four files below. If the #171 edge review turns up a differing edge, I'll stop and re-cut this on main.

This PR's own changes: scanner/filegraph.go, scanner/astgrep.go, scanner/pythonrelative_test.go, testdata/python-relative-imports/.

What was wrong

Python spells a relative import as a run of dots counting package levels, not as path segments. From pkg/user.py, .mod means the sibling module pkg/mod; ..mod climbs one package.

fuzzyResolveWithWorkspace routed anything starting with . to resolveRelative, which is JS-shaped — it strips ./ and ../ and treats the rest as a path. For .mod it built pkg/.mod, which matches no file. So every intra-package edge in every Python project was lost, and --importers answered a confident zero.

Extraction was never the problem — the scanner already produced .mod and . correctly.

The second half: `from . import mod`

from . import mod names the module in the import list, not the path, so $PATH binds to a bare . and the edge is unrecoverable downstream. That's not a rare form; it's idiomatic Python. Each imported name is now re-formed as the relative module it means, during extraction:

from . import mod              ->  .mod
from . import mod, second      ->  .mod, .second
from . import (mod, second)    ->  .mod, .second
from . import mod as aliased   ->  .mod          (the module, not the alias)
from . import *                ->  nothing
from .mod import helper        ->  unchanged — the path already names the module,
                                   so the imported names are symbols, not modules

The fixture

testdata/python-relative-imports/ covers each form, and one that must not resolve:

pkg/mod.py           importers = [a_dotted.py, b_bare.py, c_multi.py, sub/d_parent.py]
pkg/second.py        importers = [c_multi.py]
pkg/sub/deep.py      importers = [e_dotted_path.py]          (from .sub.deep)
pkg/sub/__init__.py  importers = [f_package.py]              (from .sub — a package)
pkg/g_missing.py     imports    = []                          (from . import does_not_exist)

All four assertions fail against the base with importers = [].

c_multi.py resolves to exactly ['.mod', '.second'] — the alias dedupes to .mod rather than inventing a phantom .aliased, and g_missing extracts .does_not_exist but resolves to nothing rather than guessing.

TestResolvePythonRelativeLevels pins the level arithmetic directly, including three dots from a nested package reaching the repository root, and both bare-dots and unknown-module cases resolving to nothing.

Per-ecosystem summary (for release notes)

Ecosystem	Before	After
Python relative imports (`from .mod import x`, `from . import mod`, `from ..pkg.mod import x`)	resolved to nothing; intra-package edges absent	resolved, including package `__init__.py` targets
Python absolute imports, and every other ecosystem	unchanged	unchanged

Not done here

from . import mod where mod is a symbol rather than a module (a function re-exported in __init__.py) now produces an import specifier that resolves to nothing — a miss, not a wrong edge, and indistinguishable without type information. Namespace packages without __init__.py are also not resolved as packages.

Verification

go vet ./... clean, gofmt clean, go test ./... at the base's baseline (only the three known root-environment permission failures). Fixture verified end-to-end with a built binary for --importers --json and --deps --json.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo

Generated by Claude Code

Reuse shared subsystem scoring and scanner inventories directly. Bound prefix and basename routing while preserving uniqueness and ambiguity checks.

Keep Rust coverage and workspace-boundary fixtures independent of Cargo metadata latency. Preserve parser and graph contracts in focused tests.

Measure routing, scanner indexing, topology discovery, rendering, and watch publication with deterministic large inventories.

Reuse scanner inventories through fallback and CUE paths. Replace eager suffix maps with a compact sorted index while preserving exact-path ambiguity.

Collect provider files and manifests in one filtered walk. Hash cache inputs directly to avoid formatting and repeated path normalization.

Cache tree statistics and retain only the largest files. Compute skyline totals without copying the full source inventory.

Reuse resolved policy paths and startup inventories to avoid repeated worktree discovery and directory walks. Stream state directly into atomic replacements while preserving the previous state on encoding failure.

Keep the shared Cargo metadata deadline from canceling manual workspace recovery. Report fallback coverage when metadata probes time out.

Python spells a relative import as a run of dots counting package levels, not as path segments: from "pkg/user.py", ".mod" means the sibling module pkg/mod, and "..mod" climbs one package. Routing that through the JS-shaped relative resolver built the path "pkg/.mod", which matches no file, so every intra-package edge in a Python project was lost and --importers answered a confident zero for modules with many importers. Resolve the dots with Python's semantics, and fall back to a package's __init__.py when the name is a package rather than a module. "from . import mod" needed extraction too: the module is named in the import list rather than the path, so $PATH is a bare run of dots and the edge was unrecoverable later. Each imported name is re-formed as the relative module it means, taking the module name rather than an alias, and skipping star imports. A relative import naming a module that does not exist still resolves to nothing; guessing which file was meant would be a fabricated edge. Built on #171 because it rewrites tryExactMatch and the file index this resolution depends on. Rebase onto main once that lands. Relates to #136, #172 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…past root Two defects found in review, both mine. filepath.Dir("") is ".", which normalizes back to "", so the climb loop clamped at the scan root: a dot count deeper than the file's directory resolved to a root-level module the import never named. From app/pkg, "from ....a import A" and "from ......a import A" both produced an edge to the repository's own a.py. That is a fabricated edge, which is worse than a miss. Climbing past the root now resolves to nothing, since the package above the root is not visible and guessing is not resolution. pythonRelativeImportNames truncated at the first newline, so Black's default wrapping for a long list — "from . import (\n a,\n b,\n)" — lost every name. Comments are now stripped per line and the list flattened, so multiline lists resolve and per-line comments still do not. The fixture gains both cases: h_multiline.py for the wrapped list, and sub/i_over_climb.py, whose four-dot import must appear nowhere. Relates to #136, #172 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo

…olves `--min-importers 1` hid every hazard on this repository, and the reason was not the threshold. Go resolves imports at package level, and BuildFileGraph deliberately drops an import that resolves to more than one file rather than fanning it into an edge per file, so a file inside a multi-file Go package has zero file-level importers by construction. `scanner/filegraph.go` scored 0 and read as harmless. So did every same-package collision, including all six pairs issue #134 verified by hand. A shared file whose language resolves at package granularity is now weighted as the files outside its package that import the package, plus the package's other files, and the count is labelled `package importers` so it is not read as a file-level number. Languages whose imports name files keep the file-level count and the plain label. FileGraph.Packages is populated for Go and nothing else, which is exactly the set this is correct for. The cross-package term cannot come from the graph — the edges are the ones that were dropped — so collide now keeps the scan outcome it was already paying for and counts the raw import strings. ScanForDeps plus BuildFileGraphFromOutcome is the same single scan BuildFileGraph was doing. Real effect on this repository, where the default previously printed nothing: 6 PRs scanner/filegraph.go 73 package importers <- #171, #174, #175, ... 3 PRs config/config.go 38 package importers <- #171, #181, #182 3 PRs main.go 6 package importers <- #175, #179, #180 --min-importers now defaults to 0. A file two open PRs both change is a hazard whatever its weight, and a default that hides hazards answers "no collisions" on a repository full of them. The flag stays for narrowing a long list. Three tests added: a Go fixture where two same-package files collide and carry a non-zero package weight (with the self-import and third-party cases held out of the count), a file-resolved language keeping file scope and its plain label, and #134's six measured pairs proved unchanged by the weighting — reordering them is allowed, adding or dropping one is not. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t

@reneleonhardt

#180) * feat: codemap collide, rank open PRs by shared-file merge-order hazard CI structurally cannot see cross-PR collisions: every PR is built against main and never against its siblings. Issue #134 measured that blind spot by merging six worktree pairs by hand, and #117/#118 shipped a miscompile through it. `codemap collide` reads open PRs through `gh pr list --json files`, intersects their changed paths, and weights each shared file by the importer count from the graph on the current checkout. The intersection is glue; the weighting is the part that needs codemap, because only the graph knows that a collision on a 23-importer hub is a different severity from one on a test fixture. Honesty rules, per the design principle in #134 (a composite inherits the honesty of its primitives and states it with more authority): - Importer counts are stated as facts only while graph coverage is complete. Degraded coverage prints "unknown importers", drops the verdict to TRUST LOW, and says ranking fell back to shared-file count. - A "no collisions" answer from a degraded graph is TRUST LOW too: a negative finding from a partial graph is as unreliable as a positive one. - Coverage attribution is whole-graph, not per-language. Narrowing "partial" to a subset of languages by matching free-text notes would hand back confidence the graph never claimed. #174's ResolvesFileLevelImports is the supported seam for per-language attribution; collideImportersKnown is the single function it belongs in. - --min-importers never hides a file whose count is unknown, and never drops a hazard silently: the hidden count and the way to see them are printed. - A file the graph carries no edges for at all (a YAML rule, a fixture) reports "not in graph" rather than a zero that reads as "nothing imports it". Tests cover the pair/shared-file computation against issue #134's measured 4-PR matrix (6 of 6 pairs, including the 2-vs-3 distinction), the ranking order, degraded coverage yielding TRUST LOW with unknown counts, and a golden human output. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t * fix(scanner): Keep the Rust fallback when cargo metadata times out Ported verbatim from @reneleonhardt's open PR #171, which fixes this already. Carrying it here so this PR can go green rather than waiting on that one to merge; it becomes a no-op once main has it. buildRustWorkspaceIndex shadowed its caller's ctx with the cargo-metadata deadline, so once that deadline passed ctx.Err() returned DeadlineExceeded and the whole graph build failed with a bare "context deadline exceeded" instead of falling back to the manually derived Rust workspace. On a cold or loaded runner three seconds is not always enough for cargo metadata, and mcp/TestRustGraphContextHandlersDisclosePartialCoverage has now failed this way on three separate pull requests. Separating the metadata context from the caller's lets an expired deadline break out of the loop and keep the fallback index, which is what the test asserts and what a consumer needs: partial coverage disclosed, not a failed graph. Relates to #147, #172 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo (cherry picked from commit 24af8fb) * fix(watch): Treat an unparseable readiness file as not-ready-yet waitWatchReadiness returned on the first successful read, so a readiness file caught mid-write — existing but empty or partial — failed json.Unmarshal and aborted the wait immediately, reporting a startup failure for a daemon that had not finished writing. Only os.ErrNotExist counted as "not ready". Measured against the real function: an empty file returns "reading daemon readiness: unexpected end of JSON input" after 0s, without waiting out any part of the 30s timeout. Keep polling on a parse failure until the deadline, and surface the last parse error when the deadline passes, so a file that never becomes valid still says why rather than only that it timed out. publishWatchReadiness already renames its payload into place atomically, so codemap's own daemon does not open this window; the reader was brittle to any writer that is not atomic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo (cherry picked from commit 35d2af3) * fix(collide): Weight Go collisions at the granularity Go actually resolves `--min-importers 1` hid every hazard on this repository, and the reason was not the threshold. Go resolves imports at package level, and BuildFileGraph deliberately drops an import that resolves to more than one file rather than fanning it into an edge per file, so a file inside a multi-file Go package has zero file-level importers by construction. `scanner/filegraph.go` scored 0 and read as harmless. So did every same-package collision, including all six pairs issue #134 verified by hand. A shared file whose language resolves at package granularity is now weighted as the files outside its package that import the package, plus the package's other files, and the count is labelled `package importers` so it is not read as a file-level number. Languages whose imports name files keep the file-level count and the plain label. FileGraph.Packages is populated for Go and nothing else, which is exactly the set this is correct for. The cross-package term cannot come from the graph — the edges are the ones that were dropped — so collide now keeps the scan outcome it was already paying for and counts the raw import strings. ScanForDeps plus BuildFileGraphFromOutcome is the same single scan BuildFileGraph was doing. Real effect on this repository, where the default previously printed nothing: 6 PRs scanner/filegraph.go 73 package importers <- #171, #174, #175, ... 3 PRs config/config.go 38 package importers <- #171, #181, #182 3 PRs main.go 6 package importers <- #175, #179, #180 --min-importers now defaults to 0. A file two open PRs both change is a hazard whatever its weight, and a default that hides hazards answers "no collisions" on a repository full of them. The flag stays for narrowing a long list. Three tests added: a Go fixture where two same-package files collide and carry a non-zero package weight (with the self-import and third-party cases held out of the count), a file-resolved language keeping file scope and its plain label, and #134's six measured pairs proved unchanged by the weighting — reordering them is allowed, adding or dropping one is not. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t * fix(collide): rank a pair by its heaviest shared file, not the first one seen Shared files sort by PR count first, so a pair colliding on a hub could be reported by a fixture touched by more PRs and ranked below a lesser pair. Found by independent review; the new test reproduces it and fails without the change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: r <r@r>

reneleonhardt and others added 9 commits September 4, 2026 13:45

perf(context): Avoid eager case-folded indexes

fb30f2c

Reuse shared subsystem scoring and scanner inventories directly. Bound prefix and basename routing while preserving uniqueness and ambiguity checks.

test(scanner): Isolate Rust graph fixtures

ab5eca9

Keep Rust coverage and workspace-boundary fixtures independent of Cargo metadata latency. Preserve parser and graph contracts in focused tests.

test(perf): Cover repository-scale hot paths

fb7fd24

Measure routing, scanner indexing, topology discovery, rendering, and watch publication with deterministic large inventories.

perf(scanner): Compact dependency graph construction

46fbfb9

Reuse scanner inventories through fallback and CUE paths. Replace eager suffix maps with a compact sorted index while preserving exact-path ambiguity.

perf(topology): Share manifest and source discovery

3cf35cf

Collect provider files and manifests in one filtered walk. Hash cache inputs directly to avoid formatting and repeated path normalization.

perf(render): Bound large-inventory aggregation

883437e

Cache tree statistics and retain only the largest files. Compute skyline totals without copying the full source inventory.

perf(watch): Bound daemon hot paths

be4a68b

Reuse resolved policy paths and startup inventories to avoid repeated worktree discovery and directory walks. Stream state directly into atomic replacements while preserving the previous state on encoding failure.

fix(scanner): Preserve Cargo topology fallback

640b858

Keep the shared Cargo metadata deadline from canceling manual workspace recovery. Report fallback coverage when metadata probes time out.

Copilot AI lite review requested due to automatic review settings September 4, 2026 14:26

Copilot AI reviewed Sep 4, 2026

This was referenced Sep 4, 2026

coverage: unresolved TS/JS/Python imports are dropped silently; only Rust discloses them #183

Open

fix(scanner): Resolve TypeScript ESM specifiers that name emitted files #182

Merged

JordanCoin merged commit ced2e72 into main Sep 5, 2026
12 checks passed

JordanCoin deleted the claude/codemap-graph-accuracy-136 branch September 5, 2026 20:43

JordanCoin mentioned this pull request Sep 5, 2026

Graph accuracy: fix the four known holes before building on the graph #172

Open

Navigation Menu

Uh oh!

fix(scanner): Resolve Python relative imports - #181

fix(scanner): Resolve Python relative imports#181
JordanCoin merged 10 commits into
mainfrom
claude/codemap-graph-accuracy-136

JordanCoin commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

JordanCoin commented Sep 4, 2026

What was wrong

The second half: from . import mod

The fixture

Per-ecosystem summary (for release notes)

Not done here

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

The second half: `from . import mod`