diff --git a/graphify/detect.py b/graphify/detect.py index eec8aaf1d..122a69595 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -40,6 +40,35 @@ class FileType(str, Enum): #: keep the two in sync. _MTIME_COARSE_S = 2.0 _MTIME_SUBSECOND_S = 0.05 +_MTIME_GRANULARITY_NS = 2_000_000_000 + + +def _mtime_granularity_ns() -> int: + """Return the assumed filesystem mtime granularity in nanoseconds. + + Read fresh on every call so tests can override GRAPHIFY_MTIME_GRANULARITY_MS. + """ + raw = os.environ.get("GRAPHIFY_MTIME_GRANULARITY_MS", "").strip() + if raw: + try: + ms = float(raw) + except ValueError: + return _MTIME_GRANULARITY_NS + if ms >= 0: + return int(ms * 1_000_000) + return _MTIME_GRANULARITY_NS + + +def _is_racily_clean(st_mtime_ns: int, indexed_at_ns: int) -> bool: + """True if indexed_at_ns falls inside the racily-clean window of st_mtime_ns. + + If st_mtime_ns + granularity > indexed_at_ns, the write tick had not yet + safely closed when indexed_at_ns was recorded. A subsequent edit inside + the same tick could preserve size and mtime_ns, so the stat fastpath cannot + prove content currency without hashing. + """ + return st_mtime_ns + _mtime_granularity_ns() > indexed_at_ns + 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', '.ml', '.mli', '.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', '.lisp', '.cl', '.lsp', '.asd', '.robot', '.resource'} DOC_EXTENSIONS = {'.md', '.mdx', '.qmd', '.skill', '.txt', '.rst', '.html', '.yaml', '.yml'} @@ -2099,6 +2128,19 @@ def _stat_and_hash(path_str: str) -> tuple[str, float, str] | None: return None +def _stat_and_hash_for_manifest(path_str: str) -> tuple[str, str, int, int, int] | None: + """Stat + MD5 for manifest and incremental state: (path, md5, size, mtime_ns, observed_at_ns).""" + try: + p = Path(path_str) + t_before = time.time_ns() + st = os.stat(_os_path(p)) + h = _md5_file(p) + return path_str, h, st.st_size, st.st_mtime_ns, t_before + except OSError: + return None + + + def _nfc(s: str) -> str: """NFC-normalize a path string used as a manifest key. @@ -2193,6 +2235,78 @@ def load_manifest( return {_nfc(_to_absolute_from_storage(k, root)): v for k, v in raw.items()} +def _state_path_for(manifest_path: str | Path = _MANIFEST_PATH) -> Path: + """Location of the machine-local incremental filesystem state for a manifest.""" + return Path(manifest_path).parent / "cache" / "incremental-state.json" + + +def _load_incremental_state( + manifest_path: str | Path = _MANIFEST_PATH, + *, + root: Path | None = None, +) -> dict[str, dict]: + """Load the machine-local incremental state file. Returns {} on any error. + + Keys are normalized to NFC. When ``root`` is provided, stored relative + keys are re-anchored against ``root`` to match the absolute keys used + internally by detect_incremental. + """ + state_file = _state_path_for(manifest_path) + try: + raw = json.loads(state_file.read_text(encoding="utf-8")) + except Exception: + return {} + if not isinstance(raw, dict): + return {} + cleaned: dict[str, dict] = {} + for k, v in raw.items(): + if isinstance(v, dict): + size = v.get("size") + mtime_ns = v.get("mtime_ns") + indexed_at_ns = v.get("indexed_at_ns") + if isinstance(size, int) and isinstance(mtime_ns, int) and isinstance(indexed_at_ns, int): + target_k = _nfc(_to_absolute_from_storage(k, root)) if root is not None else _nfc(k) + cleaned[target_k] = { + "size": size, + "mtime_ns": mtime_ns, + "indexed_at_ns": indexed_at_ns, + } + return cleaned + + +def _save_incremental_state( + state: dict[str, dict], + manifest_path: str | Path = _MANIFEST_PATH, + *, + root: Path | None = None, +) -> None: + """Synchronously and atomically save machine-local incremental state. + + Keys are stored as forward-slash relative paths when ``root`` is provided, + matching manifest.json storage. Serialization is deterministic (keys sorted). + Skips disk write if the on-disk state is identical. + """ + if root is not None: + disk_state = {_nfc(_to_relative_for_storage(k, root)): v for k, v in state.items()} + else: + disk_state = {_nfc(k): v for k, v in state.items()} + + disk_state = {k: disk_state[k] for k in sorted(disk_state.keys())} + state_file = _state_path_for(manifest_path) + + if state_file.is_file(): + try: + current_raw = json.loads(state_file.read_text(encoding="utf-8")) + if isinstance(current_raw, dict) and current_raw == disk_state: + return + except Exception: + pass + + from graphify.paths import write_json_atomic + state_file.parent.mkdir(parents=True, exist_ok=True) + write_json_atomic(str(state_file), disk_state, indent=2) + + def save_manifest( files: dict[str, list[str]], manifest_path: str = _MANIFEST_PATH, @@ -2203,11 +2317,11 @@ def save_manifest( clear_semantic: set[str] | list[str] | None = None, clear_ast: set[str] | list[str] | None = None, ) -> None: - """Save current file mtimes + content hashes for change detection. + """Save content hashes in manifest.json and local filesystem stats in cache/incremental-state.json. kind="ast" — written by `graphify update` (AST-only rebuild). Stamps ast_hash; preserves an existing semantic_hash only when - the file content is unchanged (mtime + hash match). + the file content is unchanged (content hash matches). kind="semantic" — written by `graphify extract` after semantic extraction. Stamps semantic_hash; preserves existing ast_hash. kind="both" — full pipeline: stamps both hashes (default). @@ -2222,28 +2336,14 @@ def save_manifest( corpus (absolute paths) so seeded rows for in-root files that are still alive on disk but no longer part of the scan (newly excluded via .graphifyignore/.gitignore/--exclude) are dropped instead of surviving - forever and masquerading as deletions in detect_incremental. It must be - the RAW detect output, not a stamp-filtered subset — pruning to a - filtered set would erase rows the filter merely omitted (failed chunks, - --code-only doc rows). Out-of-root entries are never pruned. Callers - saving a SUBSET of files (changed_paths hooks, skill runbooks, #917) - must leave this None so their untouched rows are preserved. - - ``clear_semantic`` (#1948): files that were dispatched this run but - produced no stamped output (e.g. the LLM omitted their chunk on a - --force re-run) are absent from ``files``, so the seed loop below would - otherwise copy their prior semantic_hash verbatim — masking the omission - and making detect_incremental(kind="semantic") report them unchanged. - Pass the set of such files (any path form ``scan_corpus`` accepts) to - force their seeded semantic_hash to "" instead of inheriting it. - - ``clear_ast`` (#2543): same idea for AST failures (missing optional extra, - zero-node anomalous extract). Blanks BOTH ``ast_hash`` and - ``semantic_hash`` on the seeded row so either detect_incremental kind - re-queues the file after the failure is fixed, without deleting - graphify-out/. + forever and masquerading as deletions in detect_incremental. + + ``clear_semantic`` (#1948): forces seeded semantic_hash to "" for omitted files. + + ``clear_ast`` (#2543): blanks BOTH ast_hash and semantic_hash on AST failures. """ existing = load_manifest(manifest_path, root=root) + existing_state = _load_incremental_state(manifest_path, root=root) # Index both raw and NFC forms so scan/clear membership survives the # same NFC/NFD mismatch that breaks manifest lookups (#2221). @@ -2314,20 +2414,18 @@ def _in_root(path_str: str) -> bool: def _normalise_entry(entry): if isinstance(entry, (int, float)): - return {"mtime": entry, "ast_hash": "", "semantic_hash": ""} - if isinstance(entry, dict) and "hash" in entry and "ast_hash" not in entry: - return {"mtime": entry.get("mtime", 0), "ast_hash": entry["hash"], "semantic_hash": ""} + return {"ast_hash": "", "semantic_hash": ""} if isinstance(entry, dict): - return entry + ast_h = entry.get("ast_hash", entry.get("hash", "")) + sem_h = entry.get("semantic_hash", "") + return { + "ast_hash": ast_h if isinstance(ast_h, str) else "", + "semantic_hash": sem_h if isinstance(sem_h, str) else "", + } return None # Seed from the existing manifest so incremental callers passing a subset # of files don't silently erase entries for untouched files (#917). - # Prune entries whose file no longer exists on disk — those are genuine - # deletions that detect_incremental() should treat as gone. When the - # caller supplied the full scan corpus, additionally prune in-root rows - # the scan no longer covers: those files were excluded, not deleted, and - # keeping the row makes them look deleted on every future run (#1908). manifest: dict[str, dict] = {} for f, entry in existing.items(): normalised = _normalise_entry(entry) @@ -2341,26 +2439,27 @@ def _normalise_entry(entry): if scan_set is not None and not _in_scan(f) and _in_root(f): continue # excluded-but-alive: drop the stale row (#1908) if clear_ast_set is not None and _in_clear_ast(f): - # AST failure this run (missing extra / zero nodes, #2543): blank - # both hashes so either detect_incremental kind re-queues. - normalised = {**normalised, "ast_hash": "", "semantic_hash": ""} + # AST failure this run (#2543): blank both hashes so detect_incremental re-queues. + normalised = {"ast_hash": "", "semantic_hash": ""} elif clear_set is not None and _in_clear(f): - # Dispatched-but-omitted this run: don't inherit the stale - # semantic_hash, or detect_incremental would call it unchanged (#1948). + # Dispatched-but-omitted this run (#1948): clear semantic_hash. normalised = {**normalised, "semantic_hash": ""} manifest[f] = normalised + # Seed machine-local incremental state from existing state for surviving manifest files + state: dict[str, dict] = {f: existing_state[f] for f in manifest if f in existing_state} + all_files = [f for file_list in files.values() for f in file_list] with ThreadPoolExecutor() as pool: - raw = pool.map(_stat_and_hash, all_files) - hashed: dict[str, tuple[float, str]] = { - r[0]: (r[1], r[2]) for r in raw if r is not None + raw = pool.map(_stat_and_hash_for_manifest, all_files) + hashed: dict[str, tuple[str, int, int, int]] = { + r[0]: (r[1], r[2], r[3], r[4]) for r in raw if r is not None } for f in all_files: if f not in hashed: continue # file deleted between detect() and manifest write - mtime, h = hashed[f] + h, size, mtime_ns, observed_at_ns = hashed[f] key = _nfc(f) prev = _normalise_entry(existing.get(key, {})) or {} if kind in ("ast", "both"): @@ -2373,76 +2472,71 @@ def _normalise_entry(entry): # Preserve semantic_hash only when content is unchanged 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(), + manifest[key] = { "ast_hash": ast_h, "semantic_hash": sem_h, } - manifest[key] = entry + + # Check if local state already has a valid entry for this unchanged file + prev_state = existing_state.get(key) + if ( + isinstance(prev_state, dict) + and prev_state.get("size") == size + and prev_state.get("mtime_ns") == mtime_ns + and isinstance(prev_state.get("indexed_at_ns"), int) + and prev.get("ast_hash") == ast_h + and prev.get("semantic_hash") == sem_h + and not _in_clear_ast(f) + and not _in_clear(f) + ): + # File is completely unchanged; preserve prior indexed_at_ns to avoid state churn + indexed_at_ns = prev_state["indexed_at_ns"] + else: + indexed_at_ns = observed_at_ns + + state[key] = { + "size": size, + "mtime_ns": mtime_ns, + "indexed_at_ns": indexed_at_ns, + } + + # Prune state entries that are no longer in manifest + state = {k: state[k] for k in manifest if k in state} + if root is not None: - # Persist in portable form: forward-slash relative paths. Keys outside - # ``root`` (out-of-tree symlinked corpora, --include sources) keep - # their absolute form so the manifest round-trips on the saving - # machine even when not every entry can be portably encoded. - # NFC after relativize so on-disk keys match what load_manifest - # re-anchors and compares against (#2221). 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()} + manifest = {k: manifest[k] for k in sorted(manifest.keys())} # Avoid rewriting manifest.json when the serialized payload is identical (#2838). manifest_p = Path(manifest_path) + should_write_manifest = True 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 + should_write_manifest = False 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. - write_json_atomic(manifest_path, manifest, indent=2) - + if should_write_manifest: + from graphify.paths import write_json_atomic + manifest_p.parent.mkdir(parents=True, exist_ok=True) + 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? + # Synchronously and atomically write cache/incremental-state.json + _save_incremental_state(state, manifest_path, root=root) - 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. - """ +def _mtime_may_hide_a_rewrite(current_mtime: float, stored: dict) -> bool: + """Legacy helper preserved for backward compatibility.""" 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. + return False coarse = float(current_mtime).is_integer() return delta < (_MTIME_COARSE_S if coarse else _MTIME_SUBSECOND_S) @@ -2461,23 +2555,15 @@ def detect_incremental( kind="semantic" (default for extract): a file is "changed" when its semantic_hash is missing or its content has changed since the last - semantic extraction pass. Use this for `graphify extract` so that - files touched by `graphify update` (AST-only) are re-extracted - semantically. + semantic extraction pass. kind="ast": a file is "changed" when its ast_hash is missing or its content has changed. Use this for `graphify update`. - Fast path: mtime unchanged + hash matches → unchanged (free, no disk IO - beyond stat). Slow path: mtime bumped → compare MD5 against the relevant - hash field before re-extracting. - - Backwards compatible with legacy manifests storing plain float mtime values - or {mtime, hash} dicts (treated as ast_hash only; semantic_hash = miss). - - The ``follow_symlinks`` flag is forwarded to :func:`detect` so in-root - symlinked sub-trees are scanned consistently between full and incremental - runs. ``None`` (default) does not follow symlinked directories; callers must - opt in explicitly, and resolved targets outside the scan root are skipped. + Fast path: local cache/incremental-state.json has matching size + mtime_ns + outside the racily-clean window → unchanged (zero MD5 hashing). + Slow path: local state missing, stale, or within racily-clean window → + compare MD5 against manifest.json content hash. If matching, update local + state without mutating manifest.json. """ full = detect( root, @@ -2501,57 +2587,71 @@ def detect_incremental( full["excluded_files"] = [] return full + state = _load_incremental_state(manifest_path, root=root) + state_updated = False + new_files: dict[str, list[str]] = {k: [] for k in full["files"]} unchanged_files: dict[str, list[str]] = {k: [] for k in full["files"]} for ftype, file_list in full["files"].items(): for f in file_list: - # Manifest keys are NFC; scan paths may arrive NFD (#2221). - stored = manifest.get(_nfc(f)) - try: - current_mtime = os.stat(_os_path(Path(f))).st_mtime - except Exception: - current_mtime = 0 - - # Legacy manifest: plain float value stores only mtime. - # Compare with `!=` so backwards mtime motion (git checkout of an - # older commit, tarball restore, rsync --times) still triggers a - # re-extract; the previous `>` silently kept the stale cache and - # the graph drifted from disk (#1859). No stored hash means we - # cannot verify content — any mtime delta forces a re-extract, - # and the next save promotes the entry into the dict schema. - if isinstance(stored, (int, float)): + key = _nfc(f) + stored = manifest.get(key) + if stored is None: + changed = True + elif isinstance(stored, (int, float)): + # Legacy manifest: plain float value stores only mtime + try: + current_mtime = os.stat(_os_path(Path(f))).st_mtime + except Exception: + current_mtime = 0 changed = current_mtime != stored elif isinstance(stored, dict): - # Normalise legacy {mtime, hash} to new schema if "hash" in stored and "ast_hash" not in stored: - stored = {"mtime": stored.get("mtime", 0), "ast_hash": stored["hash"], "semantic_hash": ""} + stored = {"ast_hash": stored["hash"], "semantic_hash": ""} hash_key = "semantic_hash" if kind == "semantic" else "ast_hash" stored_hash = stored.get(hash_key, "") - # Missing semantic_hash means update ran but extract hasn't — always re-extract if not stored_hash: + # Missing hash means update ran but extract hasn't — always re-extract changed = True else: - stored_mtime = stored.get("mtime") - # Schema-drift guard (#1163): tolerate a nested {mtime: ...} - # dict or any non-numeric value without crashing. - if isinstance(stored_mtime, dict): - stored_mtime = stored_mtime.get("mtime") - if not isinstance(stored_mtime, (int, float)): - stored_mtime = None - 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 + try: + p = Path(f) + st = os.stat(_os_path(p)) + except OSError: + st = None + + if st is None: + changed = True else: - changed = False + state_entry = state.get(key) + is_fastpath = ( + isinstance(state_entry, dict) + and state_entry.get("size") == st.st_size + and state_entry.get("mtime_ns") == st.st_mtime_ns + and isinstance(state_entry.get("indexed_at_ns"), int) + and not _is_racily_clean(st.st_mtime_ns, state_entry["indexed_at_ns"]) + ) + + if is_fastpath: + changed = False + else: + # Local state missing, stale, or racily-clean: + # fall back to MD5 content hashing against manifest hash. + t_before = time.time_ns() + current_hash = _md5_file(Path(f)) + if current_hash and current_hash == stored_hash: + changed = False + # Content confirmed identical! Update local state + # so subsequent calls hit the fastpath. + state[key] = { + "size": st.st_size, + "mtime_ns": st.st_mtime_ns, + "indexed_at_ns": t_before, + } + state_updated = True + else: + changed = True else: changed = True # unknown format, re-extract to be safe @@ -2560,6 +2660,11 @@ def detect_incremental( else: unchanged_files[ftype].append(f) + if state_updated: + # Prune state to manifest keys before saving + clean_state = {k: v for k, v in state.items() if k in manifest} + _save_incremental_state(clean_state, manifest_path, root=root) + # Manifest rows that left the corpus, split by disk existence (#1908): # a row whose file is gone from DISK is a genuine deletion (its cached # nodes are ghosts); a row whose file still exists but is out of the diff --git a/tests/test_detect.py b/tests/test_detect.py index 22099029a..165d3187a 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -3272,52 +3272,64 @@ def test_detect_incremental_exclusion_stable_across_runs(tmp_path): assert inc2["excluded_files"] == [] -# ── #2838: manifest seen timestamps preserved for unchanged entries ── +# ── #3643: manifest byte stability and machine-local incremental state ── -def test_save_manifest_unchanged_file_preserves_seen(tmp_path): - """#2838: save_manifest preserves existing seen timestamp for unchanged entries.""" +def test_save_manifest_unchanged_file_preserves_local_state(tmp_path): + """#3643: save_manifest stores hashes in manifest.json and preserves local state 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") + manifest_path = tmp_path / "graphify-out" / "manifest.json" + state_path = tmp_path / "graphify-out" / "cache" / "incremental-state.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)) + save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path) + raw1 = json.loads(manifest_path.read_text(encoding="utf-8")) + assert set(raw1["a.py"].keys()) == {"ast_hash", "semantic_hash"} - # 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"] + state1 = json.loads(state_path.read_text(encoding="utf-8")) + indexed_1 = state1["a.py"]["indexed_at_ns"] + assert isinstance(indexed_1, int) + + # Second save on unchanged file must keep identical local state and manifest bytes + save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path) + raw2 = json.loads(manifest_path.read_text(encoding="utf-8")) + state2 = json.loads(state_path.read_text(encoding="utf-8")) + + assert set(raw2["a.py"].keys()) == {"ast_hash", "semantic_hash"} + assert raw2["a.py"] == raw1["a.py"] + assert state2["a.py"]["indexed_at_ns"] == indexed_1 + assert state2["a.py"]["mtime_ns"] == state1["a.py"]["mtime_ns"] + assert state2["a.py"]["size"] == state1["a.py"]["size"] -def test_save_manifest_changed_file_updates_seen(tmp_path): - """#2838: save_manifest assigns a new seen timestamp when file content changes.""" +def test_save_manifest_changed_file_updates_state(tmp_path): + """#3643: save_manifest updates hash and incremental-state 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") + manifest_path = tmp_path / "graphify-out" / "manifest.json" + state_path = tmp_path / "graphify-out" / "cache" / "incremental-state.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"] + save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path) + raw1 = json.loads(manifest_path.read_text(encoding="utf-8")) + state1 = json.loads(state_path.read_text(encoding="utf-8")) + indexed_1 = state1["a.py"]["indexed_at_ns"] # 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")) + save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path) + raw2 = json.loads(manifest_path.read_text(encoding="utf-8")) + state2 = json.loads(state_path.read_text(encoding="utf-8")) - assert raw2["a.py"]["seen"] >= seen_1 + assert set(raw2["a.py"].keys()) == {"ast_hash", "semantic_hash"} assert raw2["a.py"]["ast_hash"] != raw1["a.py"]["ast_hash"] + assert state2["a.py"]["indexed_at_ns"] >= indexed_1 def test_save_manifest_noop_skips_disk_write(tmp_path): - """#2838: save_manifest does not rewrite manifest.json when payload is identical.""" + """#2838 / #3643: 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") @@ -3335,7 +3347,7 @@ def test_save_manifest_noop_skips_disk_write(tmp_path): def test_save_manifest_ast_kind_noop_then_change(tmp_path): - """#2838's literal path: `graphify update` calls save_manifest with kind='ast'. + """#2838 / #3643: `graphify update` calls save_manifest with kind='ast'. A no-op re-run must leave the manifest byte-identical; a real edit must update it.""" import json a = tmp_path / "a.py" @@ -3344,7 +3356,8 @@ def test_save_manifest_ast_kind_noop_then_change(tmp_path): save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path, kind="ast") bytes_1 = manifest_path.read_bytes() - seen_1 = json.loads(bytes_1)["a.py"]["seen"] + raw_1 = json.loads(bytes_1) + assert set(raw_1["a.py"].keys()) == {"ast_hash", "semantic_hash"} save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path, kind="ast") # no-op assert manifest_path.read_bytes() == bytes_1, "ast-kind no-op re-run churned the manifest" @@ -3353,8 +3366,8 @@ def test_save_manifest_ast_kind_noop_then_change(tmp_path): a.write_text("x = 2\n", encoding="utf-8") save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path, kind="ast") raw2 = json.loads(manifest_path.read_text(encoding="utf-8")) - assert raw2["a.py"]["ast_hash"] != json.loads(bytes_1)["a.py"]["ast_hash"] - assert raw2["a.py"]["seen"] >= seen_1 + assert raw2["a.py"]["ast_hash"] != raw_1["a.py"]["ast_hash"] + assert set(raw2["a.py"].keys()) == {"ast_hash", "semantic_hash"} def test_save_manifest_corrupt_existing_manifest_still_writes(tmp_path): @@ -3369,7 +3382,9 @@ def test_save_manifest_corrupt_existing_manifest_still_writes(tmp_path): save_manifest({"code": [str(a)]}, str(manifest_path), root=tmp_path) raw = json.loads(manifest_path.read_text(encoding="utf-8")) # must parse now - assert "a.py" in raw and isinstance(raw["a.py"]["seen"], (int, float)) + assert "a.py" in raw + assert set(raw["a.py"].keys()) == {"ast_hash", "semantic_hash"} + # ── #2106: sensitive-filter over-match (prose/source rescued, real secrets kept) ── diff --git a/tests/test_stable_manifest.py b/tests/test_stable_manifest.py new file mode 100644 index 000000000..403c3b19b --- /dev/null +++ b/tests/test_stable_manifest.py @@ -0,0 +1,308 @@ +"""Regression tests for Issue #3643: Manifest Byte Stability & Incremental State. + +Separates tracked, portable manifest.json from machine-local +cache/incremental-state.json. +""" +import json +import os +import time +from pathlib import Path + +import pytest + +from graphify import detect as det + + +def _assert_strict_manifest_schema(manifest_dict: dict) -> None: + """Verify every manifest row contains strictly {ast_hash, semantic_hash}.""" + assert isinstance(manifest_dict, dict), "manifest must be a dictionary" + for path_key, entry in manifest_dict.items(): + assert isinstance(path_key, str), f"key {path_key!r} must be a string path" + assert isinstance(entry, dict), f"entry for {path_key!r} must be a dict" + assert set(entry.keys()) == {"ast_hash", "semantic_hash"}, ( + f"Entry for {path_key!r} contains invalid keys: {set(entry.keys())}" + ) + assert isinstance(entry["ast_hash"], str) + assert isinstance(entry["semantic_hash"], str) + + +@pytest.fixture() +def sample_repo(tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + (repo / "a.py").write_text("def a(): return 1\n", encoding="utf-8") + (repo / "b.py").write_text("def b(): return 2\n", encoding="utf-8") + (repo / "docs").mkdir() + (repo / "docs" / "guide.md").write_text("# Guide\n\nSome documentation.\n", encoding="utf-8") + manifest_path = repo / "graphify-out" / "manifest.json" + state_path = repo / "graphify-out" / "cache" / "incremental-state.json" + return repo, manifest_path, state_path + + +def test_strict_manifest_schema(sample_repo): + """Scenario 8: Manifest schema must strictly contain ONLY ast_hash and semantic_hash.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + data = json.loads(manifest_path.read_text(encoding="utf-8")) + _assert_strict_manifest_schema(data) + assert "a.py" in data + assert "b.py" in data + assert "docs/guide.md" in data + + +def test_manifest_byte_stability_across_utime(sample_repo): + """Scenario 1: Changing file mtimes with os.utime leaves manifest.json bytes identical.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + bytes_before = manifest_path.read_bytes() + + # Move mtime back by 1 hour on all files + old_time = time.time() - 3600 + for f in repo.rglob("*.py"): + os.utime(f, (old_time, old_time)) + + det.save_manifest(files, str(manifest_path), root=repo) + bytes_after = manifest_path.read_bytes() + + assert bytes_before == bytes_after, "manifest.json bytes must remain identical when mtime moves" + + +def test_deterministic_ordering(sample_repo): + """Scenario 2: Manifest key serialization order is deterministic regardless of input order.""" + repo, manifest_path, _ = sample_repo + f_a = str(repo / "a.py") + f_b = str(repo / "b.py") + f_doc = str(repo / "docs" / "guide.md") + + # Order 1: a, b, doc + files_1 = {"code": [f_a, f_b], "document": [f_doc]} + det.save_manifest(files_1, str(manifest_path), root=repo) + bytes_1 = manifest_path.read_bytes() + + # Order 2: reverse order + files_2 = {"document": [f_doc], "code": [f_b, f_a]} + det.save_manifest(files_2, str(manifest_path), root=repo) + bytes_2 = manifest_path.read_bytes() + + assert bytes_1 == bytes_2, "manifest serialization must be strictly deterministic" + + +def test_fresh_checkout_cache_miss(sample_repo): + """Scenario 3: Fresh checkout / cache miss preserves manifest.json and recreates local state.""" + repo, manifest_path, state_path = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + manifest_bytes_orig = manifest_path.read_bytes() + + assert state_path.is_file(), "incremental-state.json must exist after save_manifest" + + # Simulate fresh checkout on another machine: remove untracked cache + state_path.unlink() + assert not state_path.exists() + + # Run incremental detection + inc = det.detect_incremental(repo, str(manifest_path)) + + # All files should be recognized as unchanged (0 new files) + queued = [f for flist in inc["new_files"].values() for f in flist] + assert queued == [], f"expected no queued files, got {queued}" + + # Incremental state must be recreated + assert state_path.is_file(), "cache/incremental-state.json must be recreated on cache miss" + state_data = json.loads(state_path.read_text(encoding="utf-8")) + assert "a.py" in state_data + assert "size" in state_data["a.py"] + assert "mtime_ns" in state_data["a.py"] + assert "indexed_at_ns" in state_data["a.py"] + + # Manifest must remain completely untouched + assert manifest_path.read_bytes() == manifest_bytes_orig + + +def test_real_modification(sample_repo): + """Scenario 4: Genuinely modifying a file updates only its content hash and manifest entry.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + manifest_before = json.loads(manifest_path.read_text(encoding="utf-8")) + + # Modify b.py + time.sleep(0.02) + (repo / "b.py").write_text("def b(): return 'changed'\n", encoding="utf-8") + + inc = det.detect_incremental(repo, str(manifest_path)) + queued = [f for flist in inc["new_files"].values() for f in flist] + assert len(queued) == 1 + assert queued[0].endswith("b.py") + + det.save_manifest(det.detect(repo)["files"], str(manifest_path), root=repo) + manifest_after = json.loads(manifest_path.read_text(encoding="utf-8")) + + _assert_strict_manifest_schema(manifest_after) + assert manifest_after["a.py"] == manifest_before["a.py"] + assert manifest_after["docs/guide.md"] == manifest_before["docs/guide.md"] + assert manifest_after["b.py"]["ast_hash"] != manifest_before["b.py"]["ast_hash"] + + +def test_mtime_only_change(sample_repo): + """Scenario 5: Changing mtime without changing content causes no false modification or manifest churn.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + manifest_bytes_orig = manifest_path.read_bytes() + + # Touch mtime of a.py to some future or past timestamp + a_py = repo / "a.py" + stat_before = a_py.stat() + new_mtime = stat_before.st_mtime - 100 + os.utime(a_py, (new_mtime, new_mtime)) + + inc = det.detect_incremental(repo, str(manifest_path)) + queued = [f for flist in inc["new_files"].values() for f in flist] + assert queued == [], f"mtime-only change must not queue file: {queued}" + + det.save_manifest(det.detect(repo)["files"], str(manifest_path), root=repo) + assert manifest_path.read_bytes() == manifest_bytes_orig, "manifest.json must not churn on mtime-only change" + + +def test_same_size_rapid_rewrite(sample_repo): + """Scenario 6: Same-size rapid rewrite within racily-clean window is detected reliably.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + target = repo / "a.py" + stat_before = target.stat() + + # Rewrite with exact same length + content_orig = target.read_text(encoding="utf-8") + content_new = "def a(): return 9\n" + assert len(content_orig) == len(content_new) + assert content_orig != content_new + + target.write_text(content_new, encoding="utf-8") + # Pin mtime to original stat so stat signature appears identical + 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 + + inc = det.detect_incremental(repo, str(manifest_path)) + queued = [f for flist in inc["new_files"].values() for f in flist] + assert len(queued) == 1, f"same-size rapid rewrite must be detected; queued={queued}" + assert queued[0].endswith("a.py") + + +def test_deletion(sample_repo): + """Scenario 7: Deletion of an indexed file is accurately reported.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + (repo / "b.py").unlink() + + inc = det.detect_incremental(repo, str(manifest_path)) + assert any(f.endswith("b.py") for f in inc["deleted_files"]) + assert inc["excluded_files"] == [] + + +def test_exclusion(sample_repo): + """Scenario 8: Newly excluded file is reported as excluded, not deleted.""" + repo, manifest_path, _ = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + # Pass extra_excludes to exclude b.py while it still exists on disk + inc = det.detect_incremental(repo, str(manifest_path), extra_excludes=["**/b.py"]) + assert any(f.endswith("b.py") for f in inc["excluded_files"]) + assert not any(f.endswith("b.py") for f in inc["deleted_files"]) + + +def test_legacy_manifest_migration(sample_repo): + """Scenario 9: Legacy manifest with mtime/seen upgrades cleanly to new schema.""" + repo, manifest_path, state_path = sample_repo + # Write a legacy manifest with mtime, seen, and single hash + legacy_data = { + "a.py": { + "mtime": 1600000000.0, + "seen": 1600000001.0, + "hash": det._md5_file(repo / "a.py"), + }, + "b.py": { + "mtime": 1600000000.0, + "seen": 1600000001.0, + "ast_hash": det._md5_file(repo / "b.py"), + "semantic_hash": det._md5_file(repo / "b.py"), + }, + } + manifest_path.parent.mkdir(parents=True, exist_ok=True) + manifest_path.write_text(json.dumps(legacy_data, indent=2), encoding="utf-8") + + # Run save_manifest to migrate + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + migrated = json.loads(manifest_path.read_text(encoding="utf-8")) + _assert_strict_manifest_schema(migrated) + + for row in migrated.values(): + assert "mtime" not in row + assert "seen" not in row + + # Incremental state must be created with valid fields + assert state_path.is_file() + state_data = json.loads(state_path.read_text(encoding="utf-8")) + for row in state_data.values(): + assert isinstance(row["size"], int) + assert isinstance(row["mtime_ns"], int) + assert isinstance(row["indexed_at_ns"], int) + + +def test_malformed_incremental_state_fallback(sample_repo): + """Scenario 10: Corrupted incremental-state.json falls back safely to hashing.""" + repo, manifest_path, state_path = sample_repo + files = det.detect(repo)["files"] + det.save_manifest(files, str(manifest_path), root=repo) + + # Corrupt the incremental state + state_path.write_text("{ this is corrupted invalid json }}}", encoding="utf-8") + + # detect_incremental should not crash, but safely fall back to hashing + inc = det.detect_incremental(repo, str(manifest_path)) + queued = [f for flist in inc["new_files"].values() for f in flist] + assert queued == [], f"expected unchanged files to remain unqueued despite corrupted state, got {queued}" + + +def test_save_manifest_no_op_preserves_both_files(sample_repo): + """Scenario 11: Double save_manifest without changes skips disk writes for both files.""" + repo, manifest_path, state_path = sample_repo + files = det.detect(repo)["files"] + + # First save + det.save_manifest(files, str(manifest_path), root=repo) + manifest_bytes_1 = manifest_path.read_bytes() + manifest_mtime_1 = manifest_path.stat().st_mtime_ns + + state_bytes_1 = state_path.read_bytes() + state_mtime_1 = state_path.stat().st_mtime_ns + + # Small sleep to ensure mtime moves if rewritten + time.sleep(0.02) + + # Second save (no-op) + det.save_manifest(files, str(manifest_path), root=repo) + manifest_bytes_2 = manifest_path.read_bytes() + manifest_mtime_2 = manifest_path.stat().st_mtime_ns + + state_bytes_2 = state_path.read_bytes() + state_mtime_2 = state_path.stat().st_mtime_ns + + assert manifest_bytes_1 == manifest_bytes_2, "manifest.json bytes must be identical" + assert manifest_mtime_1 == manifest_mtime_2, "manifest.json disk write must be skipped" + + assert state_bytes_1 == state_bytes_2, "incremental-state.json bytes must be identical" + assert state_mtime_1 == state_mtime_2, "incremental-state.json disk write must be skipped"