Forward cache_root through the incremental extract detection path - #3850
ayushcodes10 wants to merge 3 commits into
Conversation
extract with an out destination outside the scan root stayed clean on a fresh scan, since that branch already passes cache_root to detect, but leaked the word count stat index cache file into the scan root on the very next incremental run against the same destination. detect_incremental had no cache_root parameter at all, so there was nowhere for the caller to pass it even if it wanted to, and the word count cache fell back to anchoring at the scan root instead of the requested out directory. detect_incremental now accepts cache_root and forwards it into its own internal call to detect, and the incremental branch in cli.py passes cache_root equal to out_root, matching what the fresh scan branch a few lines below it already does. Fixes issue 3847.
Covers issue 3847 directly: an incremental detect call given a cache_root outside the scan root must not create anything under the scan root, and the word count stat index cache file must land under the requested cache_root instead. Confirmed against pre fix code by restoring the prior versions of detect.py and cli.py directly, the call raises a type error there since the parameter did not exist yet.
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
Adds a cache_root parameter to detect_incremental() and forwards it to the underlying detect() call, so an incremental extract --out <dir> run anchors its word-count stat index at the requested output directory. Without it, the second run against an existing manifest fell back to the scan root and leaked graphify-out/cache/stat-index.json into the corpus. dispatch_command now passes out_root through.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2945 functions depend on the 799 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 710 callers, 47 callees - new:
_rebuild_code()— 144 callers, 55 callees - new:
detect()— 112 callers, 15 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
save_manifest()— 41 callers, 11 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 50 more — each is listed as a finding
Verification — 2945 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: 1918 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
301 of 301 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— impact, full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— impact, full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— impact, full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— impact, full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_external_call_guards.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safety- … and 251 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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.
· 58 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.69 (now on PyPI) via an authorship-preserving cherry-pick, so your commit keeps contributor-graph credit. Thanks @ayushcodes10! Incremental detection now forwards cache_root, so the stat-index no longer leaks into the corpus on an out-of-root --out. Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.69 |
Fixes #3847.
Summary
extract --out <dir>stays clean on a fresh scan against a destination outside the scan root — that branch already passescache_root=out_roottodetect(). It leaksgraphify-out/cache/stat-index.jsoninto the scan root on the very next incremental run against the same destination, once a manifest already exists there.detect_incremental()had nocache_rootparameter at all, socli.py's incremental branch had no way to pass it even if it wanted to, and the word-count cache (cached_word_count->_ensure_stat_index) fell back to anchoring at the scan root instead of the requested--outdirectory.Fix
detect_incremental()now acceptscache_root: Path | None = Noneand forwards it into its own internal call todetect().cli.py's incremental branch now passescache_root=out_root, matching what the fresh-scan branch a few lines below it already does.Testing
test_detect_incremental_respects_cache_roottotests/test_detect.py, following the same pattern as the existingtest_detect_office_conversion_respects_cache_root(detect() writes converted sidecars into the scanned tree; cache_root does not redirect them #2787) test fordetect()itself: asserts nographify-out/is created in the scan root and the stat index file lands undercache_rootinstead.detect.py/cli.py) that the call raisesTypeErrorthere, since the parameter didn't exist yet.tree-sitter-language-packgrammars not installed in this environment, and one pre-existing environment-specific test artifact unrelated to this change).python3 -m tools.skillgen --checkpasses.