fix(detect): prevent manifest timestamp churn on unchanged updates (#2838)
save_manifest unconditionally stamped a fresh 'seen' timestamp for every file on every run, so a no-op graphify update rewrote all manifest entries and left graphify-out/ permanently dirty (a trailing graph commit on every push). Now the per-file 'seen' is preserved for genuinely unchanged entries, and the disk write is skipped entirely when the serialized manifest is byte-identical to what is already on disk; a real change still refreshes and persists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
1eb356cde1
commit
173a8814d2
+33
-9
@@ -2125,20 +2125,33 @@ def save_manifest(
|
||||
mtime, h = hashed[f]
|
||||
key = _nfc(f)
|
||||
prev = _normalise_entry(existing.get(key, {})) or {}
|
||||
# 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
|
||||
ast_h = h
|
||||
else:
|
||||
entry["ast_hash"] = prev.get("ast_hash", "")
|
||||
ast_h = prev.get("ast_hash", "")
|
||||
if kind in ("semantic", "both"):
|
||||
entry["semantic_hash"] = h
|
||||
sem_h = h
|
||||
else:
|
||||
# Preserve semantic_hash only when content is unchanged
|
||||
entry["semantic_hash"] = prev.get("semantic_hash", "") if h == prev.get("ast_hash", "") else ""
|
||||
sem_h = prev.get("semantic_hash", "") if h == prev.get("ast_hash", "") else ""
|
||||
|
||||
# Preserve previous seen timestamp if the entry's mtime and target hash(es)
|
||||
# are genuinely unchanged and no clear was requested for this file.
|
||||
prev_seen = prev.get("seen")
|
||||
is_unchanged = (
|
||||
isinstance(prev_seen, (int, float))
|
||||
and mtime == prev.get("mtime")
|
||||
and (ast_h == prev.get("ast_hash", "") if kind in ("ast", "both") else True)
|
||||
and (sem_h == prev.get("semantic_hash", "") if kind in ("semantic", "both") else True)
|
||||
and not _in_clear_ast(f)
|
||||
and not _in_clear(f)
|
||||
)
|
||||
entry: dict = {
|
||||
"mtime": mtime,
|
||||
"seen": prev_seen if is_unchanged else time.time(),
|
||||
"ast_hash": ast_h,
|
||||
"semantic_hash": sem_h,
|
||||
}
|
||||
manifest[key] = entry
|
||||
if root is not None:
|
||||
# Persist in portable form: forward-slash relative paths. Keys outside
|
||||
@@ -2150,6 +2163,17 @@ def save_manifest(
|
||||
manifest = {_nfc(_to_relative_for_storage(k, root)): v for k, v in manifest.items()}
|
||||
else:
|
||||
manifest = {_nfc(k): v for k, v in manifest.items()}
|
||||
|
||||
# Avoid rewriting manifest.json when the serialized payload is identical (#2838).
|
||||
manifest_p = Path(manifest_path)
|
||||
if manifest_p.is_file():
|
||||
try:
|
||||
disk_raw = json.loads(manifest_p.read_text(encoding="utf-8"))
|
||||
if isinstance(disk_raw, dict) and disk_raw == manifest:
|
||||
return
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
from graphify.paths import write_json_atomic
|
||||
# Atomic write: a crash mid-write must not leave a truncated manifest that
|
||||
# detect_incremental then fails to parse.
|
||||
|
||||
@@ -2989,6 +2989,68 @@ def test_detect_incremental_exclusion_stable_across_runs(tmp_path):
|
||||
assert inc2["excluded_files"] == []
|
||||
|
||||
|
||||
# ── #2838: manifest seen timestamps preserved for unchanged entries ──
|
||||
|
||||
def test_save_manifest_unchanged_file_preserves_seen(tmp_path):
|
||||
"""#2838: save_manifest preserves existing seen timestamp for unchanged entries."""
|
||||
import json
|
||||
a = tmp_path / "a.py"
|
||||
a.write_text("x = 1\n", encoding="utf-8")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
|
||||
save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path)
|
||||
raw1 = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
seen_1 = raw1["a.py"]["seen"]
|
||||
assert isinstance(seen_1, (int, float))
|
||||
|
||||
# Second save on unchanged file must keep identical seen value
|
||||
save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path)
|
||||
raw2 = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
assert raw2["a.py"]["seen"] == seen_1
|
||||
assert raw2["a.py"]["ast_hash"] == raw1["a.py"]["ast_hash"]
|
||||
assert raw2["a.py"]["mtime"] == raw1["a.py"]["mtime"]
|
||||
|
||||
|
||||
def test_save_manifest_changed_file_updates_seen(tmp_path):
|
||||
"""#2838: save_manifest assigns a new seen timestamp when file content changes."""
|
||||
import json
|
||||
import time
|
||||
a = tmp_path / "a.py"
|
||||
a.write_text("x = 1\n", encoding="utf-8")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
|
||||
save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path)
|
||||
raw1 = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
seen_1 = raw1["a.py"]["seen"]
|
||||
|
||||
# Modify file content (new hash)
|
||||
time.sleep(0.01)
|
||||
a.write_text("x = 2\n", encoding="utf-8")
|
||||
save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path)
|
||||
raw2 = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
|
||||
assert raw2["a.py"]["seen"] >= seen_1
|
||||
assert raw2["a.py"]["ast_hash"] != raw1["a.py"]["ast_hash"]
|
||||
|
||||
|
||||
def test_save_manifest_noop_skips_disk_write(tmp_path):
|
||||
"""#2838: save_manifest does not rewrite manifest.json when payload is identical."""
|
||||
a = tmp_path / "a.py"
|
||||
a.write_text("x = 1\n", encoding="utf-8")
|
||||
manifest_path = Path(tmp_path / "graphify-out" / "manifest.json")
|
||||
|
||||
save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path)
|
||||
mtime_1 = manifest_path.stat().st_mtime_ns
|
||||
bytes_1 = manifest_path.read_bytes()
|
||||
|
||||
save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path)
|
||||
mtime_2 = manifest_path.stat().st_mtime_ns
|
||||
bytes_2 = manifest_path.read_bytes()
|
||||
|
||||
assert bytes_1 == bytes_2
|
||||
assert mtime_1 == mtime_2
|
||||
|
||||
|
||||
# ── #2106: sensitive-filter over-match (prose/source rescued, real secrets kept) ──
|
||||
|
||||
@pytest.mark.parametrize("path", [
|
||||
|
||||
Reference in New Issue
Block a user