Repository navigation
fix: resolve import-alias calls in Python and TypeScript (#258) - #274
Open
JunaidAhamed-7777 wants to merge 16 commits into
Open
JunaidAhamed-7777 wants to merge 16 commits into
JunaidAhamed-7777 wants to merge 16 commits into
Conversation
…db71#258) `from money import money_to_minor_units as to_minor` then `to_minor()` records no edge: nothing is named `to_minor`. When another module does define a function named like the alias, the call binds to that one instead, so the edge exists and points at the wrong function. These parse real Python with the real parser and build the graph. They cover a plain module, a dotted module with a same-named decoy, a relative module, several aliases in one import (comma and parenthesised), an alias shadowing an unrelated function, an alias that is never called, and a wrapper named like the function it imports. All seven fail on main.
Two behaviours the alias fix must keep, both passing on main: a function the caller's own file defines under the alias name still wins over the import, and an alias of a name from a module outside the project (`from numpy import array as arr`) links to nothing, even when the project defines a function with the imported name.
An import node's references hold the names the importing file can call. For `from money import money_to_minor_units as to_minor` that was only `to_minor`, so the name the module defines was lost before the graph builder ever saw it. `alias:to_minor:money_to_minor_units` keeps both, in the same prefix style as `field:` and `returns:`.
The builder keyed its import map on the local name only, so `to_minor()` was looked up as `to_minor`: nothing, or a same-named function in an unrelated module, which then passed import validation because `to_minor` was imported. An `alias:` reference now records the imported name next to the local one. A call to the alias resolves the imported name in the module the import named (`SymbolTable::resolve_imported`): that module's file first, then a package of that name. A module the project doesn't define resolves nothing, methods and the importing file are never candidates, and a definition in the caller's own file still shadows the import. A later plain import of the same name rebinds it.
) `from money import money_to_minor_units as to_minor` recorded only `to_minor` on the import node, so the graph never learned which function the alias names and `to_minor()` produced no edge. Both the bare and the parenthesised import lists now record `alias:to_minor:money_to_minor_units`. `Y as Y` stays a plain import. The end-to-end alias tests in arbor-graph now pass.
Import nodes now carry `alias:` references. A cache or saved graph from the previous extractor holds the old shape and would keep answering without the alias edges until each file happened to change.
`to_minor()` after `from money import money_to_minor_units as to_minor` now reaches money.py, so the corpus reported the recorded miss as resolved. The edge stays expected; only the known_missing entry and the notes change.
…b71#258) orders.py imports money.money_to_minor_units as to_minor while convert.py defines a real to_minor that report.py imports and calls, and legacy_money.py defines a second money_to_minor_units. The alias call must reach money.py only; the other two targets are forbidden. Hand-audited. Arbor 3.0.5 indexes this fixture as orders.total_cents -> convert.to_minor at confidence 1.0 and misses the money.py edge.
…Anandb71#258) billing/money.py and legacy/money.py both define money_to_minor_units. shop/orders.py aliases it from billing.money; legacy/invoices.py aliases it from .money. Each alias call must reach only the module its import names, so the crossed edges are forbidden. Hand-audited. Arbor 3.0.5 records no call edges for this fixture.
…tion
`import { formatName as fmt } from "./format"` then `fmt()` has the
same gap as Python's `from x import y as z`: the TypeScript parser
records only `fmt`, so the call binds to an unrelated `fmt` in another
file, or to nothing. Fails on main and on this branch until the parser
records the imported name.
`import { formatName as fmt } from "./format"` recorded only `fmt`, so
`fmt()` bound to an unrelated `fmt` elsewhere in the project, or to
nothing. Named specifiers with an alias now record
`alias:fmt:formatName`, and the shared graph resolution from Anandb71#258 sends
the call to format's `formatName`. This covers .ts, .tsx and the JS
extensions, which share the extractor.
`{ default as Widget }` keeps recording `Widget`, as a default import
does, because no export is named `default`. `{ a as a }` stays plain.
app.ts imports formatName as fmt from ./format while legacy/fmt.ts defines a real fmt that legacy/report.ts imports and calls. The alias call must reach format.ts; the crossed edges are forbidden. Hand-audited. Arbor 3.0.5 indexes this fixture as app.label -> legacy/fmt.fmt at confidence 1.0 and misses format.ts.
`import { POST as subscribe } from "../route"` names one file, but the
alias resolution added for Anandb71#258 matched the module by its last segment,
so every `route.ts` that exports POST was a candidate. With up to four
of them the call gets an edge to each, raised to 0.95 by import
validation; before Anandb71#258's fix these aliases produced no edge at all.
Python's `from ..money import f as g` has the same shape.
Covers a Next.js-style route test, a parent-package Python import, and a
relative import of a file that only re-exports the name. All three fail
on this branch.
`../route`, `./format`, `.money` and `..core.money` each name one place. resolve_imported matched them by their last segment, so any same-named module was a candidate and a Next.js-style app could get an edge from one route's test to every route's POST. A relative module is now resolved against the importing file's directory: the module's own file or its package `index`/`__init__` first, then anything inside that package. `./format.js` names `format.ts`, and an import that climbs out of the tree resolves nothing. `from . import f as g` was a free-for-all with a same-directory tie break; it now reads the importing package, so the symbol-table test added with the alias fix on this branch is renamed and its expectations changed to the anchored results. Non-relative modules are unchanged, and one that normalizes to nothing now resolves nothing instead of matching any definition. The three relative tests in import_alias_tests now pass.
Contributor
Author
Anandb71
requested changes
Oct 13, 2026
Anandb71
left a comment
Anandb71
left a comment
Owner
There was a problem hiding this comment.
Thanks for this. It's a careful PR: small commits, failing tests before fixes, an A/A control on the benchmarks, and dogfood numbers. The main fix is right. to_minor() now reaches money.money_to_minor_units, and the old wrong 1.0 edge to a same-named function is gone. I'd like two small changes before merging, both in the new non-relative-module path.
What I checked
- CI is green on
f8ddc15: 13/13, includingeval_corpus, clippy-D warnings, fmt, criterion, the cross-platform builds, installer and npm smoke tests, cargo-deny and arbor-impact. It merges cleanly intomain(e10cb78). - I read the full diff and the code around it:
add_nodes,resolve_edges,apply_import_validation,resolve_imported,pick,module_isandentry_belongs_to_module. - Nothing else reads import-node
references.changes.rsusesqualified_nameonly, andsync_serveris #271. Theextract-5bump is needed and is enough. - No recursion is added, and every choice between candidates goes through the sorted
pick. - I built the branch in a scratch worktree and ran extra builder tests with absolute paths, as
arbor indexproduces them.- These came out right: TS
from '.'→index.ts, and a nested localdef c()shadowing an aliasc. - The cases below came out wrong.
- These came out right: TS
Requested changes
-
crates/arbor-graph/src/symbol_table.rs:379-394: the non-relative branch doesn't treatpkg/__init__.pyas the module's own file.- Setup:
pkg/__init__.pydefinesf,pkg/sub.pyalso definesf, andapp.pyhasfrom pkg import f as gand callsg(x). - Result:
run -> pkg/__init__.py::f @0.95andrun -> pkg/sub.py::f @0.95. - Cause:
normalize_module_pathdropsindex/modbut not__init__, somodule_ismisses the package file. Then theentry_belongs_to_modulefallback takes the whole package. - The relative branch already gets this right through
is_module_file, and the doc comment says "the module's own file first". - Fix: in the non-relative
in_modulefilter, also accept an entry whose stem is__init__and whose parent normalizes towanted. Please add a test with both definitions present.
- Setup:
-
CHANGELOG.md:11(and the PR description): "An alias imported from a module outside the project links nothing" isn't true when the external module's name matches a project path.module_ismatches the last segment, andentry_belongs_to_modulematches any segment of the absolute file path. Both of these are verified on the branch:import { request as httpRequest } from 'http', with the project'ssrc/lib/http.tsexportingrequest, givesgo -> src/lib/http.ts::request @0.95.maingives no edge for the alias.from flask import render_template as rt, in a checkout under/home/u/flask/…that defines its ownrender_template, gives an edge at 0.95.
- The unaliased forms already bind like this on
main(at 1.0), so aliases now behave like plain imports rather than worse. But the changelog promises more than the code does. - Two options:
- (a) Reword it, e.g. "…links nothing unless a project file or directory has the same name".
- (b) Skip single-segment bare specifiers for JS-family callers.
http,pathandreactare packages or Node builtins; baseUrl imports likecomponents/Buttonhave more than one segment.
- (a) is enough for this PR, and (b) can be a follow-up.
Nits / follow-ups (not blocking)
builder.rs:422: whenresolve_importedreturnsAmbiguous,apply_import_validationraises every candidate to 0.95, because the alias is in the import map.- Example:
from billing import convert as conv, withbilling/a.pyandbilling/b.pyboth definingconvert, gives two 0.95 edges. - Plain
from billing import convertdoes the same today, so this is #272 territory. It may still be worth not boosting an ambiguous alias resolution.
- Example:
- Pre-existing, not from this PR:
arbor index --changed-only(commands.rs:1611-1672) rebuilds frombase_graph.nodes(). Import nodes are never graph vertices, so unchanged files lose theirimport_map, and now theirimport_aliases. After an incremental index, alias calls in unchanged files go back to the old name lookup. This is worth an issue next to #271. - #221 will need a rebase after this lands. It already conflicts with
main, and this PR adds conflicts at theGraphBuilderfields andnew(). The two fit together: #221's visibility check keys on the local name, and this PR keeps the local name as theimport_mapkey.
Happy to re-review once 1 and 2 are in.
13 of 19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Calls through an aliased import now reach the imported function. This covers Python
from m import f as gand TypeScript/JavaScriptimport { f as g } from "./m".Before this PR,
g()recorded no edge. Worse, when another file defined a function namedg, the call was bound there at confidence 1.0. Both are fixed, and relative modules (.m,..m,./m,../m) are read from the importing file's directory, so a same-named module elsewhere is never the target.Fixes #258. Addresses case 2 of #178 (
from x import y as z). #178's re-export and module-alias cases are not changed here; see Known limitations.Follow-up issues opened from this work: #271, #272, #273. #178 covers (b) and (c).
Type of Change
Changes Made
crates/arbor-core/src/node.rs,lib.rs: new reference encodingalias:<local>:<original>withimport_alias_ref/import_alias(prefix constantIMPORT_ALIAS_REF_PREFIX).crates/arbor-core/src/languages/python.rs:from m import f as g(bare and parenthesised lists) recordsalias:g:f.crates/arbor-core/src/languages/typescript.rs:import { f as g }recordsalias:g:f. This covers.ts,.tsxand the JS extensions, which share the extractor.crates/arbor-graph/src/builder.rs: a per-file alias map. Calls to an alias resolve throughSymbolTable::resolve_imported.crates/arbor-graph/src/symbol_table.rs:resolve_imported, plus the relative-module helpersrelative_module_pathandis_module_file.crates/arbor-graph/src/store.rs:CACHE_VERSIONbumpedextract-4→extract-5.eval/: retirespy-alias'sknown_missing; adds thepy-alias-shadow,py-alias-dottedandts-aliasfixtures.CHANGELOG.md: entry under[Unreleased].29 files changed, +1087 / −43.
Root cause (line numbers at
e10cb78)crates/arbor-core/src/languages/python.rs:244-256(bare list) and:265-275(parenthesised list) pushed only the alias for analiased_import.crates/arbor-core/src/languages/typescript.rs:462-471did the same forimport_specifier.from money import money_to_minor_units as to_minor, the import node'sreferencesheld onlyto_minor.crates/arbor-graph/src/builder.rs:116-119insertedimport_map[file][to_minor] = "money".resolve_edgesthen looked up a symbol namedto_minor.eval/corpus/py-aliashad a recordedknown_missing.apply_import_validation(builder.rs:357-360) treated the match as corroborated becauseto_minoris imported. Measured ate10cb78:py-alias-shadow:orders.py::total_cents -> convert.py::to_minorat confidence 1.0, while the real targetmoney.py::money_to_minor_unitshad no edge.ts-alias:app.ts::label -> legacy/fmt.ts::fmtat 1.0, whileformat.ts::formatNamehad no edge.import { POST as subscribe } from "../route"bound a route test to an unrelateduseMotionPreference.ts::subscribeat 1.0.Design
Encoding:
alias:<local>:<original>The import node now records both names, in the same prefix style as
field:andreturns:. This is safe for three reasons:add_nodesreads them into the import map andcontinues (builder.rs:103-126). Outside the parsers,GraphBuilderis the only reader of an import node'sreferences; the sled store persists nodes opaquely and replays them through the builder.import_mapkey. So everything keyed on it keeps working: import validation,resolve_ref_with_imports's tie-break, and thelocal_definition_still_shadows_an_importrule. An alias call gets the same final confidence as an unaliased call to a uniquely named import:ViaImport0.93, raised to 0.95 by validation.Y as Ystays plain. TS{ default as Widget }keeps recordingWidget, as a default import does, because no export is nameddefault. Python and TS identifiers can't contain:.A later plain import of the same local name rebinds it and removes the alias entry (
a_plain_import_rebinds_an_earlier_alias).Resolution:
SymbolTable::resolve_imported(name, module, context_file)GraphBuilder::resolve_aliasruns only for a reference with a known receiver whose file aliased exactly that name. If the caller's own file defines the alias name, it returnsNone: a local definition still wins, as for unaliased imports. Every other reference goes through the unchanged paths:resolve_pathfor::,resolve_ref_with_importsotherwise.resolve_importedrules:Class.f) never qualify. Candidates must be in the caller's language family.from money import f as _f, thendef f(): return _f()),_freaches money'sf, not the wrapper itself../x,../x,.x,..x,.,..) are resolved to one path from the importing file's directory:index/__init__, first;./format.jsnamesformat.ts;pkg.mod,money): the module's own file first (module_is), then anything inside a package of that name (entry_belongs_to_module).numpy,react,@scope/pkg) resolves nothing. Before this PR the alias name also resolved to nothing in that case, so no edge is lost or added there.Cache bump to
extract-5store.rs:9-11says to "bumpextract-Nwhenever extracted nodes or references change shape". The CLI also stamps saved graphs withCACHE_VERSION, so a graph from another extractor is rebuilt rather than reused. Without the bump, upgraded users would keep cached import nodes withoutalias:entries, and keep the wrong or missing edges, until each file happened to change. Indexes rebuild once.Course correction: relative modules
The first version of
resolve_importedmatched every module by its last segment, the same way the existingViaImporttie-break does. Dogfooding on a Next.js app with 12route.tsfiles that each exportPOSTshowed the risk. The fan-out cap happened to drop that call, but with 2–4 same-named route filesimport { POST as handler } from "../route"would have produced an edge to every one at 0.95, wheremainproduced none.8b5a02ffirst added three failing tests: a Next.js-shaped route test, afrom ..money import …parent-package import, and a relative import of a file that only re-exports the name. They produced edges to every same-named module.36f6cedthen anchored relative modules to the importing file.The symbol-table test from the first version (
a_relative_package_alias_prefers_the_callers_directory) was renamed and its expectations changed to the anchored results in that commit. It was added earlier in this PR, not taken frommain.Commit map
2ec9e3aGraphBuilder), all failing onmain2af5a40main: a local definition shadows the alias; an alias from an external module links nothing3904f28alias:<local>:<original>encoding and round-trip testd5b16b2resolve_alias,SymbolTable::resolve_imported, hand-built-node unit tests66e3d71ac6561bCACHE_VERSION→extract-5b6e6609py-aliasknown_missing(plus notes and manifest text)3574db0py-alias-shadowfixtureb3a60d3py-alias-dottedfixture6d00f6b03efaf6d241468ts-aliasfixture99587e6[Unreleased]entry8b5a02f36f6cedf8ddc15Commits are deliberately small: each failing test lands before its fix. Squash-merge is fine.
Testing
cargo test --workspace(the workspace equivalent ofcargo test --all)cargo clippy --workspace --all-targets --all-features -- -D warningsflutter test: not applicableResults on
f8ddc15Run on Windows 11, rustc 1.98.1, with a dedicated
CARGO_TARGET_DIRandTMP/TEMPoutside the home folder (see #273 for why):25 new tests (none removed or weakened)
builder.rsimport_alias_tests.py/.tssource → real parser →GraphBuilder, asserting the exact set of call edgesbuilder.rstestssymbol_table.rsimport_resolution_testsresolve_imported: named module only, package submodule, external module, relative Python, relative TS (./x,../x.js,./utils→index.ts, missing, climbing out), methods and other languages excludednode.rs*as:referencespython.rsf as fimportstypescript.rsa as a,default as X, default plus aliased, namespaceWhich tests fail on
main. I appended the 13 end-to-end tests unchanged (a test-only module) tobuilder.rsate10cb78. Of those 13, 10 fail and 3 pass:a_local_definition_of_the_alias_name_still_shadows_the_import,an_alias_of_an_external_name_links_nothing), anda_relative_alias_to_a_re_export_links_nothing_else, which expects no edge, the same asmain.8b5a02falso failed on this branch before36f6ced. Two failed because edges went to every same-named module; the re-export test failed because the call got an edge to each otherroute.ts.03efaf6.main; they test the new code directly.Eval corpus
main(e10cb78)f8ddc15)known_extrainpy-export-removed:after, #263)known_missingpy-alias)known_extraNew fixtures, all hand-audited:
mainpy-alias-shadoworders.total_cents -> convert.to_minorat 1.0; misses themoney.pyedgepy-alias-dottedts-aliasapp.label -> legacy/fmt.fmtat 1.0; missesformat.tspy-alias-shadow:money.py,legacy_money.pyandconvert.py, with an alias named likeconvert.to_minorand a secondmoney_to_minor_units.py-alias-dotted:billing/money.pyandlegacy/money.py, withfrom billing.money import … as …andfrom .money import … as ….ts-alias:import { formatName as fmt }, plus a reallegacy/fmt.ts::fmtwith its own caller.Every other fixture is unchanged, including
py-real-slice.Benchmarks
The criterion benches don't exercise this change.
crates/arbor-graph/benches/graph_bench.rsbuildsArborGraphs directly (L7-50) and times search, impact, centrality and a query bundle. It never callsGraphBuilderorSymbolTable. I ran them anyway, then timed the code path that did change.Method.
cargo bench -p arbor-graph --bench graph_bench -- --noplot, after clearingtarget/criterion.git archive e10cb78tree, run with--save-baseline base.--baseline basein its ownCARGO_TARGET_DIR, with base's criterion data copied in.A mistake I made and corrected. My first "PR" run shared base's target directory, and cargo silently reused base's bench binary (
Finished … in 1.99s, noCompilinglines). That run was really base against base. It is useful as a noise floor: on identical code it reportedgraph_query_bundle+14.4% andcompute_centrality_100−22.8%. The corrected run compiled the PR's crates.search_symbols_500analyze_impact_depth5_200compute_centrality_100compute_centrality_10kcompute_centrality_10k_warmgraph_query_bundleThe two benches criterion flagged against the PR were re-run in 3 interleaved rounds, each against the same baseline:
search_symbols_500compute_centrality_10kNo consistent direction, and the PR stays inside the band that identical code shows. The code these benches run (
graph.rs,search_index.rs,ranking.rs,impact.rs) is unchanged.Index time, on the path that changed. Release builds,
arbor index . --no-cache, 8 interleaved runs each, read from the CLI's ownIndexed … in N ms. A byte-identical copy of the base binary is included as an A/A control:Medians. Process wall-clock was unusable here: on this Windows machine a freshly written executable paid a ~55–60 ms launch penalty per run, and the A/A copy showed the same penalty.
Dogfood (before → after)
eval/corpus/py-alias.arbor callers money_to_minor_units .onmaingivesNo callers found for 'money_to_minor_units'. On this PR:Callers of 'money_to_minor_units' (1): function total_cents (orders.py:4).eval/corpus/py-real-slice(realitsdangerouscode): all 27 call edges identical, confidences included.from … import … as …lines): 777 → 786 call edges, +9 added, 0 removed.arbor callerswent from 4 (all tests) to 5, adding its only non-test caller.909a2d8): 11080 call edges before and after, with one swap.subscribeedge described above (route test →useMotionPreference.ts::subscribe, 1.0).ReceiptsHome.tsx::ReceiptsHome -> receipts.ts::flagsat 0.95, via a multi-lineflags as findFlagsthat is called atReceiptsHome.tsx:75.subscribealias now resolves to nothing, because../routeonly re-exportsPOST(limitation c).getArbor-dev/arbor-torture(5490a11, 372 files): 1336 → 1337 edges, addinghard/aliased.py::via_alias -> hard/reexport_base.py::original_function.via_module_alias(import hard.reexport_base as base_mod; base_mod.original_function(n)) stays unresolved (limitation b).Known limitations / follow-ups
Not changed by this PR.
(a) The live re-index path bypasses all of this: #271.
arbor bridgeandarbor viz, a saved file is re-parsed byparser_v2and its calls are bound withfind_by_name(..).first()(crates/arbor-server/src/sync_server.rs:718-753ate10cb78). That skips import filtering,SymbolTableand alias resolution.remove_file(L723) also drops every edge into the saved file from other files.orders.pyunchanged made the alias edge disappear and re-boundtotal_centstoconvert.to_minor.convert.pyunchanged removedreport.summary's caller edge.mainloses the incoming edge the same way.arbor servehas no watcher (it indexes once), so it is not affected.(b) Python dotted module calls: covered by #178 (case 3).
import money as m; m.f()andimport money; money.f()produce no edge onmainor here:extract_import(python.rs:192-204ate10cb78) storesimport money as mas an import node named"money as m"with no names.m.money_to_minor_units(python.rs:427-432), which never matches a symbol.(c) Re-exports aren't followed: #178 (case 1). Arbor Cloud's
subscribeexample above is one.(d) Not covered:
class Foo(BaseAlias)): inheritance resolution doesn't consult the alias map. Untested.@/…path aliases: not mapped to directories, so an aliased import through one resolves nothing, as before.require(const { f: g } = require("./m")): not an import statement, so not recorded.(e) Plain non-aliased relative imports still use loose module matching: #272.
from ..money import fandimport { POST } from "../route"link every same-named module at 0.95. Measured onmainand here. The helpers this PR adds for aliases are the suggested fix.(f)
find_workspace_roottreats~/.arboras a project root: #273. On a Windows machine installed withinstall.ps1(default$HOME\.arbor\bin), unmarked folders under home resolve to the home directory. That includes the eval harness's%TEMP%copies, which made a localcargo test -p arbor-graph-cli --test eval_corpusstart indexing the whole profile.Risk, rollout and migration
extract-5). There are no CLI, MCP or config changes.arbor_core::{import_alias, import_alias_ref},arbor_core::node::IMPORT_ALIAS_REF_PREFIXandSymbolTable::resolve_imported. Import nodes'referencescan now containalias:entries.Checklist
cargo fmt, clippy-D warnings)CHANGELOG.md[Unreleased]; doc comments on the new functions)