From b6bed709ca5618987aed53802a197df0cbc5a4cb Mon Sep 17 00:00:00 2001 From: Artexis10 Date: Mon, 27 Jul 2026 17:00:06 +0300 Subject: [PATCH] fix(graph): stop a single write rebuilding the whole vault graph upsert_after_write -> refresh_paths takes the vault mutation boundary, and whenever the graph sidecar was missing or schema-invalid it escalated to a full-vault _rebuild_all_locked() inside that boundary. CI measured the hold at 39s over 2,000 pages and 172s over 8,000: every other vault mutation blocked for the duration, and the writing client free to time out on a write that then landed anyway. A missing sidecar is drift, not a write's problem to solve. graph_drift already reports exactly this condition and reconcile already owns rebuild_all for it, so the write defers instead. Availability is screened before the hold, so an unbuildable graph does not even contend for the boundary. Three of the six new tests fail against the previous code and pass against this one; the rest pin that the healing path still builds the sidecar and clears the drift. --- src/exomem/epistemic_graph.py | 22 +++++- tests/test_graph_write_boundary.py | 117 +++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tests/test_graph_write_boundary.py diff --git a/src/exomem/epistemic_graph.py b/src/exomem/epistemic_graph.py index 1fff52de..30a21100 100644 --- a/src/exomem/epistemic_graph.py +++ b/src/exomem/epistemic_graph.py @@ -414,16 +414,36 @@ def _mark_available(self) -> None: conn.close() def refresh_paths(self, paths: list[Path]) -> dict[str, int]: + """Index just the written paths. Never escalates to a full rebuild. + + This runs on the write path (`upsert_after_write`), inside the vault + mutation boundary. A missing or schema-invalid sidecar used to escalate + to `_rebuild_all_locked()` here, so one ordinary write rebuilt the whole + vault graph while holding the boundary — measured at 39s over 2,000 + pages and 172s over 8,000, with every other mutation blocked throughout + and the writer's own client free to time out on a write that then + succeeded. + + A missing sidecar is drift, not a write's problem to solve. `graph_drift` + reports exactly this condition, and `reconcile` owns `rebuild_all` for + it. Checked before acquiring the boundary so an unbuildable graph does + not even contend for it. + """ if not graph_enabled(): return {"indexed_files": 0, "nodes": 0, "edges": 0, "disabled": 1} + if not self.available(): + return {"indexed_files": 0, "nodes": 0, "edges": 0, "deferred": 1} with self._mutation_coordinator.hold( operation="epistemic_graph_refresh_paths", holder_kind="graph" ): return self._refresh_paths_locked(paths) def _refresh_paths_locked(self, paths: list[Path]) -> dict[str, int]: + # Defensive: `refresh_paths` already screened this. The sidecar can still + # vanish between that check and the boundary, and the write hook treats + # graph maintenance as best-effort, so defer rather than rebuild. if not self.available(): - return self._rebuild_all_locked() + return {"indexed_files": 0, "nodes": 0, "edges": 0, "deferred": 1} resolver = find_module.writer_resolver_snapshot(self.vault_root) conn = self._connect() indexed = 0 diff --git a/tests/test_graph_write_boundary.py b/tests/test_graph_write_boundary.py new file mode 100644 index 00000000..e4beec78 --- /dev/null +++ b/tests/test_graph_write_boundary.py @@ -0,0 +1,117 @@ +"""A write must never rebuild the whole graph inside the mutation boundary. + +Measured on CI before this fix: `upsert_after_write` -> `refresh_paths` took the +vault mutation boundary and, whenever the graph sidecar was missing or +schema-invalid, escalated to a full-vault `_rebuild_all_locked()` inside it -- +`hold_ms=39092` over 2,000 pages and `hold_ms=172205` over 8,000. Every other +vault mutation was blocked for that whole window, and the writer's own client +could time out on a write that then landed anyway. + +The healing path already exists elsewhere: `graph_drift` reports a missing or +schema-mismatched sidecar, and `reconcile` calls `rebuild_all` for it. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from exomem import epistemic_graph +from exomem.kbdir import kb_dirname + + +@pytest.fixture +def vault(tmp_path: Path) -> Path: + root = tmp_path / "vault" + (root / kb_dirname() / "Notes").mkdir(parents=True) + (root / kb_dirname() / "Notes" / "one.md").write_text( + "---\ntitle: One\ntype: note\n---\n\n# One\n\nBody.\n", encoding="utf-8" + ) + return root + + +def test_a_write_does_not_rebuild_when_the_sidecar_is_missing(vault, monkeypatch) -> None: + """The regression: one write must not rebuild the entire vault graph.""" + + index = epistemic_graph.EpistemicGraphIndex(vault) + assert not index.available(), "precondition: no sidecar yet" + + rebuilds: list[int] = [] + monkeypatch.setattr( + epistemic_graph.EpistemicGraphIndex, + "_rebuild_all_locked", + lambda self: rebuilds.append(1) or {}, + ) + + result = index.refresh_paths([vault / kb_dirname() / "Notes" / "one.md"]) + + assert rebuilds == [], "a write escalated to a full-vault rebuild" + assert result["deferred"] == 1 + assert result["indexed_files"] == 0 + + +def test_an_unavailable_graph_does_not_even_take_the_boundary(vault, monkeypatch) -> None: + """Screened before the hold, so an unbuildable graph cannot block writers.""" + + index = epistemic_graph.EpistemicGraphIndex(vault) + holds: list[str] = [] + real_hold = index._mutation_coordinator.hold + + def tracking_hold(*args, **kwargs): + holds.append(kwargs.get("operation", "unattributed")) + return real_hold(*args, **kwargs) + + monkeypatch.setattr(index._mutation_coordinator, "hold", tracking_hold) + index.refresh_paths([vault / kb_dirname() / "Notes" / "one.md"]) + + assert holds == [], "an unavailable graph must not contend for the boundary" + + +def test_the_write_hook_stays_bounded_without_a_sidecar(vault, monkeypatch) -> None: + """`upsert_after_write` is the real caller; it must not rebuild either.""" + + rebuilds: list[int] = [] + monkeypatch.setattr( + epistemic_graph.EpistemicGraphIndex, + "_rebuild_all_locked", + lambda self: rebuilds.append(1) or {}, + ) + + epistemic_graph.upsert_after_write(vault, [vault / kb_dirname() / "Notes" / "one.md"]) + + assert rebuilds == [] + + +def test_drift_still_reports_the_missing_sidecar(vault) -> None: + """The deferred work must remain visible, or the graph silently never builds.""" + + drift = epistemic_graph.graph_drift(vault) + + assert drift, "a missing sidecar must register as drift for reconcile to heal" + assert any("sidecar missing" in entry["reason"] for entry in drift) + + +def test_reconcile_still_owns_the_full_rebuild(vault) -> None: + """Deferring from the write path is only safe because this path exists.""" + + index = epistemic_graph.EpistemicGraphIndex(vault) + index.rebuild_all() + + assert index.available(), "rebuild_all must still build a usable sidecar" + assert not epistemic_graph.graph_drift(vault), "and must clear the drift it heals" + + +def test_a_healthy_graph_still_indexes_the_written_path(vault) -> None: + """Deferring must not disable ordinary incremental indexing.""" + + index = epistemic_graph.EpistemicGraphIndex(vault) + index.rebuild_all() + + (vault / kb_dirname() / "Notes" / "two.md").write_text( + "---\ntitle: Two\ntype: note\n---\n\n# Two\n\nBody.\n", encoding="utf-8" + ) + result = index.refresh_paths([vault / kb_dirname() / "Notes" / "two.md"]) + + assert "deferred" not in result + assert result["indexed_files"] == 1