Skip to content

fix(python): resolve named imports and imported class methods (#465) - #490

Open
ryanduguid wants to merge 5 commits into
trailhq:mainfrom
ryanduguid:ryan/python-named-imports
Open

ryanduguid wants to merge 5 commits into
trailhq:mainfrom
ryanduguid:ryan/python-named-imports

Conversation

@ryanduguid

@ryanduguid ryanduguid commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Fixes #465. graft callers missed Python calls made through a named import: an aliased function (from validators import validate_result as validate_equity_result), a class method on an imported class (Workspace.open()) and a function whose name is defined in more than one file (from first.helpers import parse_value).

This builds on #437 by @JeglovDev and includes its commit, rebased on current main, because it reuses #437's module-path resolver. If #437 merges first, this reduces to the second commit.

Unconditional module-level from x import y [as z] bindings now resolve inside the imported file only:

extract.ts  collectPythonImportedModules  module-level named imports → pythonNamedImports
                                          every import-bound name → pythonImportedNames
            call through an imported name edge carries the import's module as its specifier
            import-bound but unresolved   no edge (shadowed, local or conditional import in scope)
resolve.ts  specifier + name              the one function or class with that name in that file
            specifier + Class.method      that class's method in that file
            specifier + submodule         inside the resolved package; an ambiguous package drops it
            no unique match               no edge; never falls back to a global name match

A name an import binds resolves through that import or not at all. A call is dropped when a parameter, assignment, loop target, with or except alias, walrus (including one in another definition's header), del, match capture, type alias, local import, or a lambda, comprehension or class-body binding rebinds the name, and when the only import of the name in scope is local or conditional. Defaults, annotations and the leftmost comprehension iterable see the enclosing scope. A class-method call links to the method without inventing a constructor call.

#437's module aliases follow the same rules: they now come from unconditional module-level imports only, so a function-local import no longer leaks into other functions, and a module-level reassignment drops the alias. Python named imports feed call resolution only; they do not emit identifier references edges.

Evidence

The #465 reproduction on Windows 11 with Node 22:

Probe main This branch
graft callers validate_result . no indexed callers run (consumer.py)
graft callers Workspace.open . no indexed callers run (consumer.py:7)
graft callers parse_value . no callers for either definition run_duplicate for first/helpers.py; none for second/helpers.py

graft callers Workspace . still reports no callers, since the call goes to the method.

test/graph-python-named-imports.test.ts has 19 tests. Against #437's code alone, the resolution and shadowing cases fail. The eight added last cover module-alias scope, match captures and class bodies, walrus targets in headers and type aliases, local imports in another def, defaults and comprehension iterables, unresolved imports, submodule anchoring, and the absence of references; each case produced a wrong edge or dropped a right one in an earlier version of this commit. The three Python test files pass 26 of 26 on Node 22 and Node 20.

Full suite on Windows with Node 22: main passes 1,245 and fails 16; this branch, rebased on main, passes 1,267 with the same 16 failures (the blast, diff and ensureGitignored tests, which depend on Git fixture commits and .gitignore handling on this machine). The 22 extra passes are #437's 3 tests and the 19 in test/graph-python-named-imports.test.ts.

Risk

Python graphs gain call edges for named imports. They lose edges that a global name match produced for shadowed, local or conditional imports, and the edges #437 drew from a function-local import into other functions. Other languages are unchanged.

Unverified

  • The full suite ran on Node 22.23.2 only; on Node 20.20.2, the CI version, I ran the three Python test files.
  • Linux was not run locally; CI covers it.
  • By design, named imports inside functions or conditional blocks stay unresolved (supporting them needs per-scope binding tables), and a class-body call that comes before a same-named class-body assignment is dropped as well.
  • A package that re-exports a name in __init__.py next to a same-named submodule file can still resolve to the submodule; telling them apart needs an index of what each __init__.py binds.
  • Imports inside a class body are not modelled: a call through one in that class body is dropped when fix(python): resolve calls through an imported module alias, without guessing past it #437 would have resolved a module alias, and it can still reach a global name match, as on main. PEP 695 type parameters that reuse an imported name are not modelled either.

artemjeglov-bit and others added 2 commits September 28, 2026 08:25
A layered Python codebase imports the module, not each function, so most of
its cross-module traffic reads `billing.charge(...)`. Such a call reached the
resolver as a member call with no inferable receiver type and was dropped —
correctly, under the drop-rather-than-guess rule, since a bare `charge` says
nothing about which file it lives in.

The missing half was the import binding: collectImportedSymbols covers
TypeScript and PHP and returned an empty map for Python, so the module an
alias names never reached the call site. With a module table, the call states
both halves — the file (the import) and the symbol (the attribute) — which is
the same evidence a named import already gives a `references` edge, and
resolves exactly. No bare-name fallback is added.

Measured on a 375-file FastAPI service: 319 module-alias calls, of which the
graph knew 1; now all 319. Cross-file Python call edges go 2009 -> 3038, and
every one of them is confirmed by pyright's call hierarchy — zero edges the
compiler does not also report. Build time is unchanged (5.2s), where
`--lsp` on the same repo costs 22s.

Two supporting fixes fall out of the same gap: dotted and relative imports now
resolve to in-repo files instead of staying external strings (suffix matching,
as Java and PHP already do here), and `import a.b as x` produces an import
edge at all — importSpecifier never looked inside an `aliased_import`.
Builds on the module-alias resolver from trailhq#437. Unconditional module-level
named imports now resolve inside the imported file, including aliases,
class methods and a function whose name is defined in more than one file.

A name an import binds resolves through that import or not at all. A call
through a shadowed binding, or through a local or conditional import in
scope that does not resolve yet, is dropped instead of matched to a global
name. Shadowing covers parameters, assignments, loop, with and except
targets, walrus (including one in another definition's header), del, match
captures, type aliases, local imports, and lambda, comprehension and
class-body bindings. Defaults, annotations and the leftmost comprehension
iterable see the enclosing scope, and a def's own imports apply only inside
it. Module aliases follow the same rules, and `from pkg import submodule`
resolves inside its own package, package directory first, never through an
ambiguous one. Python named imports feed call resolution only, not
identifier references.

Refs trailhq#465
@trailhq-graft

trailhq-graft Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🌱 graft blast radius

1 area changed → 4 areas can be affected. 10 dependent symbols, depth 2.
Tests: 1 area updated its tests.
Tag: @anirudhkumar-nanonets — 5 of 5 areas · @shhdwi — Graph Extraction, Dependency Graph Construction

flowchart TB
  A0(("Dependency Graph Construction<br/>5 symbols"))
  A1(("Graph Engine<br/>2 symbols"))
  A2(("Symbol Overlap Detection<br/>2 symbols"))
  A3(("Pull Request Review<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Dependency Graph Construction 5 src/graph/build.ts:L204-L472 buildGraph — calls, depth 1 Graph Extraction
Graph Engine 2 src/engine.ts:L92-L102 graph — calls, depth 2 Graph Extraction
Symbol Overlap Detection 2 src/notes/edit-overlap.ts:L77-L92 symbolsIn — calls, depth 1 Graph Extraction
Pull Request Review 1 src/app/review.ts:L45-L99 reviewPullRequest — calls, depth 2 Graph Extraction
Who knows this code — 2 people across 5 areas
Area Who knows it
Graph Extraction · changed @shhdwi — 13 commits, last 4d ago · @anirudhkumar-nanonets — 9 commits, last 2mo ago
Dependency Graph Construction · affected @anirudhkumar-nanonets — 21 commits, last 5d ago · @shhdwi — 14 commits, last 4d ago
Graph Engine · affected @anirudhkumar-nanonets — 17 commits, last 5d ago
Symbol Overlap Detection · affected @anirudhkumar-nanonets — 1 commit, last 3d ago
Pull Request Review · affected @anirudhkumar-nanonets — 3 commits, last 2mo ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 10 dependent symbols, grouped by area

Dependency Graph Construction — 5 symbols in 5 files

  • src/graph/build.ts:L204-L472 — buildGraph (calls, depth 1)
    219: const files = listSourceStats(root, outDir, repoFiles);
  • src/graph/check.ts:L59-L159 — checkGraph (calls, depth 1)
    114: ? extractFile(rel, source, lang)
  • src/graph/container.ts:L152-L213 — extractContainer (calls, depth 1)
    175: ? extractFile(rel, script, lang.inner)
  • src/graph/refresh.ts:L151-L228 — ensureFreshGraph (calls, depth 2)
    200: // parallel MCP tool calls on one edited file produce four full rebuilds, three
  • src/graph/workspace.ts:L631-L656 — federateCheck (calls, depth 2)

Graph Engine — 2 symbols in 1 file

  • src/engine.ts:L92-L102 — graph (calls, depth 2)
  • src/engine.ts:L83-L85 — checkGraph (calls, depth 2)

Symbol Overlap Detection — 2 symbols in 1 file

  • src/notes/edit-overlap.ts:L77-L92 — symbolsIn (calls, depth 1)
    82: ? extractFile(rel, source, lang).nodes
  • src/notes/edit-overlap.ts:L188-L219 — functionOverlaps (calls, depth 2)
    197: const others = cache.branches.filter((b) => b.author && b.author !== me && b.files.includes(rel));

Pull Request Review — 1 symbol in 1 file

  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 2)
    64: const report = blastRadiusIn(graph, contextDir, diff.files, diff.basis, DEPTH);
Test signal per changed area — 1 ✓

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • ✓ Graph Extraction — 2 of 16 reached · 1 test file changed here: test/graph-python-named-imports.test.ts
    • not reached: walk, inHeader, collectPythonBindingNames, bind, visit, visit, collectPythonWalrusTargets, joinModulePath, …6 more
38 test suites also reference this code

47 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/container-extract.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-bindings.test.ts
  • test/graph-cross-language.test.ts
  • test/graph-extract-dedup.test.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-native-fallback.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • …18 more

graft blast · origin/main...HEAD · depth 2 · 4 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Sep 28, 2026
ryanduguid added a commit to ryanduguid/ryanduguid that referenced this pull request Sep 28, 2026
## Summary

Adds the two forks created on 28 September 2026 to FORKS.md: qmd
([tobi/qmd#1006](tobi/qmd#1006)) and Graft
([trailhq/Trail-Code#490](trailhq/Trail-Code#490)). It
also records what merged after the 27 September review:
[frappe/erpnext#59499](frappe/erpnext#59499),
which a maintainer opened from the erpnext fork's commit, and eight
openaccountants pull requests (#173 to #182).

- openaccountants has no open pull request now, so it moves to the
no-open-contribution list.
- The hledger fork shows as archived, matching #52.
- The Furtim row picks up six newer camofox-browser pull requests.
- The recent-contributions list now covers everything since 13 September
2026, so the earlier entries stay.

## Evidence

Regenerated from `users/ryanduguid/repos` and `search/issues`
(`author:ryanduguid is:pr`): 74 open pull requests, each matched to a
fork, and 8 merged since 27 September. 43 of 49 forks hold an open
upstream pull request.

## Merge danger

**Door:** two-way. **Blast radius:** one document

## Unverified

None.
github-actions Bot added a commit that referenced this pull request Oct 17, 2026
ryanduguid added a commit to ryanduguid/ryanduguid that referenced this pull request Oct 17, 2026
## What this changes

Rebuilds the current-state sections of FORKS.md from the GitHub API on
17 October 2026. The fork count goes from 49 to 115, and 101 forks now
hold an open upstream PR.

- **Active forks:** regenerated from the fork list and open PRs by
author.
- **Recent contributions:** every earlier entry is kept, and PRs that
merged or closed since the 28 September review are added.
- **Forks with no open contribution:** the list is refreshed, with a
dated note that no fork is archived any more and that 40 merged fork
branches were deleted.
- **Preserved for retirement, Pruned forks and Retention rule:**
unchanged.

## Why

FORKS.md was last reviewed on 28 September and listed 49 of what are now
115 forks. Every fork's description points here.

## Verification

```
# every PR link in the previous FORKS.md still appears (one moved to the upstream's new name, trailhq/Trail-Code#490)
git show HEAD~1:FORKS.md | grep -oE '\(https://github.com/[^)]+/pull/[0-9]+\)' | sort -u | while read l; do grep -qF "$l" FORKS.md || echo "LOST $l"; done
# LOST (trailhq/Trail-Code#490)
```

PRs that the old file listed as open were checked one by one with `GET
/repos/{owner}/{repo}/pulls/{n}`, because the search API missed four
closed jo-inc/camofox-browser PRs.

## Final review

- Reviewed head commit: see the PR head
- Review type (self-review or independent): self-review
- Review record and scope: FORKS.md only
- Findings and disposition: the search index missed some closures; fixed
by checking each PR directly
- Provider results and coverage gaps: open-PR rows come from the search
API, which can lag; a PR opened or closed in the last few minutes may be
missing
- Remaining risks and unrun checks: no link checker was run. Door:
two-way. Blast radius: documentation.

## Checklist

- [x] No client, taxpayer, employee or payroll data, and no credentials,
tokens, tenant IDs or `.env` contents, appear in the diff, the commit
messages or this description.
- [x] The change is single-purpose.
- [ ] New or changed behaviour has a test that fails without the change.
(Documentation only.)
- [x] Documentation, README and any docstring that describes the changed
rule were updated together with it.
- [x] Any new dependency is justified in the description, or there is
none.

---------

Co-authored-by: Ryan Duguid <ryanduguid@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request Oct 18, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python callers misses aliased imports, module aliases and class-method calls

2 participants