Skip to content

fix(ingestion): resolve module.symbol() calls through submodule bindings - #1733

Merged
RaghavChamadiya merged 1 commit into
repowise-dev:mainfrom
sloemo01:fix/python-submodule-binding
Aug 20, 2026
Merged

fix(ingestion): resolve module.symbol() calls through submodule bindings#1733
RaghavChamadiya merged 1 commit into
repowise-dev:mainfrom
sloemo01:fix/python-submodule-binding

Conversation

@sloemo01

Copy link
Copy Markdown
Contributor

Fixes #1193

Problem

from pkg import submodule bound submodule to the package __init__.py (the fan-out's first target), so a later submodule.symbol() call resolved against a file that declares nothing and the call was missed. Dead-code then reported the symbol as safe-to-delete despite a real caller.

Fix

Two-part:

  • resolve_python_import_all now points each submodule binding's source_file at the submodule file it resolves to, not the package init.
  • build_import_name_maps only back-fills a binding's source_file when it is unset, so the submodule pin survives.

Verified end-to-end: from sensors import foo; foo.check() now resolves to sensors/foo.py::check (0.88).

Test

Adds regression tests for the submodule-binding pin and the non-submodule (package re-export) case. test_python_resolver_fanout.py: 9 passed; full tests/unit/ingestion + tests/unit/dead_code: 2855 passed (1 pre-existing env failure unrelated to this change).

@repowise-bot

repowise-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

✅ Health of changed files: 7.9 → 8.3 (+0.4)

📋 At a glance
2 hotspots touched · 1 file with recent fix history.

Files & modules (2)
  • packages (1 file)
    • .../resolvers/python.py
  • tests (1 file)
    • .../ingestion/test_python_resolver_fanout.py

✅ Health gate: passed

🔎 More signals (2)

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (2 with dependents)"]
    f_packages_core_src_repowise_core_ingestion_resolvers_python_py[".../resolvers/python.py 🔥"]:::changed
    f_packages_core_src_repowise_core_ingestion_import_index_py[".../ingestion/import_index.py"]:::changed
  end
  f_packages_core_src_repowise_core_ingestion_graph_builder_py[".../graph/builder.py"]
  f_packages_core_src_repowise_core_ingestion_resolvers_python_py --> f_packages_core_src_repowise_core_ingestion_graph_builder_py
  f_packages_core_src_repowise_core_ingestion_resolvers___init___py[".../resolvers/__init__.py"]
  f_packages_core_src_repowise_core_ingestion_resolvers_python_py --> f_packages_core_src_repowise_core_ingestion_resolvers___init___py
  f_packages_core_src_repowise_core_ingestion_call_resolver_py[".../ingestion/call_resolver.py"]
  f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_call_resolver_py
  f_packages_core_src_repowise_core_ingestion_graph__resolvers_py[".../graph/_resolvers.py"]
  f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_graph__resolvers_py
  f_packages_core_src_repowise_core_ingestion_heritage_resolver_py[".../ingestion/heritage_resolver.py"]
  f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_heritage_resolver_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Loading

Solid arrows: code that imports the changed files (5 direct dependents, from the last indexed snapshot). Dashed: history/tests.

🔥 Hotspots touched (2)

  • .../ingestion/test_python_resolver_fanout.py: 1 commits/90d, 0 dependents · primary owner: Raghav Chamadiya (100%)
  • .../resolvers/python.py: 2 commits/90d, 3 dependents · primary owner: Raghav Chamadiya (100%)

👀 Suggested reviewers @RaghavChamadiya


📊 See the full report for this PR
Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-19 11:31 UTC

`from pkg import submodule` bound `submodule` to the package
`__init__.py` (the fan-out's first target), so a later
`submodule.symbol()` call resolved against a file that declares nothing
and the call was missed — dead-code then reported the symbol as
safe-to-delete despite a real caller (repowise-dev#1193).

Two-part fix:
- `resolve_python_import_all` now points each submodule binding's
  source_file at the submodule file it resolves to, not the package init.
- `build_import_name_maps` only back-fills a binding's source_file when
  it is unset, so the submodule pin survives.

Adds regression tests for the submodule-binding pin and the
non-submodule (package re-export) case.
@sloemo01
sloemo01 force-pushed the fix/python-submodule-binding branch from b363886 to 5666ef8 Compare August 19, 2026 11:31
@sloemo01

Copy link
Copy Markdown
Contributor Author

Rebased this onto the latest main — the CI failure was just a stale test file on my branch (the harvest_decisions field got dropped upstream in #1735). The actual change here is untouched. Should be green now, just needs a re-approve on the run. Thanks!

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @sloemo01. The two-part framing in the description is what made this quick to check, because the interesting part of this fix is not either edit on its own, it is that they have to happen in that order.

The diagnosis is right and it is a level below where I would have looked first. resolve_python_import_all already fanned out to the submodule file for the edge, since #666, so the graph knew pkg/submodule.py was imported. What was missing is that the binding still pointed at pkg/__init__.py, so a later submodule.symbol() call resolved against a file that declares nothing and produced no call site. The edge was right and the name was wrong, which is why this reads as a dead-code false positive rather than as a missing import.

I traced the ordering, because the if binding.source_file is None guard only holds if the pin is written first. Inside the builder it is: the file-level import pass at graph/builder.py:403 calls resolve_python_import_all, and _shared_import_maps in graph/_resolvers.py:38-44 builds the maps lazily during the symbol-level pass, memoised on self._import_name_maps. So the pin lands before the back-fill and survives it. Also worth noting for anyone reading later: changing maps.import_names and maps.module_aliases to read binding.source_file rather than resolved is not cosmetic, it is the half that makes the pin reach the consumers. Pinning the binding alone would have left both maps still holding the package init.

Recording one thing rather than asking for a change. build_import_name_maps is also called directly from call_resolver.py:238 and heritage_resolver.py:135, which are the fallback paths for when maps are not handed in. Those run inside the same build, so the order still holds today, but correctness now depends on shared mutable ParsedFile state being visited in a particular sequence, and nothing in the code says so out loud. Your comment at the guard is the closest thing to a statement of it, which is why I would keep that comment even if someone later thinks it is redundant. If this ever breaks it will break as a silent recall drop, not an error, so it is worth the sentence.

Not a blocker and not for this PR: the guard also makes build_import_name_maps non-idempotent in a new way, since a second call can no longer correct a stale source_file. That only matters if parsed files are ever reused across builds in one process, which I do not think they are.

Nothing to change. Waiting on the same workflow-run approval as #1731 and #1732.

@RaghavChamadiya RaghavChamadiya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same correction as on #1731 and #1732: the run I read was the superseded one. Your 11:31 rebase is green across the full matrix on this head. Merging.

@RaghavChamadiya
RaghavChamadiya merged commit ad654a1 into repowise-dev:main Aug 20, 2026
9 checks passed
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.

dead-code: unused_export false-positive on module.symbol(...) attribute calls (only direct 'from x import symbol' resolves)

3 participants