From 08166306bab783634c251dc2c252afff89fcdfd8 Mon Sep 17 00:00:00 2001 From: shazeb Date: Sat, 18 Jul 2026 21:01:30 +0530 Subject: [PATCH] fix(cache): anchor semantic cache writes to cache_root so --out round-trips (#1990, #1991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With `graphify extract --out `, the semantic cache write and read sides disagreed on both location and key anchoring, breaking the cache round-trip in two ways: - Checkpoints (#1990): `_checkpoint_chunk` called `save_semantic_cache` with only `root=target`, so per-chunk recovery checkpoints were written under `/graphify-out/` while the reader consulted `/graphify-out/` — creating an unwanted graphify-out/ inside the analyzed source tree and making every interrupted run re-extract (and re-bill) completed chunks. - Final save (#1991): cli.py passed `root=out_root`, so corpus-relative `source_file` paths resolved against the --out directory, failed `p.is_file()`, and every result group was silently skipped — the cache the reader would consult was never populated at all, with no warning. Fix, following the split the AST cache already uses (#1774): - `save_semantic_cache` and `check_semantic_cache` gain a `cache_root` parameter mirroring `load_cached`/`save_cached`: `root` stays the source-key anchor (content-hash keys, source_file resolution and relativization), `cache_root` selects where cache files live. Omitting it keeps `root` for both, so existing callers are unchanged. - `extract_corpus_parallel` plumbs `cache_root` into `_checkpoint_chunk`. - cli.py extract passes `root=target, cache_root=out_root` at the cache read, the checkpoint path, and the final save, and re-anchors the prune sweep's live hashes to `target` (keys anchored to out_root would mismatch every entry and sweep the fresh cache as orphaned). - `save_semantic_cache` now warns loudly when every result group is dropped because its source_file does not resolve to a real file — the silent-0-writes failure mode #1991 asked to surface. Regression tests cover: checkpoint written under cache_root (not the corpus, no corpus graphify-out/ created), recovery read finds the checkpoint via the same root/cache_root split, the final-save call shape writes entries where the reader looks, the all-groups-dropped warning, and backward compatibility when cache_root is omitted. Fixes #1990 Fixes #1991 --- graphify/cache.py | 43 ++++- graphify/cli.py | 17 +- graphify/llm.py | 10 ++ tests/test_semantic_cache_out_root.py | 221 ++++++++++++++++++++++++++ 4 files changed, 281 insertions(+), 10 deletions(-) create mode 100644 tests/test_semantic_cache_out_root.py diff --git a/graphify/cache.py b/graphify/cache.py index 6e266c26..0a86a7e8 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -722,6 +722,7 @@ def check_semantic_cache( mode: str | None = None, prompt: "str | Path | None" = None, prompt_file: "str | Path | None" = None, + cache_root: "Path | None" = None, ) -> tuple[list[dict], list[dict], list[dict], list[str]]: """Check semantic extraction cache for a list of absolute file paths. @@ -744,6 +745,12 @@ def check_semantic_cache( vintage is unknowable and dropping them would re-bill a whole corpus — but a warning reports how many were served. Omitting ``prompt`` keeps the historical behavior for existing callers. + + ``cache_root`` decouples *where* the cache is read from the key-anchor + ``root``, mirroring :func:`load_cached` and :func:`save_semantic_cache` + (#1774 / #1990). With ``--out``, pass the corpus as ``root`` (so content-hash + keys and relative-path resolution stay anchored to the source tree) and the + output directory as ``cache_root``. Omitting it keeps ``root`` for both. """ global _legacy_semantic_hits kind = "semantic" if mode is None else f"semantic-{mode}" @@ -757,7 +764,8 @@ def check_semantic_cache( p = Path(fpath) if not p.is_absolute(): p = Path(root) / p - result = load_cached(p, root, kind=kind, prompt=prompt, prompt_file=prompt_file) + result = load_cached(p, root, kind=kind, cache_root=cache_root, + prompt=prompt, prompt_file=prompt_file) if result is not None: cached_nodes.extend(result.get("nodes", [])) cached_edges.extend(result.get("edges", [])) @@ -808,6 +816,7 @@ def save_semantic_cache( prompt: "str | Path | None" = None, prompt_file: "str | Path | None" = None, partial_source_files: Iterable[str | Path] | None = None, + cache_root: "Path | None" = None, ) -> int: """Save semantic extraction results to cache, keyed by source_file. @@ -845,6 +854,16 @@ def save_semantic_cache( replaying them (#1939). Pass the same prompt here as to :func:`check_semantic_cache`, or the write lands in a namespace the next read won't consult. + + ``cache_root`` decouples *where* the cache directory is written from the + source-key anchor ``root`` — mirroring the same split that :func:`load_cached` + and :func:`save_cached` already expose (#1774). When given, cache files land + under ``cache_root`` while ``source_file`` paths are still resolved and + relativized against ``root``. When omitted, ``root`` is used for both + purposes (unchanged behaviour for existing callers). This fixes checkpoints + and the final save going to the corpus tree instead of ``--out`` (#1990, + #1991). + Returns the number of files cached. """ from collections import defaultdict @@ -952,6 +971,7 @@ def save_semantic_cache( ] saved = 0 + skipped_not_file = 0 for fpath, result in by_file.items(): p = resolved_source_path(fpath) if p.is_file(): @@ -975,9 +995,9 @@ def save_semantic_cache( # markers ride through, so is_partial below re-detects it) rather # than a later clean slice silently replacing it and promoting the # half-file to complete. - prev = load_cached(p, root, kind=kind, prompt=prompt, - prompt_file=prompt_file, allow_legacy=False, - allow_partial=True) + prev = load_cached(p, root, kind=kind, cache_root=cache_root, + prompt=prompt, prompt_file=prompt_file, + allow_legacy=False, allow_partial=True) _prev_partial = bool(prev.get("partial")) if prev else False if prev: result = { @@ -1002,6 +1022,19 @@ def save_semantic_cache( ) if is_partial: result = {**result, "partial": True} - save_cached(p, result, root, kind=kind, prompt=prompt, prompt_file=prompt_file) + save_cached(p, result, root, kind=kind, cache_root=cache_root, + prompt=prompt, prompt_file=prompt_file) saved += 1 + else: + skipped_not_file += 1 + if skipped_not_file and skipped_not_file == len(by_file): + warnings.warn( + f"save_semantic_cache: all {skipped_not_file} source_file group(s) were " + "skipped because their paths do not resolve to real files. This usually " + "means ``root`` is anchored to the wrong directory (e.g. the --out " + "directory instead of the corpus root). Pass the corpus directory as " + "``root`` and the output directory as ``cache_root`` (#1991).", + RuntimeWarning, + stacklevel=2, + ) return saved diff --git a/graphify/cli.py b/graphify/cli.py index 41f45588..35e397e6 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2843,8 +2843,8 @@ def dispatch_command(cmd: str) -> None: uncached_paths = list(sem_paths_str) else: cached_nodes, cached_edges, cached_hyperedges, uncached_paths = ( - _check_semantic_cache(sem_paths_str, root=out_root, mode=sem_cache_mode, - prompt=sem_prompt) + _check_semantic_cache(sem_paths_str, root=target, cache_root=out_root, + mode=sem_cache_mode, prompt=sem_prompt) ) sem_cache_hits = len(semantic_files) - len(uncached_paths) sem_cache_misses = len(uncached_paths) @@ -2860,6 +2860,7 @@ def dispatch_command(cmd: str) -> None: "backend": backend, "model": model, "root": target, + "cache_root": out_root, } if deep_mode: corpus_kwargs["deep_mode"] = True @@ -2930,7 +2931,8 @@ def dispatch_command(cmd: str) -> None: fresh.get("nodes", []), fresh.get("edges", []), fresh.get("hyperedges", []), - root=out_root, + root=target, + cache_root=out_root, allowed_source_files=uncached_paths, mode=sem_cache_mode, prompt=sem_prompt, @@ -2955,6 +2957,11 @@ def dispatch_command(cmd: str) -> None: # incremental and full branches), NOT the incremental ``semantic_files`` # changed-subset, which would delete every unchanged doc's valid entry. # Best-effort: a prune failure must never break extraction. + # Hash keys are anchored to the corpus (``target``) — the same anchor + # the cache read/write above use — while the stat-index artifact + # follows the cache location (``out_root``). Anchoring these hashes to + # ``out_root`` instead would mismatch every key under ``--out`` and + # sweep the entire fresh cache as orphaned (#1990/#1991). try: from graphify.cache import file_hash as _file_hash _live_hashes: set[str] = set() @@ -2962,11 +2969,11 @@ def dispatch_command(cmd: str) -> None: for _fp in files_by_type.get(_kind, []): _abs = Path(_fp) if not _abs.is_absolute(): - _abs = Path(out_root) / _abs + _abs = Path(target) / _abs if not _abs.is_file(): continue # deleted/missing — leave out so its entry is pruned try: - _live_hashes.add(_file_hash(_abs, out_root)) + _live_hashes.add(_file_hash(_abs, target, cache_root=out_root)) except OSError: pass _prune_semantic_cache(out_root, _live_hashes) diff --git a/graphify/llm.py b/graphify/llm.py index ef094b53..48546b86 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -2044,6 +2044,7 @@ def extract_corpus_parallel( max_concurrency: int = 4, max_retry_depth: int = 3, deep_mode: bool = False, + cache_root: "Path | None" = None, ) -> dict: """Extract a corpus in chunks, merging results. @@ -2079,6 +2080,14 @@ def extract_corpus_parallel( output_tokens. Failed chunks are logged to stderr and skipped — one bad chunk does not abort the run. + ``cache_root`` (when given) is where per-chunk checkpoint cache entries are + written, decoupled from ``root`` which anchors content-hash keys and + ``source_file`` resolution — the same split the AST cache uses (#1774). + With ``--out``, cli.py passes the corpus as ``root`` and the output + directory as ``cache_root`` so checkpoints land where the recovery read + looks, instead of creating an unwanted ``graphify-out/`` inside the + analyzed source tree (#1990). + Accepts ``str`` paths as well as ``Path``; string entries are coerced up front so packing/slicing helpers can rely on ``Path`` semantics (#1386). """ @@ -2154,6 +2163,7 @@ def extract_corpus_parallel( result.get("edges", []), result.get("hyperedges", []), root=root, + cache_root=cache_root, merge_existing=True, allowed_source_files=allowed, mode="deep" if deep_mode else None, diff --git a/tests/test_semantic_cache_out_root.py b/tests/test_semantic_cache_out_root.py new file mode 100644 index 00000000..2bde8d3c --- /dev/null +++ b/tests/test_semantic_cache_out_root.py @@ -0,0 +1,221 @@ +"""Regression tests for #1990 and #1991. + +#1990 — `graphify extract --out` saves recovery checkpoints in the wrong directory. + save_semantic_cache must write to cache_root (the --out dir), not root + (the corpus dir), when they differ. + +#1991 — Final semantic-cache save resolves source_file paths against out_root + and silently writes 0 entries when corpus files do not exist there. + Fixed by passing root=target (corpus), cache_root=out_root to + save_semantic_cache so source_file resolution and cache placement are + independently anchored. +""" +import warnings +from pathlib import Path + +import pytest + +from graphify.cache import ( + check_semantic_cache, + file_hash, + load_cached, + save_semantic_cache, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _semantic_dir(root: Path, mode: str | None = None) -> Path: + kind = "semantic" if mode is None else f"semantic-{mode}" + return root / "graphify-out" / "cache" / kind + + +def _count_cache_files(base: Path) -> int: + """Count .json files under a cache dir (recursively, excluding .tmp).""" + if not base.is_dir(): + return 0 + return sum(1 for _ in base.rglob("*.json")) + + +# --------------------------------------------------------------------------- +# #1990 — checkpoint writes to cache_root, not corpus root +# --------------------------------------------------------------------------- + + +def test_save_semantic_cache_writes_to_cache_root_not_corpus(tmp_path): + """When cache_root differs from root, cache files must land under cache_root.""" + corpus = tmp_path / "corpus" + out = tmp_path / "out" + corpus.mkdir() + out.mkdir() + + doc = corpus / "report.md" + doc.write_text("# Report\nSome content here.") + + nodes = [{"id": "n1", "label": "Report", "source_file": str(doc)}] + edges: list = [] + + saved = save_semantic_cache( + nodes, edges, root=corpus, cache_root=out + ) + + assert saved == 1, "expected 1 entry written" + # Cache file must be under the out dir, not the corpus dir + assert _count_cache_files(_semantic_dir(out)) == 1 + assert _count_cache_files(_semantic_dir(corpus)) == 0 + + +def test_save_semantic_cache_no_corpus_graphify_out_created(tmp_path): + """With cache_root set, no graphify-out/ dir should be created inside corpus.""" + corpus = tmp_path / "corpus" + out = tmp_path / "out" + corpus.mkdir() + out.mkdir() + + doc = corpus / "notes.md" + doc.write_text("Notes content.") + + save_semantic_cache( + [{"id": "x", "label": "X", "source_file": str(doc)}], + [], + root=corpus, + cache_root=out, + ) + + assert not (corpus / "graphify-out").exists(), ( + "graphify-out/ must not be created inside the corpus when cache_root is set" + ) + + +# --------------------------------------------------------------------------- +# #1990 — checkpoint written with cache_root is found on recovery read +# --------------------------------------------------------------------------- + + +def test_checkpoint_with_cache_root_is_found_by_check_semantic_cache(tmp_path): + """A checkpoint saved with cache_root can be retrieved by check_semantic_cache + using the same root/cache_root split — simulating recovery after interruption.""" + corpus = tmp_path / "corpus" + out = tmp_path / "out" + corpus.mkdir() + out.mkdir() + + doc = corpus / "paper.md" + doc.write_text("Some academic content.") + + nodes = [{"id": "p1", "label": "Paper", "source_file": str(doc)}] + + # Simulate a checkpoint written mid-run (merge_existing path) + save_semantic_cache( + nodes, [], + root=corpus, + cache_root=out, + merge_existing=True, + allowed_source_files=[doc], + ) + + # Recovery read: check_semantic_cache with the same root/cache_root split + # cli.py now uses (root=target, cache_root=out_root). + cached_nodes, cached_edges, cached_hyperedges, uncached = check_semantic_cache( + [str(doc)], root=corpus, cache_root=out + ) + + assert len(uncached) == 0, f"expected cache hit, got miss for: {uncached}" + assert any(n.get("id") == "p1" for n in cached_nodes) + + +# --------------------------------------------------------------------------- +# #1991 — final save resolves source_file against corpus root, not out_root +# --------------------------------------------------------------------------- + + +def test_final_save_with_out_root_populates_cache(tmp_path): + """When root=corpus and cache_root=out, source_file resolution must use + corpus as anchor so p.is_file() succeeds and the entry is written.""" + corpus = tmp_path / "corpus" + out = tmp_path / "out" + corpus.mkdir() + out.mkdir() + + doc = corpus / "report.md" + doc.write_text("# Annual Report\nKey findings.") + + # This is the pattern that was broken: source_file is relative to corpus, + # but was previously resolved against out_root, making p.is_file() → False. + nodes = [{"id": "r1", "label": "AnnualReport", "source_file": "report.md"}] + + saved = save_semantic_cache( + nodes, [], + root=corpus, + cache_root=out, + allowed_source_files=[doc], + ) + + assert saved == 1, ( + "final save must write 1 entry when root=corpus and source_file is " + "relative to corpus — resolving against out_root caused 0 writes (#1991)" + ) + assert _count_cache_files(_semantic_dir(out)) == 1 + + +def test_final_save_with_wrong_root_emits_warning(tmp_path): + """Passing root=out_root (the old broken behaviour) silently writes 0 entries; + the fix adds a RuntimeWarning when ALL groups are dropped.""" + corpus = tmp_path / "corpus" + out = tmp_path / "out" + corpus.mkdir() + out.mkdir() + + doc = corpus / "report.md" + doc.write_text("# Report") + + # Old (broken) call: root=out, no cache_root — the corpus-relative + # source_file is resolved against out where the file doesn't exist + # → 0 entries written. + nodes = [{"id": "r1", "label": "R", "source_file": "report.md"}] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + saved = save_semantic_cache(nodes, [], root=out) + + assert saved == 0 + # The new warning must fire when every group is dropped + assert any( + "#1991" in str(warning.message) for warning in w + ), "expected RuntimeWarning mentioning #1991 when all groups are silently skipped" + + +# --------------------------------------------------------------------------- +# Backward compat — omitting cache_root keeps legacy behaviour +# --------------------------------------------------------------------------- + + +def test_save_semantic_cache_backward_compat_no_cache_root(tmp_path): + """When cache_root is omitted, cache files still land under root (unchanged).""" + root = tmp_path / "project" + root.mkdir() + + doc = root / "main.md" + doc.write_text("Main content.") + + nodes = [{"id": "m1", "label": "Main", "source_file": str(doc)}] + + saved = save_semantic_cache(nodes, [], root=root) + + assert saved == 1 + assert _count_cache_files(_semantic_dir(root)) == 1 + + +def test_extract_corpus_parallel_accepts_cache_root_kwarg(): + """extract_corpus_parallel must accept a cache_root kwarg without raising + (import + signature check — no actual LLM call).""" + import inspect + from graphify.llm import extract_corpus_parallel + + sig = inspect.signature(extract_corpus_parallel) + assert "cache_root" in sig.parameters, ( + "extract_corpus_parallel must expose cache_root so cli.py can plumb " + "out_root through to _checkpoint_chunk (#1990)" + )