perf(resolution): eliminate the warm-run Python re-parse via a content-hash fact cache - #3649
abhay-codes07 wants to merge 2 commits into
Conversation
…ntent hash _collect_python_symbol_resolution_facts re-read and re-parsed every .py file on every run — including AST-cache hits — to gather import/use facts. Split it into a content-only raw-facts extraction (import statements as written, and call sites keyed by their function name — no filesystem, no baked paths) and a fresh filesystem apply (module->file resolution, caller-id derivation). The raw facts are a pure function of the file's bytes, so they cache by the same content hash the AST cache uses; a warm `graphify update` loads them and skips the walk for unchanged files, while the filesystem-dependent half always runs fresh so a file added/removed elsewhere can never be served a stale target. Warm extract of the 364-file self-corpus: 4.0s -> 3.2s. Edges are byte- identical (verified cold and warm against the pre-change output). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
…esolution The first-half facts cache (7f8c84d) removed the re-parse from the symbol-resolution facts pass, but `_resolve_cross_file_imports` — a separate, later pass — still re-read and re-parsed every .py file on a warm `graphify update`, so the parse was merely relocated, not removed. Extend the content-hash-cached raw-facts payload (schema 1 -> 2) with the two things this pass derives from the tree: `xfile_imports` (each from-import's module reference in the pass's own parse shape, plus its imported/local name pairs) and `xfile_refs` (for each identifier bound by a from-import, the chain of enclosing class/function names, outermost first, and the line). Both are pure functions of the file's bytes, so they cache by content hash exactly like the existing facts. The filesystem-dependent half stays fresh: the module->stem resolution and the caller-id mapping (chain -> first enclosing name that maps to a node in this file) run against the current node index at apply time, so a file added or removed elsewhere can never be served a stale target. The enclosing-name CHAIN is cached rather than a single resolved id precisely so that outermost-mapped attribution is decided fresh, matching the original module-scope walk in the unmapped-outer/mapped-inner case. A shared `_python_raw_facts_for` helper backs both passes, so each file is parsed at most once across them on a cold run and not at all on a warm run: warm `_parse_python_tree` calls drop from 364 to 0 on the 364-file corpus. Cold and warm edge dumps are byte-identical to the pre-change baseline (11972 nodes, 25726 edges). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
![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 5 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds a per-file derived-facts cache (load_derived_facts/save_derived_facts in a version+schema-namespaced dir keyed by the same content hash as the AST cache) and rewires the Python symbol-resolution passes to use it. _extract_python_raw_facts now derives only path-free, content-pure facts (imports, call uses, and cross-file import/ref data) that are cached and reused when a file is unchanged, while path/filesystem resolution (_apply_python_raw_imports, cross-file import resolution) always runs fresh; the schema version orphans stale entries on shape changes. _augment_symbol_resolution_edges and _resolve_cross_file_imports now take root/cache_root so they can hit the cache.
Worth a look
- Removing the recursive body leaves an empty for-loop and makes resolution.py unparsable —
graphify/extractors/resolution.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Derived-facts cache namespace permits absolute-path/path-traversal writes —
graphify/cache.py:1151· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Partial os.write publishes truncated derived-facts cache —
graphify/cache.py:1179· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- xfile_refs collected but never applied —
graphify/extractors/resolution.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
- Attribute names are cached as imported-name references —
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
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2616 functions depend on the 493 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 645 callers, 45 callees - new:
_rebuild_code()— 129 callers, 54 callees - new:
detect()— 112 callers, 15 callees - new:
save_semantic_cache()— 63 callers, 9 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
load_cached()— 53 callers, 7 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 60 more — each is listed as a finding
Verification — 2616 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: 2443 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
137 of 286 test file(s) selected (48%) 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_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_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_extract_cli.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_ignore_file_encoding.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_incremental_mtime_collision.py— impacttests/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— impact- … and 87 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 \_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 \_collect\_python\_symbol\_resolution\_facts.
The verifier did not have enough to check \_collect\_python\_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 \_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
· 1 grounded finding(s) anchored inline below; 67 more finding(s) on lines outside this diff (see the check run).
| return None | ||
|
|
||
|
|
||
| def _extract_python_raw_facts(root_node, source: bytes) -> dict: |
There was a problem hiding this comment.
_extract_python_raw_facts()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
What
On a warm
graphify update, Python symbol resolution re-reads and re-parses every.pyfile — twice — even when the file is unchanged and its AST is already a cache hit:_collect_python_symbol_resolution_factswalks each file to gather the import/use facts the resolver needs, and_resolve_cross_file_imports(a separate, later pass) parses each file again to turnfrom .module import Nameinto direct class-levelusesedges.Both walks are pure waste on a warm run: the bytes haven't changed. This PR caches the per-file facts by content hash so neither pass parses an unchanged file.
How — a content-only / filesystem-dependent split
Caching resolution facts is only safe if you cache the half that depends solely on the file's own bytes and re-run the half that depends on other files. So each file's extraction is split along exactly that line:
_extract_python_raw_facts(tree)— content-only. Thefrom … import …statements as written (relativelevel, module string, imported/local name pairs, line); the bare call sites inside each top-level function keyed by the function's own name; and, for the cross-file pass, each from-import's module reference plus the chain of enclosing class/function names (outermost first) for every identifier a from-import binds. No filesystem access, no baked paths — a pure function of the file's bytes._apply_*/ the resolver body — filesystem-dependent, always run fresh: resolving a module string to a file on disk (is_fileprobes,_resolve_python_module_path), mapping a module to the global node index, and deriving each caller's node id from the current path.Because the raw facts are content-only, they cache under the same content hash the AST cache already uses (
load/save_derived_facts, namespaced by version+schema undercache/pyfacts/). A single shared_python_raw_facts_forhelper backs both passes, so a file is parsed at most once on a cold run and not at all on a warm one.The subtle bit: the cross-file pass attributes a reference to the first enclosing symbol whose name resolves to a node in this file. That resolution depends on the current node index, so it must stay fresh — which is why the cache stores the whole enclosing-name chain and the first-mapped choice is made at apply time, rather than caching a single resolved id that could diverge in the unmapped-outer / mapped-inner case.
Measured
Warm extract of graphify's own 364-file corpus, second run with caches populated,
parallel=False, best-of-3, all three revisions measured back-to-back under the same machine load:v8v8(no caching)The machine-independent metric:
_parse_python_treecalls on a warm run drop from 364 to 0 — the parse is fully eliminated for unchanged files across both resolution passes.Correctness
Edges are byte-identical to the pre-change output, verified both cold (which populates the cache) and warm (which reads it): both produce the exact same sorted
source|relation|target|context|confidenceedge set asv8— 11972 nodes, 25726 edges. The two-phase split preserves the original two-pass order (imports for all files, then uses), and the cross-file refactor preserves the original walk's ordering and first-writer/first-line semantics, so resolution is unchanged.Tests
The Python resolution / cross-file / incremental / cache suites pass. The full suite matches a fresh
v8(0.9.63) baseline: the same 22 pre-existing failures on this Windows machine (readonly-atomic-write, fifo/socket/symlink rejection, install/skill destinations, a terraform test, a timing-flaky mtime test, two pre-existing unicode-id tests) fail identically on cleanv8— zero regressions from this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q