diff --git a/graphify/export.py b/graphify/export.py index 43c9b9adb..0b1136de8 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -21,6 +21,7 @@ from graphify.paths import ( stem_filename_budget, write_json_atomic, write_text_atomic, + write_text_atomic_if_changed, ) from graphify.exporters.graphdb import push_to_falkordb, push_to_neo4j # noqa: E402,F401 @@ -725,13 +726,20 @@ def to_obsidian( def _owned_write(rel_name: str, content: str) -> bool: """Write a graphify-owned file, refusing to overwrite a pre-existing file - graphify didn't create. Returns True if written.""" + graphify didn't create. Returns True if the note is owned/current (whether + or not it was physically written). + + The disk write is skipped when the content is byte-identical to what is + already there (#3060) — an export re-runs on every graph.json change, and + rewriting an unchanged note churns disk and fires inotify / re-index / + sync for nothing. The note is still recorded in ``_written`` so it stays + owned and is neither pruned as stale nor dropped from the manifest.""" target = out / rel_name if target.exists() and rel_name not in _owned: _skipped.append(rel_name) return False target.parent.mkdir(parents=True, exist_ok=True) - write_text_atomic(target, content) + write_text_atomic_if_changed(target, content) _written.append(rel_name) return True diff --git a/graphify/paths.py b/graphify/paths.py index c4286583f..8cfca2e65 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -157,6 +157,29 @@ def write_text_atomic(path: "str | Path", text: str) -> None: _atomic_replace(path, lambda f: f.write(text)) +def write_text_atomic_if_changed(path: "str | Path", text: str) -> bool: + """Atomically write ``text`` only if it differs from what is already on disk; + return ``True`` if a write happened, ``False`` if the file was left untouched. + + Exporters regenerate their whole page set on every ``graph.json`` change and + used to rewrite every file unconditionally — tens of thousands of identical + pages per run, which also fires inotify / re-index / sync on unchanged + content (#3060). Skipping the atomic replace when nothing changed avoids the + rename entirely, so mtime and inode are preserved. + + The comparison is on DECODED text, never bytes: :func:`write_text_atomic` + writes in text mode and (on Windows) translates ``\\n`` to ``\\r\\n`` while + :func:`Path.read_text` translates it back, so a byte compare would report + every file as changed. A missing or non-UTF-8 destination counts as changed.""" + try: + if Path(os.path.realpath(str(path))).read_text(encoding="utf-8") == text: + return False + except (OSError, UnicodeDecodeError): + pass + write_text_atomic(path, text) + return True + + def write_json_atomic(path: "str | Path", obj, *, indent: "int | None" = None, ensure_ascii: bool = True) -> None: """Atomically write ``obj`` as JSON to ``path``, streaming the encode into the temp file rather than materializing the whole string first (matters for very diff --git a/graphify/wiki.py b/graphify/wiki.py index cc53c880c..b08b22eb6 100644 --- a/graphify/wiki.py +++ b/graphify/wiki.py @@ -7,7 +7,7 @@ import re import networkx as nx from graphify.build import edge_data -from graphify.paths import stem_filename_budget +from graphify.paths import stem_filename_budget, write_text_atomic_if_changed # Room _unique_slug needs for the collision suffix ("_2" … "_9999") it appends # after _safe_filename has already capped the slug. The suffix is technically @@ -319,14 +319,15 @@ def to_wiki( "Re-run `graphify extract .` to regenerate .graphify_analysis.json." ) - # Clear stale .md files from previous runs to prevent orphan accumulation. - # Community labels are LLM-generated (per skill.md Step 5) and non-deterministic - # across runs — the same conceptual community may be named differently each time - # (e.g. "AutoAgent Skills" → "AutoAgent Methodology"), leaving the previous file - # as an orphan. Since to_wiki() owns wiki/ entirely (always writes the full set), - # it can safely clear .md files at the start of each call. - for old_article in out.glob("*.md"): - old_article.unlink() + # Orphans (from LLM-generated community labels that drift across runs, e.g. + # "AutoAgent Skills" → "AutoAgent Methodology") are swept at the END of the + # run, once the produced set is known — not by an up-front unlink of every + # *.md. Unlinking first forced a full rewrite of the identical page set on + # every export (an export re-runs on any graph.json change: tens of thousands + # of unchanged pages, disk + inotify + re-index churn, #3060) and opened a + # window where a reader saw an empty wiki/ mid-export. to_wiki() still owns + # wiki/ entirely, so anything it did not produce this run is still removed. + produced: set[str] = set() labels = community_labels or {cid: f"Community {cid}" for cid in communities} cohesion = cohesion or {} @@ -388,18 +389,29 @@ def to_wiki( for cid, nodes in communities.items(): label = labels.get(cid, f"Community {cid}") article = _community_article(G, cid, nodes, label, labels, cohesion.get(cid), node_community, resolver) - (out / f"{community_slugs[cid]}.md").write_text(article, encoding="utf-8") + fname = f"{community_slugs[cid]}.md" + write_text_atomic_if_changed(out / fname, article) + produced.add(fname) count += 1 for nid, slug in god_articles: article = _god_node_article(G, nid, labels, node_community, resolver) - (out / f"{slug}.md").write_text(article, encoding="utf-8") + fname = f"{slug}.md" + write_text_atomic_if_changed(out / fname, article) + produced.add(fname) count += 1 # Index - (out / "index.md").write_text( + write_text_atomic_if_changed( + out / "index.md", _index_md(communities, labels, god_nodes_data, G.number_of_nodes(), G.number_of_edges(), resolver), - encoding="utf-8", ) + produced.add("index.md") + + # Sweep orphans: remove only the *.md this run did not produce, so a relabelled + # community's stale page is still cleaned up without rewriting the survivors. + for old_article in out.glob("*.md"): + if old_article.name not in produced: + old_article.unlink() return count diff --git a/tests/test_export_idempotent_writes.py b/tests/test_export_idempotent_writes.py new file mode 100644 index 000000000..8cc7e072e --- /dev/null +++ b/tests/test_export_idempotent_writes.py @@ -0,0 +1,108 @@ +"""Exporters must not rewrite a page whose content did not change (#3060). + +An export re-runs on every graph.json change, and the wiki/Obsidian writers used +to regenerate and rewrite the entire page set unconditionally — tens of thousands +of identical files per run, each rewrite also firing inotify / re-index / sync. +""" +from __future__ import annotations + +import os +from pathlib import Path + +import networkx as nx + +from graphify.paths import write_text_atomic_if_changed +from graphify.export import to_obsidian +from graphify.wiki import to_wiki + + +def _mtime(p: Path) -> int: + return os.stat(p).st_mtime_ns + + +def test_write_text_atomic_if_changed_skips_identical(tmp_path): + target = tmp_path / "note.md" + + # Missing file -> writes, reports True. + assert write_text_atomic_if_changed(target, "hello\nworld\n") is True + assert target.read_text(encoding="utf-8") == "hello\nworld\n" + before = _mtime(target) + + # Identical content -> no write, reports False, mtime untouched. + assert write_text_atomic_if_changed(target, "hello\nworld\n") is False + assert _mtime(target) == before + + # Different content -> writes, reports True. + assert write_text_atomic_if_changed(target, "hello\nworld!\n") is True + assert target.read_text(encoding="utf-8") == "hello\nworld!\n" + + +def _two_node_graph(): + G = nx.Graph() + G.add_node("n1", label="Database", community=0, source_file="app/db.py", type="code") + G.add_node("n2", label="Server", community=0, source_file="app/srv.py", type="code") + G.add_edge("n1", "n2") + return G, {0: ["n1", "n2"]} + + +def test_to_obsidian_rerun_does_not_rewrite_unchanged_notes(tmp_path): + """A second identical export leaves each note's bytes and mtime untouched, + yet the note stays owned (not pruned).""" + G, communities = _two_node_graph() + out = tmp_path / "obsidian" + to_obsidian(G, communities, str(out), community_labels={0: "Backend"}) + + note = out / "Database.md" + assert note.exists() + before = _mtime(note) + + to_obsidian(G, communities, str(out), community_labels={0: "Backend"}) + + assert note.exists(), "an unchanged, owned note must not be pruned" + assert _mtime(note) == before, "an unchanged note must not be rewritten (#3060)" + + +def test_to_obsidian_rerun_still_rewrites_changed_notes(tmp_path): + """Skipping identical writes must not skip real changes: giving Database a new + neighbor changes Database.md's body (same filename), so it is rewritten.""" + G, communities = _two_node_graph() + out = tmp_path / "obsidian" + to_obsidian(G, communities, str(out), community_labels={0: "Backend"}) + note = out / "Database.md" + before_text = note.read_text(encoding="utf-8") + + G.add_node("n3", label="Cache", community=0, source_file="app/cache.py", type="code") + G.add_edge("n1", "n3") + to_obsidian(G, {0: ["n1", "n2", "n3"]}, str(out), community_labels={0: "Backend"}) + + assert note.read_text(encoding="utf-8") != before_text, "a changed note must be rewritten" + + +def test_to_wiki_rerun_does_not_rewrite_unchanged_articles(tmp_path): + G, communities = _two_node_graph() + out = tmp_path / "wiki" + to_wiki(G, communities, str(out), community_labels={0: "Backend"}) + + index = out / "index.md" + community = out / "Backend.md" + assert index.exists() and community.exists() + i_before, c_before = _mtime(index), _mtime(community) + + to_wiki(G, communities, str(out), community_labels={0: "Backend"}) + + assert index.exists() and community.exists() + assert _mtime(index) == i_before, "unchanged index must not be rewritten (#3060)" + assert _mtime(community) == c_before, "unchanged article must not be rewritten (#3060)" + + +def test_to_wiki_rerun_sweeps_orphaned_relabelled_article(tmp_path): + """The end-of-run orphan sweep still removes a page the run did not produce + (a community relabelled to a new slug).""" + G, communities = _two_node_graph() + out = tmp_path / "wiki" + to_wiki(G, communities, str(out), community_labels={0: "Backend"}) + assert (out / "Backend.md").exists() + + to_wiki(G, communities, str(out), community_labels={0: "Infrastructure"}) + assert (out / "Infrastructure.md").exists() + assert not (out / "Backend.md").exists(), "the relabelled community's stale page must be swept"