Repository navigation
Conversation
Node ids are case-folded, so the sourceless stub that a `flask.Flask` annotation leaves for the type `Flask` gets the id `flask`, which is also the dotted-module alias of `src/flask/__init__.py`. The Graphify-Labs#2072 src-layout pass refuses any alias that already exists as a node id, so `import flask` in Flask's tests never reached the real package. Keep the refusal for source-backed nodes. Ignore a colliding id only when every node holding it is sourceless, the alias names exactly one importable scanned file that the import spells exactly (NFKC), the edge has no target_file, no other node shares the destination id, the name is not a stdlib or built-in module (a fixed 333-name list), and the on-disk checks show nothing else is found first, both inside the candidate's own tree and in the scan root or the importer's ancestors. Unlistable directories refuse. On Flask, tests/ import edges to real source files go from 28 to 64 and SCIP import recall from 74.37% to 96.48%. FastAPI, requests, httpx and rich graphs are identical to v8. Refs Graphify-Labs#2072, Graphify-Labs#1475, Graphify-Labs#3793 Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for the pull request, @krishhgg. 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 1 advisory finding(s) below merit a look before merge.
Formal verification. PR-changed functions: 0/2 verified (0 proven, 0 may-equivalent, 0 distinguished) · 2 not verified (2 vacuous).
Not verified on this run: \_repoint\_python\_package\_imports (vacuous: never exercised), \_suppress\_ambiguous\_python\_imports (vacuous: never exercised).
Graphify review — findings
Lets _repoint_python_package_imports repoint a src/-layout import edge even when a sourceless, case-folded stub holds the alias id (e.g. the Flask type stub left by a flask.Flask annotation), carrying the exact dotted spelling through on kept edges as _python_import_spelling. It does this only when the import spells the module exactly under NFKC, the target is a sole-holder .py/.pyi file, and the top-level name isn't a stdlib or built-in module. On disk, the file must also be what Python's finder would load first from its own root (_wins_own_tree) with nothing earlier on the path shadowing it; any unreadable directory or failed guard leaves the edge dangling and the stub untouched, as before.
Worth a look
- Scan root is always probed even when it is the candidate's own sys_root, so a stub alias directly under root shadows itself —
graphify/extract.py· 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 — 2484 functions depend on the 343 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 783 callers, 50 callees - new:
_rebuild_code()— 149 callers, 56 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
main()— 102 callers, 3 callees - new:
dispatch_command()— 2 callers, 128 callees - new:
_get_extractor()— 27 callers, 6 callees - new:
collect_files()— 23 callers, 7 callees - …and 39 more — each is listed as a finding
Verification — 2484 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: 2294 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
147 of 334 test file(s) selected (44%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_blade_extractor.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_cache.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_method_declarations.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_tuple_type_refs.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_elixir_unqualified_call_scope.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— impact- … and 97 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.
Docs that may be stale (advisory)
CHANGELOG.md§ 0.9.19 (2026-07-18) (lines 729-739): references changed symbolsdeny
Formal verification
Could not verify: Could not verify \_repoint\_python\_package\_imports.
The verifier did not have enough to check \_repoint\_python\_package\_imports, 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: not verifiable: all 264 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)
Could not verify: Could not verify \_suppress\_ambiguous\_python\_imports.
The verifier did not have enough to check \_suppress\_ambiguous\_python\_imports, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 2 grounded finding(s) anchored inline below; 45 more finding(s) on lines outside this diff (see the check run).
| }) | ||
|
|
||
|
|
||
| def _repoint_python_package_imports(paths, all_nodes, all_edges, root) -> None: |
There was a problem hiding this comment.
_repoint_python_package_imports()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| e["target"] = stub_alias_map[tgt][0] | ||
|
|
||
|
|
||
| def _repoint_python_sibling_imports(paths, all_nodes, all_edges, root) -> None: |
There was a problem hiding this comment.
_repoint_python_sibling_imports()
high coupling complexity (Ca·Ce = 12).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
On the advisory about the scan root shadowing a candidate under it: that case doesn't reach the new check. Checked with a flat layout: The two coupling notes are on |
This branch has not been deployed
1 participant
What does this PR do?
In a src-layout repo, a Python package import could not reach the package when a type with the same case-folded name was referenced from another file.
In Flask (d73fa1c), the tests do
import flaskand annotateapp: flask.Flask. The annotation leaves a sourceless stub for the typeFlask. Node ids are case-folded, so the stub's id isflask, the same id as the dotted-module alias forsrc/flask/__init__.py._repoint_python_package_imports(#2072) refuses any alias that already exists as a node id, so it never repointedimport flaskorfrom flask import .... No file undertests/had an import edge tosrc/flask/__init__.py, andaffectedon the package listed no tests.The refusal protects a real case: when a scan-root
pkg.pyexists, the resolver bindsimport pkgto it, and the nestedsrc/pkgalias must not take that edge. This PR keeps the refusal for any id held by a node with asource_file. It ignores the collision only when all of these hold:.pyor.pyisuffix, every path component an NFKC-stable identifier (sosrc/shop.v2/never spellsshop.v2).flask, notFlask).target_file, so the resolver's own pick is never replaced.source_fileis the alias file. If__init__.hor__init__.pyishares the id, the edge stays where v8 put it._PYTHON_STDLIB_MODULE_NAMES(333 names), the union ofsys.stdlib_module_names | sys.builtin_module_nameson CPython 3.10, 3.12, 3.13 and 3.14, not from the running interpreter. It is a deliberate superset..graphifyignorestill count. A package directory on the way must have no same-named module beside it, and the file must come first in finder order (extension modules,.py,.pyc,.pyi). Extension modules are matched by a fixed pattern (<name>.so,<name>.pyd,<name>.<tag>.so,<name>.<tag>.pyd), not by the host'sEXTENSION_SUFFIXES.<top>directory and no<top>module file of any of those kinds in the scan root or in the importer's non-package ancestors, scanned or not. A rootshop.pytherefore keepsimport shop.coreoffsrc/shop/core.py.To check the spelling,
_suppress_ambiguous_python_importsleaves the raw module name it already reads on kept import edges as_python_import_spelling._repoint_python_package_importspops it from every edge, so it never reaches graph.json.The stub and its
referencesedges are not touched, and member-call resolution (flask.redirect(...)) is unchanged.One side effect: Flask's tests had 16
callsedges to an externalflasknode (flask.redirect(),flask.Flask(), ...), created by the #3793 external-module path. Becauseimport flasknow reaches a real file, that path no longer applies and those 16 edges are gone. They pointed at a stub that claimed the in-repo package was external. Resolving those calls through the package is a separate change.Type of change
Verification & Invariants
Invariant: a sourceless stub no longer keeps an exactly spelled import away from the one scanned module it names. The exception applies only when every condition above holds; otherwise the edge keeps its v8 target. It never overrides a real node, a resolver-stamped
target_file, an ambiguous module name, a shared destination id, a file that cannot spell the import, a file that loses on disk inside its own tree, a shadowing entry in the scan root or the importer's ancestors, or a directory that cannot be listed.The checks do not model the whole import system. They do not consult site-packages, PYTHONPATH,
.pthfiles, custom importers, zip imports, or the relative order of the candidate's root and the importer's roots.Determinism: the result depends only on the corpus on disk. Two tests run one corpus under different host values and get identical graphs: one patches
importlib.machinery.EXTENSION_SUFFIXESto the 3.13 and 3.10 lists, the other patchessys.stdlib_module_namesandsys.builtin_module_names.Persisted state: no cache format change.
_python_import_spellingis set after the per-file cache is written and popped before output; none of the rebuilt corpus graphs contains it.Limitations:
shop.pylosing tosrc/shop,src/shop.v2/matchingimport shop.v2, andsrc/shop/beating a scan-rootshop.pyc. This PR leaves that path unchanged, and the branch matches v8 for each layout. The same guards could be applied there in a follow-up.__init__.pyand__init__.pyidoes not get the exception, because two nodes hold its id._rebuild_code(changed_paths=...)) builds the alias map from changed files only, so after editing just a test fileimport shopstays unresolved there. v8 diverges from a full rebuild the same way.graphify updatematches a clean build.from pkg import submoduleare still not file edges. These are the remaining 7 Flask SCIP import misses.How was this tested?
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (Not applicable: no skill fragments changed.)