fix(go): repoint intra-module imports_from edges from go_pkg_ sinks to the package's files - #3748
carterko23 wants to merge 1 commit into
Conversation
…o the package's files extractors/go.py mints every import target as `go_pkg_<raw import string>` with no module lookup, so an intra-module import never reaches a file: the sink has no source_file and nothing links it onward. `affected` and any reachability walk miss every consumer that arrives through an import, while qualified calls (`b.F()`) resolve, because the call pass already filters callees by _go_import_path_for_file. This inverts that mapping once per run and repoints the intra-module `imports_from` edges onto the imported package's file nodes, the way the JS/TS resolver repoints module specifiers. Stdlib and external imports have no file in the corpus and stay sinks by design. Measured on a 744-file single-go.mod module, same commit, with and without this change: a 111-file handler package goes from 0/21 direct intra-module imports resolved to 21/21 (24/24 transitive); the 57 intra-module go_pkg_ nodes disappear; +16,616 imports_from edges; no other relation changes. Closes Graphify-Labs#3746 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
Repoints Go intra-module imports_from edges away from the synthetic go_pkg_<path> sink onto the actual file nodes of the imported package, so affected/reachability now follows Go imports instead of dead-ending at a node with no source file. Resolution reuses _go_import_path_for_file to map each .go file to its canonical import path, skips self-imports and existing edge pairs, and leaves stdlib/external imports (which have no file in the corpus) as sinks. Adds tests/test_go_import_repoint.py covering that every file of an imported package is reached, the qualified call edge survives, no self-import is created, and fmt stays a sink.
No blocking issues surfaced. 8 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2071 functions depend on the 257 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 657 callers, 45 callees - new:
_rebuild_code()— 142 callers, 54 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 30 more — each is listed as a finding
Verification — 2071 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: 1896 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
122 of 291 test file(s) selected (42%) 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_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_extract.py— impacttests/test_extract_cache_location.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— impact, changed-testtests/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— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impact- … and 72 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.
· 38 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.66 (on PyPI). Cherry-picked with authorship preserved so it shows under your GitHub contributions. Thanks @carterko23! |
…hify-Labs#1565) `graphify affected` (blast radius) was blind to a function passed BY NAME as a call argument — `executor.submit(fn)`, `Thread(target=fn)`, `map(fn, xs)`, callbacks — so those callers were silently dropped from the affected set (an under-counting blast radius reads complete while missing exactly where regressions hide). Capture them as a distinct `indirect_call` relation (INFERRED, context "argument"), kept separate from `calls` so strict call-graph queries stay precise, and add it to DEFAULT_AFFECTED_RELATIONS. Hardened over the original PR against the name-collision false edge (the Doxygen Graphify-Labs#3748 trap): emit only when the argument identifier resolves to a callable definition AND is not shadowed by a parameter or local binding in the enclosing function. So `def via(pool, handler): pool.submit(handler)` (handler is the param, not the module function) and `process(config)` where `config` is local data emit no edge, while a genuine module function passed by name still resolves. Dedup-safe against existing direct `calls` edges. Python only; dict-literal / getattr-by-string / decorator dispatch deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repoints intra-module Go
imports_fromedges from the syntheticgo_pkg_<raw>sink onto the imported package's file nodes, using the module-path resolver the call pass already relies on.Closes #3746
Before / after
Same corpus (744
.gofiles, onego.mod, noreplace/vendor/go.work), same commit,graphify extract . --code-only, with and without this change:go list -depspackages reachablego_pkg_*sink nodesgo_pkg_*sinksimports_fromedgesWhat changes
One block in
extract()right after the shared call-resolution loop, where_go_module_cache,sf_to_file_nidandexisting_pairsare already in scope:pkg_files = {_go_import_path_for_file(sf): [file node ids]}over every Go file node (one pass, reuses the cache).imports_fromedge whose target id is_make_id("go", "pkg", <known import path>), replace it with one edge per file of that package, deduped againstexisting_pairsand skipping self-imports.Design note for reviewers: this mirrors the JS/TS path (edge → file node), which for Go means one edge per file of the imported package — hence the edge count. The alternative in #3746 is to keep the
go_pkg_node and give itcontainsedges to its files (one hop more for traversals, far fewer edges). Happy to switch if you prefer that shape.Test
tests/test_go_import_repoint.py— a two-package fixture (a/a.goimportsexample.com/m/bandfmt;b/b.go+b/c.go):imports_fromedges to every file ofb, and no edge points atgo_pkg_example_com_m_bany more;Run() → F()survives and no self-import appears;fmtstill targetsgo_pkg_fmtwith no source file (external sinks preserved).Suite
uv run --frozen pytest tests/ -q: 5,733 passed, 97 skipped, 4 failed — the 4 aretests/test_ollama_retry_cap.py,ModuleNotFoundError: No module named 'openai'; they fail identically on the unpatched base under a plainuv sync --frozenand pass where--all-extrasis installed (as CI's test job does).🤖 Generated with Claude Code