fix: widen incremental cross-file symbol resolution to unchanged neighbors (#2230) - #3589
Aman-goel-04 wants to merge 1 commit into
Conversation
![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. No changes could be formally verified in this run.
Graphify review — findings
Threads the caller-supplied unchanged-corpus resolution_context_nodes into _augment_symbol_resolution_edges and _resolve_cross_file_imports so cross-file symbol resolution can bind to symbols defined in files that weren't re-extracted this run. Fixes #2230, where an incremental single-file re-extract dropped a changed file's INFERRED calls/imports/uses edges pointing at an unchanged neighbor, since the neighbor's node was absent from the batch's index even though it still lived in the merged graph; context nodes now widen both the symbol-node lookup in _apply_symbol_resolution_facts and Pass 1's stem index (anchoring root-relative persisted paths to absolute so they key the same as batch nodes), with batch entries winning on collision and context nodes never appended to the owned node set. Adds incremental regeneration tests covering both the --no-cluster merge path and the clustered incremental-scan path.
Worth a look
- Python import resolver indexes non-Python context nodes —
graphify/extractors/resolution.py:2481· 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 — 2277 functions depend on the 445 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 639 callers, 45 callees - new:
_rebuild_code()— 123 callers, 53 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
extract_objc()— 27 callers, 9 callees - …and 48 more — each is listed as a finding
Verification — 2277 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: 2102 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
120 of 284 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_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_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impact, changed-testtests/test_incremental.py— impact, changed-testtests/test_incremental_cross_file_symbol_resolution.py— impact, changed-testtests/test_indirect_call_arrow_single_param_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— impacttests/test_inferred_confidence_rubric.py— impacttests/test_inherited_field_receivers.py— impacttests/test_issue_3405_python_resolution.py— impact- … and 70 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.
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, 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 `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
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
Could not verify: Could not verify \_augment\_symbol\_resolution\_edges.
The verifier did not have enough to check \_augment\_symbol\_resolution\_edges, 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
Could not verify: Could not verify \_resolve\_cross\_file\_imports.
The verifier did not have enough to check \_resolve\_cross\_file\_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: parameter `root` is annotated `Path | None` — outside the synthesizable primitive/collection set
· 56 more finding(s) on lines outside this diff (see the check run).
1 participant
Summary
Fixes #2230: on an incremental rebuild that re-extracts only a changed file, its INFERRED cross-file calls/imports/uses edges to a symbol defined in an unchanged neighbor (e.g. from .exc import BadData; raise BadData() where exc.py isn't re-extracted) were silently dropped. The merge correctly evicts the graph's old copies of those edges since the changed file's fresh result replaced them, but nothing regenerated them, because two symbol-resolution passes in extract() only indexed nodes from the current batch:
extract() already accepts resolution_context_nodes, a read-only, caller-supplied snapshot of the unchanged corpus that both cli.py and watch.py build from the persisted graph on every incremental run. It already widens the direct-call/indirect-call resolution indexes (#2406/#2437/#2438) but was never threaded into these two import-based passes, which is the actual root cause here.
Fix
Thread resolution_context_nodes (and root, for path-form normalization) into both passes so they fall back to the unchanged-corpus snapshot when a symbol isn't in the current batch:
Nothing from context_nodes is parsed, mutated, or appended to the output graph. Ownership of those symbols stays with the unchanged files that already emit them, matching the read-only contract resolution_context_nodes already documents.
Tests
All three new/updated tests confirmed to fail on v8 before this change and pass after it. Full suite: 5612 passed, 97 skipped (4 deselected, pre-existing failures unrelated to this change, missing openai package in the test env).