diff --git a/graphify/detect.py b/graphify/detect.py index aaeb4de21..a6ec90aab 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -6,6 +6,7 @@ import os import re import shlex import stat +import time import unicodedata from concurrent.futures import ThreadPoolExecutor from enum import Enum @@ -31,6 +32,13 @@ class FileType(str, Enum): _MANIFEST_PATH = str(out_path("manifest.json")) +#: Window in which a manifest row's own timestamp is too close to the file's +#: mtime for "mtime unchanged" to prove the content is unchanged. Coarse for +#: filesystems that round mtime to whole seconds; tight when real sub-second +#: precision is reported. +_MTIME_COARSE_S = 2.0 +_MTIME_SUBSECOND_S = 0.05 + CODE_EXTENSIONS = {'.py', '.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '.mjs', '.cjs', '.ejs', '.ets', '.go', '.rs', '.java', '.groovy', '.gradle', '.cpp', '.cc', '.cxx', '.c', '.h', '.hpp', '.cu', '.cuh', '.metal', '.rb', '.rake', '.swift', '.kt', '.kts', '.cs', '.scala', '.php', '.lua', '.luau', '.toc', '.zig', '.ps1', '.psm1', '.psd1', '.ex', '.exs', '.m', '.mm', '.jl', '.vue', '.svelte', '.astro', '.dart', '.v', '.sv', '.svh', '.sql', '.r', '.f', '.F', '.f90', '.F90', '.f95', '.F95', '.f03', '.F03', '.f08', '.F08', '.pas', '.pp', '.dpr', '.dpk', '.lpr', '.inc', '.dfm', '.lfm', '.lpk', '.sh', '.bash', '.json', '.tf', '.tfvars', '.hcl', '.dm', '.dme', '.dmi', '.dmm', '.dmf', '.sln', '.slnx', '.csproj', '.fsproj', '.vbproj', '.xaml', '.razor', '.cshtml', '.cls', '.trigger'} DOC_EXTENSIONS = {'.md', '.mdx', '.qmd', '.skill', '.txt', '.rst', '.html', '.yaml', '.yml'} PAPER_EXTENSIONS = {'.pdf'} @@ -1925,7 +1933,11 @@ def save_manifest( mtime, h = hashed[f] key = _nfc(f) prev = _normalise_entry(existing.get(key, {})) or {} - entry: dict = {"mtime": mtime} + # seen: when this row was written. If the file's mtime sits inside the + # same filesystem tick, a later same-length edit can land in that tick + # without moving mtime, so the mtime-unchanged fastpath cannot prove + # the content is still current and detect_incremental re-hashes. + entry: dict = {"mtime": mtime, "seen": time.time()} if kind in ("ast", "both"): entry["ast_hash"] = h else: @@ -1952,6 +1964,35 @@ def save_manifest( write_json_atomic(manifest_path, manifest, indent=2) +def _mtime_may_hide_a_rewrite(current_mtime: float, stored: dict) -> bool: + """Was this manifest row written in the same tick as the file it describes? + + The incremental gate treats "mtime unchanged" as proof the content is + unchanged. That is only true while the filesystem can distinguish the two + writes: an edit keeping the file the same length and landing in the same + timestamp tick moves neither size nor mtime, so the file silently skips + re-extraction and the graph keeps serving the old content. + + ``seen`` records when the row was stamped. If the file's mtime falls inside + the same tick, this row cannot prove currency and the caller pays for one + MD5. Every other row — the whole settled corpus, and any manifest written + by an earlier run — keeps the free stat-only fastpath. + + Rows predating ``seen`` are treated as safe: they necessarily come from an + earlier process, where a later write would have had to move mtime. + """ + seen = stored.get("seen") + if not isinstance(seen, (int, float)): + return False + delta = float(seen) - float(current_mtime) + if delta < 0: + return False # file is newer than the row; the mtime check already fired + # Derive granularity from the timestamp: a whole-second mtime means the + # filesystem cannot separate writes inside that second. + coarse = float(current_mtime).is_integer() + return delta < (_MTIME_COARSE_S if coarse else _MTIME_SUBSECOND_S) + + def detect_incremental( root: Path, manifest_path: str = _MANIFEST_PATH, @@ -2047,6 +2088,14 @@ def detect_incremental( if stored_mtime is None or current_mtime != stored_mtime: # mtime bumped — verify with content hash before re-extracting changed = _md5_file(Path(f)) != stored_hash + elif _mtime_may_hide_a_rewrite(current_mtime, stored): + # mtime is unchanged, but it was recorded in the same + # filesystem tick the file was written in — a later + # same-length edit lands in that tick without moving + # mtime, and the file silently skips re-extraction + # while the graph keeps serving the old content. + # Only this narrow window pays for a content hash. + changed = _md5_file(Path(f)) != stored_hash else: changed = False else: diff --git a/tests/test_incremental_mtime_collision.py b/tests/test_incremental_mtime_collision.py new file mode 100644 index 000000000..942dcb10b --- /dev/null +++ b/tests/test_incremental_mtime_collision.py @@ -0,0 +1,126 @@ +"""An incremental run must not skip a file whose mtime did not move. + +``detect_incremental`` treats "mtime unchanged" as proof the content is +unchanged. That holds only while the filesystem can separate the two writes: an +edit keeping the file the same length and landing inside one timestamp tick +moves neither size nor mtime, so the file is classified unchanged and never +re-extracted, while the graph keeps serving the old content. + +The stat-only fastpath is the point of the gate and must survive: a settled +corpus with a manifest from an earlier run has to cost zero content hashes. +""" +import os +import tempfile +import time +from pathlib import Path + +import pytest + +from graphify import detect as det + + +@pytest.fixture() +def corpus(tmp_path): + src = tmp_path / "src" + src.mkdir() + for i in range(3): + (src / f"f{i}.md").write_text(f"# Doc {i}\n\nBody {i}.\n", encoding="utf-8") + # manifest lives outside the scanned tree so it is never part of the corpus + return src, str(tmp_path / "manifest.json") + + +def _queued(result): + return [f for flist in result["new_files"].values() for f in flist] + + +def test_same_size_rewrite_in_one_tick_is_requeued(corpus): + """The bug: a rewrite the filesystem cannot distinguish by stat alone. + + The mtime is pinned back to the manifest's recorded value so the collision + is reproduced deterministically rather than depending on how fast the + manifest write happened to be. + """ + import json + + src, manifest = corpus + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + target = src / "f1.md" + stat_before = target.stat() + target.write_text("# Doc X\n\nBody Y.\n", encoding="utf-8") + # same length, and stat now reports exactly what the manifest recorded + os.utime(target, ns=(stat_before.st_atime_ns, stat_before.st_mtime_ns)) + + assert target.stat().st_size == stat_before.st_size + assert target.stat().st_mtime_ns == stat_before.st_mtime_ns + + queued = _queued(det.detect_incremental(src, manifest, kind="semantic")) + assert len(queued) == 1, "a rewritten file must be re-queued" + assert queued[0].endswith("f1.md") + + +def test_a_settled_corpus_costs_no_content_hashes(corpus, monkeypatch): + """The fastpath must survive: an untouched corpus does zero MD5 work.""" + src, manifest = corpus + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + old = time.time() - 3600 + for f in src.glob("*.md"): + os.utime(f, (old, old)) + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + calls = {"n": 0} + real = det._md5_file + + def counting(p): + calls["n"] += 1 + return real(p) + + monkeypatch.setattr(det, "_md5_file", counting) + result = det.detect_incremental(src, manifest, kind="semantic") + + assert calls["n"] == 0, "an unchanged corpus must not be re-hashed" + assert _queued(result) == [] + + +def test_a_genuinely_edited_file_is_still_requeued(corpus): + """Control: the ordinary size-change path is untouched.""" + src, manifest = corpus + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + (src / "f2.md").write_text( + "# Doc 2\n\nA substantially longer body than before.\n", encoding="utf-8") + + queued = _queued(det.detect_incremental(src, manifest, kind="semantic")) + assert len(queued) == 1 and queued[0].endswith("f2.md") + + +def test_an_untouched_file_is_not_requeued_after_a_neighbour_changes(corpus): + """Only the edited file moves; its neighbours keep the fastpath.""" + src, manifest = corpus + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + old = time.time() - 3600 + for f in src.glob("*.md"): + os.utime(f, (old, old)) + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + + (src / "f0.md").write_text("# Doc 0\n\nA different and longer body.\n", + encoding="utf-8") + queued = _queued(det.detect_incremental(src, manifest, kind="semantic")) + assert len(queued) == 1 and queued[0].endswith("f0.md") + + +def test_a_legacy_manifest_row_without_seen_still_works(corpus): + """Rows predating the `seen` field come from an earlier run: trusted.""" + import json + + src, manifest = corpus + det.save_manifest(det.detect(src)["files"], manifest, root=src, kind="semantic") + data = json.loads(Path(manifest).read_text(encoding="utf-8")) + for row in data.values(): + if isinstance(row, dict): + row.pop("seen", None) + Path(manifest).write_text(json.dumps(data), encoding="utf-8") + + assert _queued(det.detect_incremental(src, manifest, kind="semantic")) == []