mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-24 00:01:35 +00:00
With `graphify extract --out <dir>`, 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 `<corpus>/graphify-out/` while the reader consulted `<out>/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
This commit is contained in:
shazeb
+38
-5
@@ -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
|
||||
|
||||
+12
-5
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)"
|
||||
)
|
||||
Reference in New Issue
Block a user