Skip to content

fix: prevent phantom cycles from package-module name collisions - #3784

Closed
hopstreax wants to merge 1 commit into
Graphify-Labs:v8from
hopstreax:fix/3777-module-package-conflation
Closed

hopstreax wants to merge 1 commit into
Graphify-Labs:v8from
hopstreax:fix/3777-module-package-conflation

Conversation

@hopstreax

Copy link
Copy Markdown
Contributor

Summary

Fixes #3777 by retracting provisional Python imports_from edges when package-form imports are successfully resolved to submodules.

Previously, an import such as:

from nettacker import logger

could produce both:

app.py → nettacker.py
app.py → nettacker/logger.py

when a nettacker.py module and nettacker/ package coexisted. The incorrect provisional edge could create phantom import cycles.

What changed

  • Retract the provisional context="import" edge when the same import site resolves to a submodule on disk.
  • Match provisional edges using canonical source paths and source locations.
  • Preserve the existing submodule_import resolution behavior and edge metadata.
  • Handle multi-imports, semicolon-separated imports, relative imports, and namespace packages.
  • Leave standalone module imports unchanged.

Tests

Added regression coverage for:

Verified with:

pytest tests/test_python_import_resolution.py
9 passed

Also verified git diff --check passes cleanly.

@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 1 advisory finding(s) below merit a look before merge.

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


Graphify review — findings

Fixes phantom import cycles (#3777) caused when a package name collides with a same-named sibling module file: from nettacker import logger no longer emits a provisional imports_from edge pointing at nettacker.py. During symbol-resolution fact application, provisional package-form imports_from edges are indexed by source location and retracted when a resolved submodule import matches the same line, matching on candidate package/module target IDs (or the sole edge at that location as a fallback). Adds regression tests covering the collision case, nested package/module collisions, namespace-package submodule imports, and standalone module imports left untouched.

Worth a look

  • Fallback retraction of sole provisional edge over-retracts legitimate package imports — graphify/extractors/resolution.py:1490 · 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 — 2088 functions depend on the 165 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 690 callers, 45 callees
  • new: _rebuild_code() — 144 callers, 55 callees
  • new: _extract_generic() — 18 callers, 29 callees
  • new: extract_js() — 87 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: _resolve_js_module_path() — 34 callers, 9 callees
  • new: main() — 98 callers, 3 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • …and 40 more — each is listed as a finding

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

Test selection

Test selection

36 of 298 test file(s) selected (12%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_build.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_extract.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_indirect_call_block_scoped_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_js_exported_scalar_bindings.py — impact
  • tests/test_languages.py — impact
  • tests/test_multilang.py — impact
  • tests/test_package_json_subpath_imports.py — impact
  • tests/test_pascal.py — impact
  • tests/test_pascal_resolution.py — impact
  • tests/test_phantom_external_import.py — impact
  • tests/test_python_import_resolution.py — impact, changed-test
  • tests/test_python_parse_memoization.py — impact
  • tests/test_python_underscore_resolution.py — impact
  • tests/test_rationale.py — impact
  • tests/test_resolve_memoization.py — impact
  • tests/test_ruby_resolution.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_source_key_memoization.py — impact
  • tests/test_src_layout_import_resolution.py — impact
  • tests/test_swift_computed_properties.py — impact
  • tests/test_swift_protocol_requirements.py — impact
  • tests/test_trailing_newline_not_a_syntax_error.py — impact
  • tests/test_ts_new_expression_calls.py — impact
  • tests/test_typescript_module_extensions.py — impact
  • tests/test_unmapped_at_alias_resolution.py — impact
  • tests/test_vue_extraction.py — impact
  • tests/test_walk_python_tree_iterative.py — impact

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 \_apply\_symbol\_resolution\_facts.

The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, 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: no capturable inputs from the test suite; property tier: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

· 2 grounded finding(s) anchored inline below; 46 more finding(s) on lines outside this diff (see the check run).

assert ("process_many()", "Payload", "generic_arg") in pairs


def test_issue_3777_package_module_collision_phantom_cycle_absent(tmp_path: Path):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Health regression — test_issue_3777_package_module_collision_phantom_cycle_absent()

fans out to 6 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

assert find_import_cycles(graph) == []


def test_issue_3777_nested_module_package_collision_resolves_to_submodule(tmp_path: Path):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Health regression — test_issue_3777_nested_module_package_collision_resolves_to_submodule()

fans out to 6 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

safishamsi added a commit that referenced this pull request Sep 23, 2026
…ge imports

#3729 (absolute package import resolution) and #3784 (retract phantom
package/module-collision import edges) landed in the same cut and interacted: a
legitimate `from pkg import sub` (pure package, no colliding pkg.py) now resolves
its provisional edge to the real `pkg/__init__.py` node, but #3784 retracted it —
via the __init__.py candidate id and the len==1 blind fallback — even though there
is no name collision (no phantom cycle to fix). This dropped the correct
package-dependency edge (caught by test_absolute_package_import_targets_package_init).

Tighten #3784 to retract ONLY genuine module-collision phantoms: drop the
__init__.py candidate id forms (keep the module-file `pkg`/`pkg.py` collision ids
that the #3777 fix actually targets) and remove the len==1 blind fallback (a real
phantom always matches by its module-file id, so no fallback is needed). Both the
#3729 package-import tests and all 26 #3784 phantom-cycle tests now pass together.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.67 (on PyPI). Cherry-picked with authorship preserved. Thanks @hopstreax! On merge I tightened the retraction to genuine module-collision phantoms only, so it composes with #3729's new package-import edges (dropped the init.py candidate ids + the blind len==1 fallback).

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.

Top-level module X.py and package X/ are conflated, producing phantom import cycles

2 participants