From 9bd01a0203c8330ac13f8a89ddd737f3af1bc342 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 07:41:09 +0000 Subject: [PATCH] fix(mem0): reset() proves the namespace is writable, never sleeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading empty is necessary but not sufficient: a delete_all keeps reaping writes after it stops being visible to search, which is how 6/14 writes were lost. reset() now proves the namespace accepts writes again before returning — write a sentinel, poll for it, delete it. Only on the delete path; the skip path (namespace already empty, the common case) is untouched and still costs nothing. If the namespace never accepts a write within the timeout, raise rather than start a scenario whose first facts would be swallowed and scored against the provider. No blanket settle added. The sentinel retries rather than firing once. A write swallowed by the still-reaping delete_all never becomes retrievable however long it is polled, so a single-shot sentinel would abort runs a second attempt moments later would have saved — trading a phantom FAIL for a phantom abort. Each attempt gets 5s; the loop is bounded by the 30s converge timeout, so it cannot hang. Every deleting reset now records empty_after_s, empty_to_writable_s and sentinel_attempts, and logs them to stderr. That series is the number nobody has had — how long a delete_all keeps reaping after it stops being observable — collected free on every run, including runs that pass. Four tests added against a client whose delete_all arms a window of swallowed writes after the namespace already reads empty: the sentinel absorbs the window so the first real write survives, no sentinel is left behind, the gap is recorded only on the delete path, and reset raises rather than starting against an unwritable namespace. Invariant 9: this can convert phantom missing_current_fact FAILs into passes. Founder-instructed, recorded in HANDOFF. A genuine finding is untouched — the sentinel proves writability and is removed before any scenario value is written. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EqxND2pAdrMMubjBnp3VLR --- HANDOFF.md | 122 +++++++++++++++++++++++++++++++ src/memorycheck/adapters/mem0.py | 115 ++++++++++++++++++++++++++++- tests/test_mem0_adapter.py | 92 +++++++++++++++++++++++ 3 files changed, 327 insertions(+), 2 deletions(-) diff --git a/HANDOFF.md b/HANDOFF.md index 6157998..847413c 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -2563,3 +2563,125 @@ to stop every Mem0 run from this environment. External launch remains **HELD**. --- +## 2026-08-03 — reset() verifies writability instead of hoping (founder-instructed) + +**Invariant 9 note, up front.** This change can convert phantom +`missing_current_fact` FAILs into passes — the guarded direction. It was +**instructed by the founder**, which is the sign-off invariant 9 requires +before merge, and this entry is the record of it. The test of good faith +named in the invariant holds: a genuine finding survives untouched, because +the sentinel only proves the namespace accepts writes again and is deleted +before any scenario value is written. Nothing about retrieval, supersession, +scoping or deletion behaviour is affected. + +### 1. What shipped + +`reset()` no longer treats "the namespace reads empty" as proof the +`delete_all` finished. When it actually deleted, it now proves the namespace +is writable again before returning. + +- Write a sentinel under identifiers no scenario maps onto, poll for it, + delete it, return. +- **Only on the delete path.** The skip path — namespace already empty, the + overwhelmingly common case — is untouched and still costs nothing. +- If the namespace never accepts a write within the timeout, **raise**. A + scenario is not started on a store that would swallow its first facts. +- **No blanket settle was added**, per instruction. + +### 2. Findings + +**One deliberate refinement of the brief: the sentinel retries.** The brief +said write a sentinel, poll, raise if it never lands. Implemented as a bounded +retry loop instead, because a single-shot sentinel is wrong in exactly the +case it exists for: a write swallowed by the still-reaping delete_all **never +becomes retrievable however long it is polled**. Polling harder cannot help; +only writing again can. A single-shot sentinel would therefore abort runs that +a second attempt moments later would have saved — trading a phantom FAIL for a +phantom abort. + +Each attempt gets a short window (`_SENTINEL_ATTEMPT_TIMEOUT` = 5s); the loop +as a whole is bounded by `_CONVERGE_TIMEOUT` = 30s, so it cannot hang. The +attempt count is recorded and logged. Say if you want the strict single-shot +version instead — it is a two-line change. + +**The measurement series, gathered free on every run.** Each reset that +deletes appends to `adapter.reset_convergence`: + +``` +{"app_id": ..., "empty_after_s": ..., "empty_to_writable_s": ..., + "sentinel_attempts": ...} +``` + +and prints to stderr: + +``` + [reset] mc_ns: namespace read empty after 2.5s, writable 1.5s later (2 sentinel attempt(s)) +``` + +`empty_to_writable_s` is the number nobody has had: **how long a delete_all +keeps reaping after it stops being visible to search**. One value is an +anecdote; a run produces one per deleting reset, and the distribution is the +real characterisation. This makes arms (f)/(g) a confirmation rather than the +only source — and it collects data on runs that pass, not just ones that +break. + +**Four tests added, 67 → 71 passing.** They use a `ReapingClient` whose +`delete_all` arms a window of swallowed writes *after* the namespace already +reads empty — the measured 6/14 behaviour: + +- the sentinel absorbs the reaping window, so the scenario's first real write + survives (without it, that write vanishes and is scored as a missing fact); +- no sentinel is left behind; +- the empty-to-writable gap is recorded, and is **not** recorded on the skip + path; +- reset raises rather than starting a scenario against an unwritable + namespace. + +Building those tests caught a defect in my first fake: it armed the reaping +from construction rather than from `delete_all`, which is not the condition — +reaping is a consequence of the delete. Fixed before it could validate the +wrong thing. + +**Sentinel cleanup uses a per-key delete by id, never a `delete_all`**, so the +cleanup cannot restart the condition it just cleared. Arms (a)-(c) measured +per-key delete followed by an immediate write landing at 0s, three for three, +which is the evidence for that choice. + +**Residual risk, stated.** A sentinel that lands *after* cleanup has run would +persist. It sits under a tenant/user pair no scenario maps onto, so no +scenario query can retrieve it, and the next `reset()` of that namespace +removes it. It would, however, make the namespace read non-empty, which turns +a future skip-path reset into a delete-path one. Bounded and self-healing, but +worth knowing. + +### 3. Decisions + +- **Retry rather than single-shot**, for the reason above. Flagged as a + deviation. +- **Verify only when we deleted.** Verifying on the skip path would add a + write and several reads per scenario per seed for a condition that cannot + arise — meaningful SEARCH spend for nothing. +- **Raise rather than continue** when the namespace stays unwritable. The + alternative is scoring the provider on facts our own reset destroyed. + +### 4. FOR STRATEGY + +- **This does not close the `012`/`003` question**, and should not be read as + doing so. It removes one harness-side mechanism by which a run can be + killed or a provider mis-scored. Whether the aborts were that mechanism is + still open — but the next run will now *measure* the gap rather than guess. +- The `003` abort still needs promoting properly, with its results filename + and latency curve. +- Unchanged: pinning `mem0ai` 2.0.14 for the regen, exact pins for extras, + whether #6017 changes the publication plan, `get_all` pagination. + +### 5. Next + +1. Run the regen or (f)/(g) on the Mac — either now yields the + empty-to-writable series as a side effect. +2. Promote the `003` abort with its curve. +3. Founder ruling before any published text changes. + +External launch remains **HELD**. + +--- diff --git a/src/memorycheck/adapters/mem0.py b/src/memorycheck/adapters/mem0.py index 56f8995..7e35c39 100644 --- a/src/memorycheck/adapters/mem0.py +++ b/src/memorycheck/adapters/mem0.py @@ -31,7 +31,9 @@ import os import re +import sys import time +import uuid from ..ledger import Scope from .base import AdapterError, MemoryAdapter, QueryResult, poll_until @@ -48,6 +50,17 @@ _CONVERGE_TIMEOUT = 30.0 _CONVERGE_INTERVAL = 0.5 +# Key/scope fragment for the post-delete_all sentinel. Distinctive so it can +# never collide with a scenario value, and scoped to identifiers no scenario +# maps onto, so a stray sentinel could not be read by a scenario query. +_SENTINEL = "mc_reset_sentinel" + +# Per-attempt window for a sentinel write to become retrievable. Short, because +# a swallowed write never appears however long we wait — the useful move is to +# try again, not to wait longer. The retry loop overall stays bounded by +# _CONVERGE_TIMEOUT. +_SENTINEL_ATTEMPT_TIMEOUT = 5.0 + def _slug(text: str) -> str: """Collapse arbitrary text to a Mem0-safe identifier fragment.""" @@ -90,6 +103,11 @@ def __init__(self) -> None: ) from e self._client = MemoryClient() # reads MEM0_API_KEY from the environment self._namespace = "default" + #: One entry per reset that actually deleted: how long the namespace + #: took to read empty, and how much longer until it accepted a write. + #: Read this after a run — it is the empirical answer to "how long does + #: a delete_all keep reaping", accumulated at no extra cost. + self.reset_convergence: list[dict] = [] # --------------------------------------------------------------- scoping @@ -117,12 +135,20 @@ def reset(self, namespace: str) -> None: So: read the namespace first, and skip the delete entirely when it is empty (the overwhelmingly common case — namespaces are per scenario and seed). When there really is residue, delete it and wait for the - namespace to read empty before returning, so no write can race it. + namespace to read empty. + + Reading empty is necessary but **not sufficient**: the delete can keep + reaping writes after it has stopped being visible to search, which is + how the 6/14 above happened. So when we did delete, we prove the + namespace is writable again by writing a sentinel and polling for it, + then removing it. That is correct at any propagation latency, unlike a + fixed sleep — a sleep is either too short to be safe or too long to be + affordable, and it can never tell you which. """ self._namespace = namespace or "default" app_id = self._app_id() if not self._namespace_rows(app_id): - return # nothing to clear, so nothing to race + return # nothing to clear, so nothing to race, and nothing to prove _call("reset delete_all", self._client.delete_all, app_id=app_id) cleared, waited = poll_until( lambda: not self._namespace_rows(app_id), @@ -135,6 +161,91 @@ def reset(self, namespace: str) -> None: f"{waited:.1f}s — refusing to run, since leftovers would be " "scored against the provider" ) + self._confirm_namespace_writable(app_id, empty_after=waited) + + def _confirm_namespace_writable(self, app_id: str, empty_after: float) -> None: + """Prove a just-wiped namespace accepts writes again, then clean up. + + Called only when `reset()` actually issued a delete_all, so the common + skip path costs nothing. Writes one sentinel, polls for it, deletes it. + + Why a sentinel rather than a settle: the question is not "has enough + time passed" but "does a write survive", and only a write can answer + it. This is correct whether propagation takes 200ms or 20s, and it + reports the number instead of assuming one. + + The sentinel is removed with a per-key delete by id, not a delete_all, + so the cleanup cannot restart the condition it just cleared — arms + (a)-(c) measured per-key delete followed by an immediate write as + landing at 0s, three for three. + """ + sentinel_scope = Scope(tenant_id=f"{_SENTINEL}_t", user_id=f"{_SENTINEL}_u") + uid = self._user_id(sentinel_scope) + started = time.monotonic() + attempts = 0 + landed = False + + # Re-issue the sentinel rather than writing one and hoping. A write + # swallowed by the still-reaping delete_all never becomes retrievable + # no matter how long we poll it, so a single-shot sentinel would abort + # a run that a second attempt moments later would have saved. Each + # attempt gets a short window; the loop as a whole is bounded by + # _CONVERGE_TIMEOUT, so this cannot hang. + while not landed: + attempts += 1 + value = f"{_SENTINEL}-{uuid.uuid4().hex[:12]}" + _call( + "reset sentinel write", + self._client.add, + f"{_SENTINEL}: {value}", + user_id=uid, + app_id=app_id, + metadata={"key": _SENTINEL}, + infer=False, + ) + remaining = _CONVERGE_TIMEOUT - (time.monotonic() - started) + if remaining <= 0: + break + landed, _ = poll_until( + lambda v=value: any( + v in (m.get("memory") or "") + for m in self._scope_memories(sentinel_scope) + ), + timeout=min(_SENTINEL_ATTEMPT_TIMEOUT, remaining), + interval=_CONVERGE_INTERVAL, + ) + if not landed and time.monotonic() - started >= _CONVERGE_TIMEOUT: + break + + took = time.monotonic() - started + if not landed: + raise AdapterError( + f"mem0 reset: namespace {app_id!r} read empty after " + f"{empty_after:.1f}s but was still not accepting writes " + f"{took:.1f}s later — {attempts} sentinel write(s) were " + "acknowledged and never became retrievable, so the delete_all " + "is still reaping. Refusing to start a scenario, because its " + "first writes would be lost and scored against the provider " + "as missing facts." + ) + # Remove the sentinel so it cannot be seen by any scenario query. + for mem in self._scope_memories(sentinel_scope): + if (mem.get("metadata") or {}).get("key") == _SENTINEL and mem.get("id"): + _call("reset sentinel delete", self._client.delete, mem["id"]) + + # The series this produces IS the characterisation of the reset race: + # empty-to-writable, measured on every reset that actually deleted, + # gathered for free on every run. A single number is an anecdote; this + # accumulates the distribution. + self.reset_convergence.append( + {"app_id": app_id, "empty_after_s": round(empty_after, 2), + "empty_to_writable_s": round(took, 2), "sentinel_attempts": attempts} + ) + print( + f" [reset] {app_id}: namespace read empty after {empty_after:.1f}s, " + f"writable {took:.1f}s later ({attempts} sentinel attempt(s))", + file=sys.stderr, + ) def write( self, scope: Scope, key: str, value: str, ttl_steps: int | None = None diff --git a/tests/test_mem0_adapter.py b/tests/test_mem0_adapter.py index e50ae94..520dcbd 100644 --- a/tests/test_mem0_adapter.py +++ b/tests/test_mem0_adapter.py @@ -21,6 +21,7 @@ import pytest +from memorycheck.adapters.base import AdapterError from memorycheck.adapters.mem0 import Mem0Adapter from memorycheck.judge import load_judge from memorycheck.ledger import Scope @@ -95,6 +96,7 @@ def offline_adapter(monkeypatch): adapter = Mem0Adapter.__new__(Mem0Adapter) adapter._client = FakeMem0Client() adapter._namespace = "default" + adapter.reset_convergence = [] return adapter @@ -169,6 +171,96 @@ def test_reset_deletes_when_the_namespace_has_residue(offline_adapter): assert a._client.delete_all_calls == before + 1 +class ReapingClient(FakeMem0Client): + """A client whose delete_all keeps swallowing writes for `reap_writes` + more calls *after* the namespace already reads empty. + + This is the measured Mem0 behaviour the sentinel exists for: 6/14 writes + lost when a delete_all preceded them. Reading empty is not proof the + delete finished. + """ + + def __init__(self, reap_writes: int) -> None: + super().__init__() + self.reap_budget = reap_writes + self._remaining = 0 + self.swallowed = 0 + + def delete_all(self, app_id=None, **kw): + result = super().delete_all(app_id=app_id, **kw) + # The namespace now reads empty, but the delete keeps reaping — which + # is precisely the state that reading empty fails to detect. + self._remaining = self.reap_budget + return result + + def add(self, messages, **kw): + if self._remaining > 0: + self._remaining -= 1 + self.swallowed += 1 + return {"results": []} # acknowledged, never stored + return super().add(messages, **kw) + + +def _adapter_with(client): + a = Mem0Adapter.__new__(Mem0Adapter) + a._client = client + a._namespace = "default" + a.reset_convergence = [] + return a + + +def test_reset_proves_the_namespace_is_writable_again(monkeypatch): + """The sentinel absorbs the reaped writes so the scenario's first real + write survives — the whole point of the fix.""" + monkeypatch.setattr("memorycheck.adapters.mem0.time.sleep", lambda _: None) + a = _adapter_with(ReapingClient(reap_writes=2)) + a.reset("ns") + a.write(ALICE, "plan", "starter-legacy-2024") + a.reset("ns") # now has residue -> issues delete_all -> sentinel path + + # The reaping window was consumed by sentinel attempts, not by the + # scenario's fact. Without the sentinel this write would have vanished + # and been scored as a missing current fact against the provider. + a.write(ALICE, "plan", "growth-2026") + assert "growth-2026" in a.query(ALICE, "which plan?").answer + assert a._client.swallowed >= 1 + + +def test_reset_leaves_no_sentinel_behind(monkeypatch): + monkeypatch.setattr("memorycheck.adapters.mem0.time.sleep", lambda _: None) + a = _adapter_with(FakeMem0Client()) + a.reset("ns") + a.write(ALICE, "plan", "starter-legacy-2024") + a.reset("ns") + assert not [r for r in a._client.rows if "sentinel" in str(r.get("memory", ""))] + + +def test_reset_records_the_empty_to_writable_gap(monkeypatch): + monkeypatch.setattr("memorycheck.adapters.mem0.time.sleep", lambda _: None) + a = _adapter_with(FakeMem0Client()) + a.reset("ns") + a.write(ALICE, "plan", "starter-legacy-2024") + assert a.reset_convergence == [] # no delete yet, nothing to measure + a.reset("ns") + assert len(a.reset_convergence) == 1 + assert "empty_to_writable_s" in a.reset_convergence[0] + + +def test_reset_raises_rather_than_starting_an_unwritable_scenario(monkeypatch): + """If the sentinel never lands, reset must abort. Proceeding would lose + the scenario's first writes and score them against the provider.""" + monkeypatch.setattr("memorycheck.adapters.mem0.time.sleep", lambda _: None) + monkeypatch.setattr("memorycheck.adapters.mem0._CONVERGE_TIMEOUT", 0.05) + a = _adapter_with(ReapingClient(reap_writes=10_000)) # never stops reaping + a.reset("ns") + a._client.rows.append( + {"id": "residue", "memory": "x", "metadata": {}, "user_id": "u", + "app_id": a._app_id()} + ) + with pytest.raises(AdapterError, match="not accepting writes"): + a.reset("ns") + + # ------------------------------------------------------------------ live layer live = pytest.mark.skipif(