Skip to content

Forward cache_root through the incremental extract detection path - #3850

Closed
ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-3847-incremental-extract-cache-root-leak
Closed

ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-3847-incremental-extract-cache-root-leak

Conversation

@ayushcodes10

Copy link
Copy Markdown
Contributor

Fixes #3847.

Summary

extract --out <dir> stays clean on a fresh scan against a destination outside the scan root — that branch already passes cache_root=out_root to detect(). It leaks graphify-out/cache/stat-index.json into the scan root on the very next incremental run against the same destination, once a manifest already exists there.

detect_incremental() had no cache_root parameter at all, so cli.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 --out directory.

Fix

detect_incremental() now accepts cache_root: Path | None = None and forwards it into its own internal call to detect(). cli.py's incremental branch now passes cache_root=out_root, matching what the fresh-scan branch a few lines below it already does.

Testing

  • Added test_detect_incremental_respects_cache_root to tests/test_detect.py, following the same pattern as the existing test_detect_office_conversion_respects_cache_root (detect() writes converted sidecars into the scanned tree; cache_root does not redirect them #2787) test for detect() itself: asserts no graphify-out/ is created in the scan root and the stat index file lands under cache_root instead.
  • Confirmed against pre-fix code (temporarily restoring the prior versions of detect.py/cli.py) that the call raises TypeError there, since the parameter didn't exist yet.
  • Full project test suite passes (only pre-existing, unrelated failures remain: four extractor test files needing optional tree-sitter-language-pack grammars not installed in this environment, and one pre-existing environment-specific test artifact unrelated to this change).
  • python3 -m tools.skillgen --check passes.

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.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — impact, full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — impact, full-run-safety
  • tests/test_astro_import_ids.py — full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — impact, full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — impact, full-run-safety
  • tests/test_cargo_missing_manifest.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — full-run-safety
  • tests/test_charmap_encoding.py — impact, full-run-safety
  • tests/test_chunking.py — impact, full-run-safety
  • tests/test_cjs_module_extension.py — impact, full-run-safety
  • tests/test_claude_cli_backend.py — impact, full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_cobol_extractor.py — full-run-safety
  • tests/test_codebuddy.py — impact, full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — impact, full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — full-run-safety
  • tests/test_cross_language_call_resolution.py — full-run-safety
  • tests/test_cross_repo_external_call_guards.py — full-run-safety
  • tests/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).

@safishamsi

Copy link
Copy Markdown
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extract --out leaks graphify-out/cache/stat-index.json into the scan root on an incremental run (detect_incremental has no cache_root param)

2 participants