fix(python): resolve nested scan-root imports via namespace projection (#3843) - #3867
nikhilsaxena04 wants to merge 1 commit into
Conversation
|
Thanks for the pull request, @nikhilsaxena04. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
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 2 advisory finding(s) below merit a look before merge.
Graphify review — findings
Resolves absolute Python imports that reference the scan root's own modules when the scan root is nested inside a package hierarchy (#3843): _infer_scan_root_namespace walks upward from root collecting ancestor directory names while each has an __init__.py, and both _resolve_python_module_path and _resolve_python_namespace_dir now strip that inferred dotted prefix and re-probe relative to root when the direct probe misses. Namespace inference is memoized in _SCAN_ROOT_NAMESPACE_CACHE, keyed by resolved root path and cleared per run in extract. Third-party imports and partial-prefix matches are left unresolved, and a repo-root or non-package scan infers an empty namespace so its behaviour is unchanged.
Worth a look
- Exact scan-root namespace imports are not projected —
graphify/extractors/resolution.py:2404· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unsynchronized cache check-then-get can raise during concurrent cache clear —
graphify/extractors/resolution.py:2350· 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 — 2429 functions depend on the 438 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 712 callers, 48 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 52 more — each is listed as a finding
Verification — 2429 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: 2254 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
132 of 302 test file(s) selected (44%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_elixir_import_resolution.py— impacttests/test_erlang_extractor.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_import_repoint.py— impacttests/test_go_interface_methods.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impact- … and 82 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.
· 60 more finding(s) on lines outside this diff (see the check run).
Graphify-Labs#3843) When the scan root is a sub-directory of a monorepo package hierarchy (e.g., root = Company/Apps/Jobs/Team/), absolute imports using the full dotted namespace (from Company.Apps.Jobs.Team.lib import delivery) were unresolved because the scan root probed root/Company/Apps/Jobs/Team/lib which doesn't exist inside root. _infer_scan_root_namespace walks above the scan root collecting ancestor directory names that contain __init__.py, establishing the root's namespace prefix. When an absolute import's module name starts with that prefix, the prefix is stripped and the remainder is re-probed relative to root. The existing root/rel fast path, ancestor walk (Graphify-Labs#2072), ambiguous import guard (Graphify-Labs#3729), and phantom-edge retraction (Graphify-Labs#3784) are all untouched. Third-party imports cannot false-match because the prefix check requires a strict dotted prefix plus a remaining local component. Co-Authored-By: Google Antigravity <noreply@google.com>
1443af3 to
640e39e
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Resolves absolute Python imports when the scan root sits inside a package hierarchy: _infer_scan_root_namespace walks upward from the root collecting ancestor names while each has an __init__.py, and _resolve_python_module_path/_resolve_python_namespace_dir strip that dotted prefix and re-probe relative to the root, so from Company.Apps.Jobs.Team.lib import delivery resolves when only Team/ is scanned. Third-party imports and partial prefix matches (Company.AppService.foo) are left unresolved, and roots at or above the package boundary infer an empty namespace and behave as before. Inferred namespaces are cached per resolved root path in _SCAN_ROOT_NAMESPACE_CACHE, which extract clears each run.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2429 functions depend on the 438 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 712 callers, 48 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 52 more — each is listed as a finding
Verification — 2429 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: 2254 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
132 of 302 test file(s) selected (44%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_elixir_import_resolution.py— impacttests/test_erlang_extractor.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_import_repoint.py— impacttests/test_go_interface_methods.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impact- … and 82 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.
· 60 more finding(s) on lines outside this diff (see the check run).
|
Hi @safishamsi Hope you're having a great day! Just a heads-up that I force-pushed a quick update to address both of the advisory findings caught by the automated review agent:
The bot should report a completely clean bill of health now! Let me know if you have any feedback on the implementation! |
Security: close the Fortran cpp #include arbitrary-file-read (GHSA-pcc4-rvhr-2pr8), the last Aider/Devin monolith --watch shell sink (#3852), and terraform name/value secret redaction (#3870). Plus Windows watch rebuild locking (#3883), C# tuple element-name refs (#3877), JSX component-usage calls (#3855), and nested scan-root Python import projection (#3867). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Shipped in v0.9.70 (now on PyPI) via an authorship-preserving cherry-pick, so your commit keeps contributor-graph credit. Thanks @nikhilsaxena04! Nested scan-root absolute imports resolve via namespace projection. Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.70 |
2 participants
What does this PR do?
Resolves #3843 (Monorepo Scan-Root Resolution)
This PR resolves absolute imports originating from within a namespace package when that package itself sits at the top of the scan root. Previously, absolute imports like
from Company.Apps.Jobs.core import XYZfailed to resolve ifCompany.Apps.Jobs.Teamwas the namespace that the scanner was rooted in, resulting in dropped edges in cross-repo setups.It implements a bottom-up
__init__.pyclimb through_infer_scan_root_namespace()to logically infer the package prefix of the scan root itself. Absolute imports containing this prefix have it gracefully stripped during fallback resolution, correctly wiring the graph structure regardless of namespace depth.Type of change
Verification & Invariants
Invariants Protected:
We ensure we do not infinitely climb into the generic filesystem (
/); the climb cleanly bounds itself to explicit__init__.pypackage chains.Cached inferences are strictly tied to the fully-resolved
rootand are cleanly reset during standardextract()passes to avoid stale state if project structure morphs mid-session.Read the CONTRIBUTING.md guide.
Reproduced the issue and identified the invariant.
Made the smallest fix necessary.
Added a regression test (if bug fix) or isolated boundary test.
Kept the PR description synchronized with the final implementation.
Documented any limitations / unsupported cases explicitly.
How was this tested?
Added 6 specific regression tests that construct complex virtual namespaces and assert perfect edge tracking/fallback mapping.