fix(detect): re-queue a same-tick same-length rewrite in incremental runs (#2466)

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:

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")) == []