fix(python): resolve ambiguous absolute imports by proximity, not first-writer - #3445
fix(python): resolve ambiguous absolute imports by proximity, not first-writer#3445burgetjp wants to merge 1 commit into
Conversation
…st-writer
`_resolve_cross_file_imports` resolved `from app.config import Settings` in
three steps: exact directory-qualified stem, unique suffix match, then a
bare-stem index. That last index kept only the first file written for a given
basename ("first writer wins when names collide"), so when the first two steps
could not disambiguate, every importer of that module name repo-wide bound to
whichever package happened to be extracted first.
A monorepo where two services each ship their own `app/` package and each
define `Settings` therefore got `uses` edges at INFERRED/0.95 from one
service's `create_app()` and tests into the *other* service's class. The two
share no import, no call and no field, yet the mis-bound class became the
graph's highest-betweenness node — a false edge spanning communities that
nothing else connects is, structurally, the most valuable kind.
The bug needs two collisions at once (a shared module basename AND a shared
class label inside it), which is why it surfaces as one wrong class rather
than broad noise: `from app.state import BoardState` mis-resolves to the other
package's `state`, finds no `BoardState`, and drops silently.
Resolve an ambiguous module path by proximity instead: the candidate sharing
the longest leading path with the importing file, which is what Python's own
sys.path resolution amounts to. On a genuine tie, emit nothing rather than
guess. Unambiguous single-suffix-match imports are unaffected.
On the reporter's repo this removed 8 phantom cross-service edges and added 14
correct intra-service ones over an identical 101-file corpus (981 nodes both
ways), and dropped the mis-bound class's betweenness from 0.048 to 0.034.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 2 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_\_getattr\_\_ changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_\_getattr\_\_ behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"name":"'save\_query\_result'"\}, the old code produced raises AttributeError but the new code produces \<function save\_query\_result at 0x7fb4ec45c400\>. Paste that input straight into a regression test.
Behavior changes: run\_benchmark changes behavior, here is the input that shows it.
The verifier found a concrete input on which run\_benchmark behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"graph\_path":"None","corpus\_words":"None","questions":"None"\}, the old code produced raises TypeError but the new code produces \{'corpus\_tokens': 1076600, 'corpus\_words': 807450, 'nodes': 16149, 'edges': 30311, 'avg\_query\_tokens': 6682, 'reduction\_ratio': 161\.1, 'per\_question': \[\{'question': 'how does authentication…. Paste that input straight into a regression test.
Graphify review — findings
Adds a full CI/CD and repo-hygiene layer for the graphify project: a CI workflow that runs pytest on Python 3.10/3.12, verifies end-to-end install, guards that skillgen-generated artifacts match their fragments, and runs bandit/pip-audit non-blocking. Adds publish.yml to publish graphifyy to PyPI via OIDC on release (refusing to publish when the pyproject version and release tag disagree) and release-graph.yml to build, cluster, and attach a self-graph HTML/JSON asset using AST-only extraction. Adds .dockerignore, .gitattributes (marks HTML as vendored so Linguist reports Python), an expanded .gitignore, a pre-commit config wiring the skillgen check plus ruff, and an AGENTS.md.
Worth a look
- Manual workflow can publish arbitrary checked-out ref to PyPI —
.github/workflows/publish.yml:16· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Docker image defaults to unauthenticated network HTTP service —
Dockerfile:25· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- build deduplicates nodes and edges but leaves hyperedges pointing at removed node ids —
graphify/build.py:119· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- build() changes default graph identity semantics by enabling deduplication —
graphify/build.py:119· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- build_merge() default bypasses the existing shrink safety guard —
graphify/build.py:191· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 15788 functions depend on the 15788 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 565 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
build_from_json()— 199 callers, 19 callees - new:
deduplicate_entities()— 75 callers, 22 callees - new:
detect()— 108 callers, 15 callees - new:
build_merge()— 65 callers, 14 callees - new:
to_obsidian()— 38 callers, 14 callees - new:
save_semantic_cache()— 58 callers, 9 callees - …and 231 more — each is listed as a finding
Verification — 15788 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit 3f82bf7 (diverged from this PR's base — delta is approximate).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 15788 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
263 of 263 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— impact, changed-test, full-run-safetytests/test_affected_member_seed.py— impact, changed-test, full-run-safetytests/test_agents_platform.py— impact, changed-test, full-run-safetytests/test_analyze.py— impact, changed-test, full-run-safetytests/test_anthropic_custom_endpoint.py— changed-test, full-run-safetytests/test_antigravity_install.py— changed-test, full-run-safetytests/test_apm_fallback_version.py— impact, changed-test, full-run-safetytests/test_architecture_doc.py— impact, changed-test, full-run-safetytests/test_astro_extraction.py— impact, changed-test, full-run-safetytests/test_astro_import_ids.py— impact, changed-test, full-run-safetytests/test_atomic_canvas_export.py— impact, changed-test, full-run-safetytests/test_atomic_version_stamp.py— impact, changed-test, full-run-safetytests/test_atomic_writes.py— impact, changed-test, full-run-safetytests/test_backend_extras.py— impact, changed-test, full-run-safetytests/test_benchmark.py— impact, changed-test, full-run-safetytests/test_benchmark_raw_graph.py— impact, changed-test, full-run-safetytests/test_build.py— impact, changed-test, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, changed-test, full-run-safetytests/test_build_merge_shrink_guard.py— impact, changed-test, full-run-safetytests/test_builtin_global_type_refs.py— impact, changed-test, full-run-safetytests/test_cache.py— impact, changed-test, full-run-safetytests/test_callflow_html.py— impact, changed-test, full-run-safetytests/test_cargo_introspect.py— impact, changed-test, full-run-safetytests/test_carried_hyperedge_remap.py— impact, changed-test, full-run-safetytests/test_case_sensitive_resolution.py— impact, changed-test, full-run-safetytests/test_charmap_encoding.py— impact, changed-test, full-run-safetytests/test_chunking.py— impact, changed-test, full-run-safetytests/test_cjs_module_extension.py— impact, changed-test, full-run-safetytests/test_claude_cli_backend.py— impact, changed-test, full-run-safetytests/test_claude_md.py— impact, changed-test, full-run-safetytests/test_cli_broken_pipe.py— changed-test, full-run-safetytests/test_cli_export.py— impact, changed-test, full-run-safetytests/test_cli_help.py— changed-test, full-run-safetytests/test_cluster.py— impact, changed-test, full-run-safetytests/test_codebuddy.py— impact, changed-test, full-run-safetytests/test_community_hub_labels.py— impact, changed-test, full-run-safetytests/test_community_labels_skill.py— impact, changed-test, full-run-safetytests/test_confidence.py— impact, changed-test, full-run-safetytests/test_corrupt_graph_json.py— impact, changed-test, full-run-safetytests/test_cpp_nested_and_cli.py— impact, changed-test, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, changed-test, full-run-safetytests/test_cpp_preprocess.py— impact, changed-test, full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, changed-test, full-run-safetytests/test_cross_language_call_resolution.py— impact, changed-test, full-run-safetytests/test_cross_repo_member_calls.py— impact, changed-test, full-run-safetytests/test_cross_repo_shared_types.py— impact, changed-test, full-run-safetytests/test_csharp_call_site_generic_args.py— impact, changed-test, full-run-safetytests/test_csharp_enum_members.py— impact, changed-test, full-run-safetytests/test_csharp_field_generic_args.py— impact, changed-test, full-run-safetytests/test_csharp_generic_callsites.py— impact, changed-test, full-run-safety- … and 213 more
non-code file(s) changed (
.dockerignore,.gitattributes,.github/FUNDING.yml,.github/workflows/ci.yml,.github/workflows/publish.yml…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) not in the graph (
tests/eval_attention.py,tests/fixtures/sample.dm) — deleted or unindexed, so their dependent tests can't be found; running the full suite for safety
changed code file(s) with no mapped test (
.github/workflows/ci.yml,.github/workflows/publish.yml,.github/workflows/release-graph.yml,AGENTS.md,ARCHITECTURE.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Behavior changes: \_\_getattr\_\_ changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_\_getattr\_\_ behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"name":"'save\_query\_result'"\}, the old code produced raises AttributeError but the new code produces \<function save\_query\_result at 0x7fb4ec45c400\>. Paste that input straight into a regression test.
Behavior changes: run\_benchmark changes behavior, here is the input that shows it.
The verifier found a concrete input on which run\_benchmark behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"graph\_path":"None","corpus\_words":"None","questions":"None"\}, the old code produced raises TypeError but the new code produces \{'corpus\_tokens': 1076600, 'corpus\_words': 807450, 'nodes': 16149, 'edges': 30311, 'avg\_query\_tokens': 6682, 'reduction\_ratio': 161\.1, 'per\_question': \[\{'question': 'how does authentication…. Paste that input straight into a regression test.
No difference found (not proven): No behavior difference found in main (not a proof).
The verifier ran both versions of main on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_cross\_community\_surprises (not a proof).
The verifier ran both versions of \_cross\_community\_surprises on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_cross\_file\_surprises (not a proof).
The verifier ran both versions of \_cross\_file\_surprises on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_file\_category (not a proof).
The verifier ran both versions of \_file\_category on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_is\_file\_node.
The verifier did not have enough to check \_is\_file\_node, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
Could not verify: Could not verify \_surprise\_score.
The verifier did not have enough to check \_surprise\_score, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in god\_nodes (not a proof).
The verifier ran both versions of god\_nodes on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify graph\_diff.
The verifier did not have enough to check graph\_diff, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: the input domain has 25 values but only 5 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify suggest\_questions.
The verifier did not have enough to check suggest\_questions, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_query\_subgraph\_tokens (not a proof).
The verifier ran both versions of \_query\_subgraph\_tokens on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify print\_benchmark.
The verifier did not have enough to check print\_benchmark, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly KeyError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build.
The verifier did not have enough to check build, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify cache\_dir.
The verifier did not have enough to check cache\_dir, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify cached\_files.
The verifier did not have enough to check cached\_files, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify check\_semantic\_cache.
The verifier did not have enough to check check\_semantic\_cache, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify clear\_cache.
The verifier did not have enough to check clear\_cache, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify file\_hash.
The verifier did not have enough to check file\_hash, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify load\_cached.
The verifier did not have enough to check load\_cached, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify save\_cached.
The verifier did not have enough to check save\_cached, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify save\_semantic\_cache.
The verifier did not have enough to check save\_semantic\_cache, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_split\_community (not a proof).
The verifier ran both versions of \_split\_community on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify cluster.
The verifier did not have enough to check cluster, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
The bug
_resolve_cross_file_importsresolves an absolutefrom app.config import Settingsin three steps: exact directory-qualified stem, unique suffix match, then a bare-stem index. That last index keeps only the first file written for a given basename — its own comment says "first writer wins when names collide (inherently ambiguous)".So when the first two steps can't disambiguate, every importer of that module name repo-wide binds to whichever package happened to be extracted first.
I hit this on a repo where two services each ship their own
app/package and each defineSettings:Byte-identical import lines resolving to different files. The result was
usesedges at INFERRED/0.95 from one service'screate_app()and tests into the other service's class. The two services share no import, no call, and no field —grep -r service-sonos service/returns nothing — yet the mis-boundSettingsbecame the graph's highest-betweenness node, because a false edge spanning communities that nothing else connects is structurally the most valuable kind. It was the top "surprising connection" and the top suggested question inGRAPH_REPORT.md.The bug needs two collisions at once — a shared module basename and a shared class label inside it — which is why it surfaces as one confidently wrong class rather than broad noise.
from app.state import BoardStatemis-resolves to the other package'sstate, finds noBoardState, and drops silently.Note it also needs enough of a corpus to defeat the earlier resolution steps: on a cut-down tree containing only the two services,
v8resolves everything correctly and no phantom appears. Which package wins (and so the direction and exact count of the false edges) depends on extraction order.The fix
Resolve an ambiguous module path by proximity: the candidate sharing the longest leading path with the importing file, which is what Python's own
sys.pathresolution amounts to. On a genuine tie, emit nothing rather than guess.Unambiguous single-suffix-match imports take the same path as before.
bare_to_qualifiedbecame write-only once_nearest_stemreplaced its only read, so it's removed and its stale comment updated.Effect on the reporting repo
Full 101-file corpus,
cache_rootat the repo root,parallel=False, 981 nodes both ways:callsedges lostThe 14 additions are the same importers finally reaching their own package's
Settings,NowStateandNow. On the built graph this dropped the mis-bound class's betweenness from 0.048 to 0.034, and that service re-clustered from five fragments into one coherent community.Verification
tests/test_phantom_cross_package_import.py— 6 tests, red before / green after. Covers the phantom, that each importer still reaches its own target (dropping everything would be over-correction), an importer in a sibling directory, the unambiguous case, and_nearest_stemdirectly including tie and empty input.v8(test_ollama_retry_cap,test_skillgen) excepttest_ts_normalizer_scales_linearly_on_large_files, a wall-clock assertion that passes 3/3 in isolation and flakes under full-suite load._nearest_stemis only reachable from the Python resolver.ruff checkclean;pyright43 errors before and after.