Repository navigation
Conversation
|
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/4 verified (0 proven, 0 may-equivalent, 0 distinguished) · 4 not verified (2 vacuous, 1 unsupported, 1 witness not reproduced).
Not verified on this run: dispatch\_command (vacuous: never exercised), extract (vacuous: never exercised), extract\_rust (witness not reproduced), \_rebuild\_code (unsupported).
Graphify review — findings
Adds Rust use resolution: extract_rust now records each module scope's declarations, use bindings, globs, extern crates and #[path] attributes. After the shared call pass, resolve_rust_use_targets replaces the provisional per-declaration import edges with one edge per binding, pointing at the module file or item it names; if resolution fails, it logs a warning and skips. Incremental runs also pull affected Rust files back in via refresh_rust_paths, and _AST_CACHE_SCHEMA moves to 6, so existing AST caches re-extract.
Worth a look
- Valid raw-string #[path] attributes are treated as unreadable —
graphify/extractors/rust.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
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3123 functions depend on the 603 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 781 callers, 51 callees - new:
_rebuild_code()— 156 callers, 57 callees - new:
detect()— 121 callers, 16 callees - new:
save_semantic_cache()— 65 callers, 9 callees - new:
load_cached()— 56 callers, 7 callees - new:
extract_js()— 87 callers, 4 callees - new:
file_hash()— 54 callers, 6 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 60 more — each is listed as a finding
Verification — 3123 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: 3049 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
192 of 335 test file(s) selected (57%) via static blast radius.
tests/test_affected_cli.py— impacttests/test_agents_platform.py— impacttests/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_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_codebuddy.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_dedup_shrink_refuses_force_write.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_devin.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_explain_cli.py— impacttests/test_export_direction.py— impacttests/test_external_stub_endpoints.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_cli.py— impacttests/test_extract_php_closures.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impact- … and 142 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)
ARCHITECTURE.md§ Calling extract() from your own code (lines 36-49): references changed symbolsresolveCHANGELOG.md§ 0.9.77 (2026-10-05) (lines 5-24): references changed symbols__init__CHANGELOG.md§ 0.9.74 (2026-10-02) (lines 65-80): references changed symbolsnoteCHANGELOG.md§ 0.9.73 (2026-09-30) (lines 81-96): references changed symbolspackageCHANGELOG.md§ 0.9.65 (2026-09-20) (lines 182-194): references changed symbolsresolveCHANGELOG.md§ 0.9.61 (2026-09-12) (lines 234-238): references changed symbolsresolveCHANGELOG.md§ 0.9.60 (2026-09-12) (lines 239-248): references changed symbolspackageCHANGELOG.md§ 0.9.58 (2026-09-10) (lines 257-274): references changed symbols__init__,package,resolveCHANGELOG.md§ 0.9.56 (2026-09-07) (lines 283-295): references changed symbolspackage,scopeCHANGELOG.md§ 0.9.45 (2026-08-16) (lines 447-453): references changed symbolspackage
…and 10 more.
Formal verification
Could not verify: extract\_rust: a possible difference could NOT be confirmed.
The solver proposed a distinguishing input for extract\_rust, but re-running both versions on it did NOT reproduce a difference. Treat as unconfirmed, not a behavior change.
Guarantee:
Note: Solver-proposed witness refuted on replay — not a hard failure.
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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 40 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
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: not verifiable: all 90 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 \_rebuild\_code.
The verifier did not have enough to check \_rebuild\_code, 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: nondeterministic output (both versions disagreed with themselves on the witness) — not a behaviour change; a function whose output is not a function of its inputs (random / time / uuid / os.urandom, or state carried across calls) cannot be soundly distinguished by execution, so this tier abstains for the whole function
· 3 grounded finding(s) anchored inline below; 65 more finding(s) on lines outside this diff (see the check run).
| return current | ||
|
|
||
|
|
||
| def resolve_rust_use_targets(paths, per_file: list[dict | None], nodes: list[dict], |
There was a problem hiding this comment.
resolve_rust_use_targets()
fans out to 10 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| first_defs[real] = {item_id: entry[1] for item_id, entry in found.items()} | ||
| return first_defs[real].get(nid) | ||
|
|
||
| def node_for(target: tuple) -> str | None: |
There was a problem hiding this comment.
node_for()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ] + unique | ||
|
|
||
|
|
||
| def refresh_rust_paths( |
There was a problem hiding this comment.
refresh_rust_paths()
high coupling complexity (Ca·Ce = 12).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Give each binding of a use tree the import edge that v8 produces for its equivalent plain use declaration. Preserve aliases, path comments, separator whitespace, nested prefixes and self/glob binding behavior. Keep the old group edge as call evidence until the call pass finishes; exclude per-binding edges from that pass, then remove private markers. Plain imports and all call edges retain their current-v8 behavior. AST cache schema 6 -> 7 rejects results written before expansion. Replace the earlier broad resolver in Graphify-Labs#4135 with this syntax-only fix. The resolver is preserved on local branch wip/rust-use-resolver. On current v8, all 30 new tests and the full 6642-test suite pass. Fresh ripgrep builds have 864 import edges instead of 247, with every call unchanged. Existing name-based import rewiring can still produce wrong matches, just as it does for equivalent plain declarations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
e368390 to
2669ee4
Compare
krishhgg
changed the title
Give each binding of a use tree the import edge that v8 produces for its equivalent plain use declaration. Preserve aliases, path comments, separator whitespace, nested prefixes and self/glob binding behavior. Keep the old group edge as call evidence until the call pass finishes; exclude per-binding edges from that pass, then remove private markers. Plain imports and all call edges retain their current-v8 behavior. AST cache schema 6 -> 7 rejects results written before expansion. Replace the earlier broad resolver in #4135 with this syntax-only fix. The resolver is preserved on local branch wip/rust-use-resolver. On current v8, all 30 new tests and the full 6642-test suite pass. Fresh ripgrep builds have 864 import edges instead of 247, with every call unchanged. Existing name-based import rewiring can still produce wrong matches, just as it does for equivalent plain declarations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com> (cherry picked from commit 2669ee4)
|
Landed in 0.9.86 (now on PyPI), authorship preserved. Resolved the cache-schema conflict by bumping |
2 participants
What does this PR do?
use crate::util::{parse, render};produces one import edge toutil; neither binding gets its own edge. A root group such asuse {a::b, c};produces no edge.This expands each grouped
useinto the bindings of the equivalent plain declarations. It uses v8's existing import-target rule, preserving path text, aliases, comments, spacing, glob imports and nestedselfbindings. Each edge comes from the file node at the binding's line. Plain declarations and call edges stay unchanged.The group edge temporarily feeds the existing call-evidence pass. The per-binding edges do not feed it. After call resolution, the group edge and temporary markers are removed. AST cache schema advances from 6 to 7.
This replaces the earlier broader resolver in this PR. That resolver is preserved locally for separate work; this change only expands syntax and does not read Cargo.toml or resolve module paths.
Cache coordination: #4208 also advances schema 6 to 7. If it ships first, this branch needs a fresh rebase and another schema increment.
Type of change
Verification & Invariants
Invariant: grouped imports produce exactly the import nodes and edges that current v8 produces for the equivalent plain declarations, including its textual behavior. Existing plain-use and call output stays unchanged.
Limitations:
usedeclarations remain unsupported.How was this tested?
Verified against v0.9.80,
6478eb7.Tests compare extracted and persisted grouped/plain output and check watch and CLI incremental parity across edits. Astra also compiled and checked independent comment, alias, nested-prefix and glob cases. All temporary edge markers disappear before persisted output.
Graphify-specific checklist