Skip to content

fix(python): resolve absolute sibling imports to the importing file's directory - #3468

Open
oskibundles-hue wants to merge 1 commit into
Graphify-Labs:v8from
oskibundles-hue:fix/python-sibling-import-resolution
Open

fix(python): resolve absolute sibling imports to the importing file's directory#3468
oskibundles-hue wants to merge 1 commit into
Graphify-Labs:v8from
oskibundles-hue:fix/python-sibling-import-resolution

Conversation

@oskibundles-hue

Copy link
Copy Markdown

Base: v8

The bug

In graphify/extract.py, _import_python uses the bare module name as the
imports_from edge target for an absolute from-import. That id only matches a
file node when the basename is unique across the scan. Two things go wrong:

  • Duplicate basename — a repo with a live module plus a vendored copy of the
    same name leaves the target matching nothing. The edge dangles, is pruned, and
    a real dependency disappears from the graph.
  • Unique basename — the bare id survives as a phantom external node, so the
    dependency is drawn as a third-party package instead of the local file.

The symbol-level pass in extractors/resolution.py doesn't cover it either: it
resolves imported functions and classes, so from m import CONST leaves no trace
while from m import func in the same file resolves normally.

The fix

Probe the importing file's own directory before falling back to the bare name.
That is exactly how the import resolves at runtime for a script that does
sys.path.insert(0, os.path.dirname(__file__)). Setting target_path lets the
existing target_file stamp canonicalize the id, the same way the relative-import
branch already does.

The self-resolution guard is load-bearing: a module named contracting.py doing
from contracting import constants imports the external package of that name, not
itself, and must not gain a fabricated self-loop
(tests/test_import_self_loops.py catches this).

Only absolute from-imports that resolve to an existing sibling file change
behaviour. Everything else keeps the bare-name target it had before.

Tests

Two added to tests/test_python_import_resolution.py:

  • test_absolute_sibling_import_resolves_to_importing_files_directory
    live/layouts.py, a duplicate vendor/layouts.py, and live/build.py doing
    sys.path.insert then from layouts import LAYOUTS. Asserts the imports_from
    edge lands on live/layouts.py and never on the vendored twin. Fails on
    unpatched code, passes with the fix.
  • test_absolute_import_without_sibling_keeps_bare_module_target — pins the
    unchanged path: no sibling on disk means the target stays the bare module id.

Verification

Run on Python 3.12 via uv run --frozen pytest tests/ -q, on this branch's base
commit and again with the change:

passed failed skipped
baseline (v8 @ 3f82bf7) 5358 17 95
with this change 5360 17 95

The 17 failures are byte-identical sets before and after (test_skillgen.py,
test_ollama_retry_cap.py, test_install.py, and one timing-sensitive
test_ts_import_type_arguments.py case) — all pre-existing and unrelated. The +2
passed are the two new tests.

ruff check --config pyproject.toml clean on both changed files.

Effect on a real graph

graphify extract . --code-only over this repo itself, before and after:

  • 26302 → 26306 edges, 12590 → 12589 nodes.
  • worked/example/raw/{api,validator}.py previously pointed at a phantom external
    node processor; they now point at the real worked/example/raw/processor.py,
    and the phantom node is gone (that is the −1 node).
  • worked/httpx/raw/{auth,client,transport,utils}.pyworked/httpx/raw/models.py
    are 4 edges that did not exist before. models.py has a duplicate basename in
    this repo (graphify/extractors/models.py), which is exactly the ambiguity that
    made them dangle; they resolve to the correct sibling, not the twin.
  • Zero imports/imports_from/re_exports self-loops in the resulting graph.

🤖 Generated with Claude Code

… directory

An absolute from-import used the bare module name as the edge target id.
That id only matches a file node when the basename is unique across the
scan, so a repo carrying a vendored copy of the same module left the
target matching nothing: the edge dangled, was pruned, and a real
dependency vanished from the graph. The symbol-level pass does not cover
it either, since it resolves imported functions and classes -- so
`from m import CONST` left no trace while `from m import func` in the
same file resolved normally.

Probe the importing file's own directory first, which is how the import
resolves at runtime for a script that does
`sys.path.insert(0, os.path.dirname(__file__))`. Setting target_path
lets the existing target_file stamp canonicalize the id, exactly as the
relative-import branch already does.

The self-resolution guard is load-bearing: a module named contracting.py
doing `from contracting import constants` imports the external package of
that name, not itself, and must not gain a fabricated self-loop.

Only absolute from-imports that resolve to an existing sibling file
change; everything else keeps the bare-name behaviour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Resolves absolute from X import ... targets in _import_python against the importing file's own directory first, probing for a sibling .py module before falling back to the bare module name. This fixes dangling/pruned import edges when a same-named module is vendored elsewhere in the scan, since the bare name only resolves when the basename is unique. Skips the sibling when it resolves to the importing file itself, so a module can still import an external package of the same name without gaining a self-loop.

Worth a look

  • Sibling absolute import probe misses package directoriesgraphify/extract.py:391 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Multi-level absolute import builds wrong sibling pathgraphify/extract.py:400 · Escalate · medium
    • 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 — 1859 functions depend on the 248 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 565 callers, 43 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 28 more — each is listed as a finding

Verification — 1859 functions in the blast radius were not formally verified this run (proofs are advisory here).

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: 1694 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

104 of 262 test file(s) selected (40%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_astro_import_ids.py — impact
  • tests/test_build.py — impact
  • tests/test_builtin_global_type_refs.py — impact
  • tests/test_case_sensitive_resolution.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_cpp_objc_cross_file_calls.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_cross_language_call_resolution.py — impact
  • tests/test_cross_repo_member_calls.py — impact
  • tests/test_csharp_call_site_generic_args.py — impact
  • tests/test_csharp_enum_members.py — impact
  • tests/test_csharp_field_generic_args.py — impact
  • tests/test_csharp_generic_callsites.py — impact
  • tests/test_csharp_interface_dispatch.py — impact
  • tests/test_csharp_member_calls.py — impact
  • tests/test_csharp_member_nodes.py — impact
  • tests/test_csharp_object_creation.py — impact
  • tests/test_csharp_partial_classes.py — impact
  • tests/test_csharp_type_resolution.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_detect.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_duplicate_annotation_edges.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cache_location.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_file_node_id_spec.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_go_builtin_call_targets.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_import_self_loops.py — impact
  • tests/test_imported_export_forwarding.py — impact
  • tests/test_incremental.py — impact
  • tests/test_indirect_call_arrow_single_param_shadow.py — impact
  • tests/test_indirect_call_catch_binding_shadow.py — impact
  • tests/test_indirect_call_external_import_shadow.py — impact
  • tests/test_indirect_call_for_of_binding_shadow.py — impact
  • tests/test_indirect_call_function_expression_shadow.py — impact
  • tests/test_indirect_call_nested_closure_shadow.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_inferred_confidence_rubric.py — impact
  • tests/test_inherited_field_receivers.py — impact
  • tests/test_java_member_calls.py — impact
  • tests/test_java_type_resolution.py — impact
  • tests/test_js_callback_calls.py — impact
  • … and 54 more

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

Could not verify: Could not verify \_import\_python.

The verifier did not have enough to check \_import\_python, 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 AttributeError — names the real obstacle, not a sampling gap)

· 36 more finding(s) on lines outside this diff (see the check run).

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.

1 participant