From 334cff6172ab8684eb45327ec76bc8df70e0cbd3 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Sun, 26 Jul 2026 11:52:20 +0100 Subject: [PATCH] fix(cache): portable stat-index keys + normalized semantic source_file (#2199, #2197) stat-index.json keyed entries by absolute path and never pruned, so a moved/cloned corpus got 0% cache hits and the index grew unbounded. Keys are now stored root-relative and re-anchored on load (mirroring the manifest.json portability fix), and dead-file entries are pruned on flush. save_semantic_cache also normalizes source_file to root-relative before persisting so an absolute/backslash fragment can't poison later updates. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/cache.py | 134 +++++++++++++-- tests/test_stat_index_portability.py | 243 +++++++++++++++++++++++++++ 2 files changed, 366 insertions(+), 11 deletions(-) create mode 100644 tests/test_stat_index_portability.py diff --git a/graphify/cache.py b/graphify/cache.py index 0a86a7e8..7257aca7 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -187,9 +187,48 @@ def _body_content(content: bytes) -> bytes: # skip the cache reads and re-dispatch everything when needed (#1894). _stat_index: dict[str, dict] = {} _stat_index_root: Path | None = None +# Key anchor for the ON-DISK index (#2199): the first caller's key-root, i.e. +# the corpus. Distinct from _stat_index_root, which is the cache-FILE location +# (cache_root, #1774) — the two differ under --out and must not be conflated. +_stat_index_anchor: Path | None = None _stat_index_dirty: bool = False +def _stat_key_to_relative(key: str, anchor: Path) -> str: + """Return ``key`` as a forward-slash relative path from ``anchor``. + + Local duplicate of :func:`graphify.detect._to_relative_for_storage` — + detect imports cache, so cache cannot import detect without a cycle + (and pulling detect in during the atexit flush would be fragile). + Out-of-anchor and already-relative keys pass through unchanged, and + ``..``-escaping relpaths are rejected (kept absolute), mirroring the + manifest's portability rules. + """ + p = Path(key) + if not p.is_absolute(): + return key + try: + rel = os.path.relpath(p, anchor) + except (ValueError, OSError): + return key # outside anchor (e.g. Windows cross-drive) + if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): + return key # escaped anchor — keep absolute + return rel.replace(os.sep, "/") + + +def _stat_key_to_absolute(key: str, anchor: Path) -> str: + """Inverse of :func:`_stat_key_to_relative`. + + Re-anchor a stored relative key against ``anchor``. Already-absolute keys + (legacy indexes, out-of-anchor entries) pass through unchanged so an index + written by an older graphify remains readable. + """ + p = Path(key) + if p.is_absolute(): + return str(p) + return str(anchor / p) + + def _stat_index_file(root: Path) -> Path: _out = Path(_GRAPHIFY_OUT) base = _out if _out.is_absolute() else Path(root).resolve() / _out @@ -197,22 +236,36 @@ def _stat_index_file(root: Path) -> Path: def _ensure_stat_index(root: Path, cache_root: "Path | None" = None) -> None: - global _stat_index, _stat_index_root, _stat_index_dirty + global _stat_index, _stat_index_root, _stat_index_anchor, _stat_index_dirty if _stat_index_root is not None: return - # The stat index only determines the cache FILE location (entry keys are - # absolute paths), so honoring an explicit cache_root keeps detect()'s - # word-count cache under the requested --out dir instead of polluting the - # scanned corpus with a stray graphify-out/ (#1747). + # _stat_index_root determines the cache FILE location, so honoring an + # explicit cache_root keeps detect()'s word-count cache under the requested + # --out dir instead of polluting the scanned corpus with a stray + # graphify-out/ (#1747). _stat_index_anchor is the separate KEY anchor: + # in-memory keys stay absolute, but the on-disk index stores in-anchor keys + # relative so a moved/cloned corpus still hits (#2199) — same load/save + # re-anchoring the detect manifest uses. _stat_index_root = Path(cache_root if cache_root is not None else root).resolve() + _stat_index_anchor = Path(root).resolve() p = _stat_index_file(_stat_index_root) + _stat_index = {} if p.exists(): try: - _stat_index = json.loads(p.read_text(encoding="utf-8")) + raw = json.loads(p.read_text(encoding="utf-8")) + if isinstance(raw, dict): + for k, v in raw.items(): + if not isinstance(k, str): + continue + if Path(k).is_absolute(): + # Legacy/out-of-anchor key: pass through, but never + # clobber a re-anchored relative (new-format) entry + # that resolved to the same absolute path. + _stat_index.setdefault(k, v) + else: + _stat_index[_stat_key_to_absolute(k, _stat_index_anchor)] = v except (json.JSONDecodeError, OSError): _stat_index = {} - else: - _stat_index = {} atexit.register(_flush_stat_index) @@ -221,11 +274,26 @@ def _flush_stat_index() -> None: if not _stat_index_dirty or _stat_index_root is None: return p = _stat_index_file(_stat_index_root) + # Build the on-disk form (#2199): prune entries whose file is gone (the + # index otherwise grows without bound), then store in-anchor keys as + # forward-slash relative paths so the index survives a corpus move/clone. + # Out-of-anchor keys stay absolute (same rule as the detect manifest); a + # reader tells the formats apart by absoluteness, so no version marker is + # needed. In-memory keys are untouched — only the serialization changes. + on_disk: dict[str, dict] = {} + for k, v in _stat_index.items(): + try: + if not os.path.exists(k): + continue + except OSError: + continue + dk = _stat_key_to_relative(k, _stat_index_anchor) if _stat_index_anchor is not None else k + on_disk[dk] = v try: p.parent.mkdir(parents=True, exist_ok=True) fd, tmp = tempfile.mkstemp(dir=p.parent, prefix="stat-index.", suffix=".tmp") try: - os.write(fd, json.dumps(_stat_index, separators=(",", ":")).encode()) + os.write(fd, json.dumps(on_disk, separators=(",", ":")).encode()) os.close(fd) os.replace(tmp, p) except Exception: @@ -423,6 +491,29 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None: item["source_file"] = rel.replace(os.sep, "/") +def _normalize_source_file_value(src: "str | Path", root_resolved: Path) -> str: + """Return ``src`` in portable form: backslashes flipped to forward slashes, + then relativized against ``root_resolved`` when the path is in-root. + + Windows ``detect()`` emits absolute backslash paths, and a semantic + fragment carrying one verbatim used to be persisted as-is — poisoning later + ``graphify update`` runs with a machine-specific ``source_file`` (#2197). + Out-of-root absolute paths pass through (slash-normalized only), same + in/out rule and ``..``-rejection as :func:`_relativize_source_files_in`. + """ + s = str(src).replace("\\", "/") + p = Path(s) + if not p.is_absolute(): + return s + try: + rel = os.path.relpath(p, root_resolved) + except (ValueError, OSError): + return s # out-of-root (e.g. Windows cross-drive) + if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): + return s # escaped root — keep absolute + return rel.replace(os.sep, "/") + + def _absolutize_source_files_in(payload: dict, root: Path) -> None: """Inverse of :func:`_relativize_source_files_in`. @@ -869,22 +960,43 @@ def save_semantic_cache( from collections import defaultdict kind = "semantic" if mode is None else f"semantic-{mode}" + root_path = Path(root).resolve() + + def _normalized(item: dict) -> dict: + """Copy of ``item`` with a portable ``source_file`` (#2197). + + Normalizing BEFORE grouping means both the group key and the persisted + item carry the relative forward-slash form, so a fragment whose + source_file arrived absolute (Windows detect() output) can never be + cached verbatim. A shallow copy keeps the caller's dicts untouched — + downstream steps may still rely on the original absolute shape (same + reasoning as :func:`save_cached`'s on-disk deepcopy). + """ + src = item.get("source_file") + if not src: + return item + norm = _normalize_source_file_value(src, root_path) + if norm != src: + item = {**item, "source_file": norm} + return item + by_file: dict[str, dict] = defaultdict(lambda: {"nodes": [], "edges": [], "hyperedges": []}) for n in nodes: + n = _normalized(n) src = n.get("source_file", "") if src: by_file[src]["nodes"].append(n) for e in edges: + e = _normalized(e) src = e.get("source_file", "") if src: by_file[src]["edges"].append(e) for h in (hyperedges or []): + h = _normalized(h) src = h.get("source_file", "") if src: by_file[src]["hyperedges"].append(h) - root_path = Path(root).resolve() - def resolved_source_path(value: str | Path) -> Path: path = Path(value) if not path.is_absolute(): diff --git a/tests/test_stat_index_portability.py b/tests/test_stat_index_portability.py new file mode 100644 index 00000000..51ae36a6 --- /dev/null +++ b/tests/test_stat_index_portability.py @@ -0,0 +1,243 @@ +"""#2199 — stat-index.json must be portable and self-pruning. + +The on-disk stat index used to key entries by resolved ABSOLUTE path, so a +corpus reached via a different absolute path (clone, move, second mount) got +0% cache hits (100% re-extraction), and entries for deleted files were never +pruned (unbounded growth). In-memory keys stay absolute; only the on-disk +form is relativized against the key anchor — mirroring the detect manifest's +_to_relative_for_storage/_to_absolute_from_storage round-trip. + +Also covers #2197 (cache.py portion): save_semantic_cache must normalize each +item's source_file (backslashes -> forward slashes, relativize when in-root) +before persisting, so a fragment carrying an absolute path (Windows detect() +output) cannot poison the cache. +""" +from __future__ import annotations + +import hashlib +import json +import os +import shutil +from pathlib import Path + +from graphify import cache + + +def _reset_stat_index(): + """The stat-index location/anchor are chosen once per process via module + globals (#1747/#2199). Reset them so each test sees a fresh-process + decision — same pattern as tests/test_extract_cache_location.py.""" + cache._stat_index_root = None + cache._stat_index_anchor = None + cache._stat_index = {} + cache._stat_index_dirty = False + + +def _stat_index_path(root: Path) -> Path: + return root / "graphify-out" / "cache" / "stat-index.json" + + +def _read_index(root: Path) -> dict: + return json.loads(_stat_index_path(root).read_text(encoding="utf-8")) + + +def _count_read_bytes(monkeypatch): + """Wrap Path.read_bytes with a call counter (file_hash's content read).""" + calls = {"n": 0} + orig = Path.read_bytes + + def counting(self): + calls["n"] += 1 + return orig(self) + + monkeypatch.setattr(Path, "read_bytes", counting) + return calls + + +def _fail_compute(p: Path) -> int: + raise AssertionError(f"word-count compute invoked for {p}; expected a warm stat hit") + + +def test_cache_hits_survive_corpus_move(tmp_path, monkeypatch): + """Run A under tmp/a, copy the corpus (with graphify-out/) to tmp/b: run B + must be 100% warm — zero content reads, zero word-count computes, digests + identical to run A.""" + _reset_stat_index() + a = tmp_path / "a" + a.mkdir() + (a / "f1.py").write_text("x = 1\n") + sub = a / "sub" + sub.mkdir() + (sub / "f2.md").write_text("hello world one two\n") + + digests_a = { + "f1.py": cache.file_hash(a / "f1.py", a), + "sub/f2.md": cache.file_hash(sub / "f2.md", a), + } + wc_a = cache.cached_word_count(a / "f1.py", a, lambda p: len(p.read_text().split())) + cache._flush_stat_index() + + on_disk = _read_index(a) + assert on_disk, "flush should have written entries" + for k in on_disk: + assert not os.path.isabs(k), f"absolute key leaked to disk: {k}" + assert "\\" not in k, f"non-portable separator in key: {k}" + assert set(on_disk) == {"f1.py", "sub/f2.md"} + + # Move the corpus (graphify-out/ rides along; copy2 preserves mtime_ns). + b = tmp_path / "b" + shutil.copytree(a, b, copy_function=shutil.copy2) + + _reset_stat_index() + reads = _count_read_bytes(monkeypatch) + assert cache.file_hash(b / "f1.py", b) == digests_a["f1.py"] + assert cache.file_hash(b / "sub" / "f2.md", b) == digests_a["sub/f2.md"] + assert cache.cached_word_count(b / "f1.py", b, _fail_compute) == wc_a + assert reads["n"] == 0, "moved corpus should be served entirely from the stat index" + + +def test_deleted_entries_are_pruned_on_flush(tmp_path): + _reset_stat_index() + a = tmp_path / "a" + a.mkdir() + f1 = a / "f1.py" + f1.write_text("x = 1\n") + f2 = a / "f2.py" + f2.write_text("y = 2\n") + cache.file_hash(f1, a) + cache.file_hash(f2, a) + cache._flush_stat_index() + assert set(_read_index(a)) == {"f1.py", "f2.py"} + + f2.unlink() + _reset_stat_index() + # Bump f1's mtime so the re-hash dirties the index and a flush is written. + os.utime(f1, ns=(f1.stat().st_atime_ns, f1.stat().st_mtime_ns + 1_000_000)) + cache.file_hash(f1, a) + cache._flush_stat_index() + + on_disk = _read_index(a) + assert set(on_disk) == {"f1.py"}, "deleted f2.py should have been pruned" + assert not os.path.isabs(next(iter(on_disk))) + + +def test_legacy_absolute_index_migrates_gracefully(tmp_path, monkeypatch): + """A pre-#2199 index keyed by absolute paths still HITS on the unmoved + root, and the first flush prunes dead entries and rewrites live keys + relative (self-heals).""" + _reset_stat_index() + a = tmp_path / "a" + a.mkdir() + f1 = a / "f1.py" + f1.write_text("x = 1\n") + st = f1.stat() + salt = "f1.py" + digest = hashlib.sha256(f1.read_bytes() + b"\x00" + salt.encode()).hexdigest() + + dead = tmp_path / "dead" # never created + legacy = { + str(f1.resolve()): {"size": st.st_size, "mtime_ns": st.st_mtime_ns, + "hashes": {salt: digest}}, + str(dead / "x.py"): {"size": 1, "mtime_ns": 1, "hashes": {"x.py": "aa"}}, + str(dead / "y.py"): {"size": 2, "mtime_ns": 2, "hashes": {"y.py": "bb"}}, + } + p = _stat_index_path(a) + p.parent.mkdir(parents=True) + p.write_text(json.dumps(legacy), encoding="utf-8") + + reads = _count_read_bytes(monkeypatch) + assert cache.file_hash(f1, a) == digest + assert reads["n"] == 0, "legacy absolute key should still serve a warm hit" + + # Force a write so the self-heal is observable (a pure warm run leaves the + # index clean and flush is a no-op by design). + cache._stat_index_dirty = True + cache._flush_stat_index() + + on_disk = _read_index(a) + assert set(on_disk) == {"f1.py"}, "dead absolute keys should be pruned" + assert on_disk["f1.py"]["hashes"][salt] == digest + + +def test_out_of_root_key_round_trips_absolute(tmp_path, monkeypatch): + _reset_stat_index() + a = tmp_path / "a" + a.mkdir() + outside = tmp_path / "outside.txt" + outside.write_text("out of root\n") + + d1 = cache.file_hash(outside, a) + cache._flush_stat_index() + + on_disk = _read_index(a) + assert set(on_disk) == {str(outside.resolve())}, "out-of-root key must stay absolute" + + _reset_stat_index() + reads = _count_read_bytes(monkeypatch) + assert cache.file_hash(outside, a) == d1 + assert reads["n"] == 0, "second call should be a stat hit" + + +def test_relative_key_wins_over_colliding_legacy_absolute(tmp_path): + """When an old absolute key and a new relative key resolve to the same + file, the relative (new-format) entry wins on load.""" + _reset_stat_index() + a = tmp_path / "a" + a.mkdir() + f1 = a / "f1.py" + f1.write_text("x = 1\n") + p = _stat_index_path(a) + p.parent.mkdir(parents=True) + p.write_text(json.dumps({ + str(f1.resolve()): {"size": 1, "mtime_ns": 1, "hashes": {"f1.py": "legacy"}}, + "f1.py": {"size": 2, "mtime_ns": 2, "hashes": {"f1.py": "fresh"}}, + }), encoding="utf-8") + + cache._ensure_stat_index(a) + assert cache._stat_index[str(f1.resolve())]["hashes"]["f1.py"] == "fresh" + + +def test_semantic_cache_normalizes_absolute_source_file(tmp_path): + """#2197: an item whose source_file is absolute is persisted root-relative + posix, and the caller's dict is not mutated.""" + _reset_stat_index() + root = tmp_path / "corpus" + root.mkdir() + f = root / "m.py" + f.write_text("x = 1\n") + + node = {"id": "m.x", "type": "variable", "source_file": str(f.resolve())} + saved = cache.save_semantic_cache([node], [], root=root) + assert saved == 1 + assert node["source_file"] == str(f.resolve()), "caller's dict must not be mutated" + + entries = list((root / "graphify-out" / "cache" / "semantic").glob("*.json")) + assert len(entries) == 1 + persisted = json.loads(entries[0].read_text(encoding="utf-8")) + assert persisted["nodes"][0]["source_file"] == "m.py" + + # Replay resolves back to the same absolute shape a fresh extraction has. + _, _, _, uncached = cache.check_semantic_cache([str(f)], root=root) + assert uncached == [] + + +def test_semantic_cache_normalizes_backslash_poisoned_source_file(tmp_path): + """A Windows-shaped absolute source_file (backslash separators) must be + slash-normalized and relativized instead of being skipped/persisted raw.""" + _reset_stat_index() + root = tmp_path / "corpus" + root.mkdir() + sub = root / "sub" + sub.mkdir() + f = sub / "n.py" + f.write_text("y = 2\n") + + poisoned = str(root.resolve()) + "\\sub\\n.py" + node = {"id": "n.y", "type": "variable", "source_file": poisoned} + saved = cache.save_semantic_cache([node], [], root=root) + assert saved == 1 + + entries = list((root / "graphify-out" / "cache" / "semantic").glob("*.json")) + assert len(entries) == 1 + persisted = json.loads(entries[0].read_text(encoding="utf-8")) + assert persisted["nodes"][0]["source_file"] == "sub/n.py"