fix(memory): take the outside result in one query, not a fixed window

sentient/tools/builtin/memory_tool.py

@@ -74,23 +74,36 @@ async def _mark_if_outside(ctx: ToolContext, session_ids: list) -> None:
 
 async def _last_outside_result(ctx: ToolContext) -> str | None:
     """The latest successful tool result in this chat that brought in outside content (the review card shows it).
-    Errors and declined calls are skipped: they say nothing about where a fact came from."""
+    Errors and declined calls are skipped: they say nothing about where a fact came from.
+
+    SQL does only the cheap, index-backed part (session, tool role, the ``name IN`` allow-list and the newest-first
+    ordering); it carries no fixed window, so any number of failed calls after the real read never buries it (#270).
+    The success/failure decision stays in Python (``_failed``), preserving the exact truthiness semantics the old
+    code had: ``{"error": false}`` / ``{"error": 0}`` / ``{"error": {}}`` are successes, and ``{"declined": "yes"}``
+    is a decline. Rows are read in pages so a long chat never loads at once."""
     registry = ctx.extra.get("registry")
     if not ctx.session_id or registry is None or not ctx.untrusted:
         return None
-    try:
-        rows = await ctx.store.fetchall(
-            "SELECT name, content FROM messages WHERE session_id = ? AND role = 'tool'"
-            " ORDER BY created_at DESC, rowid DESC LIMIT 12",
-            (ctx.session_id,),
-        )
-    except Exception:
+    names = [t.name for t in registry.tools(include_hidden=True) if brings_untrusted(t)]
+    if not names:
         return None
-    for r in rows:
-        t = registry.get(r["name"] or "")
-        if t is not None and brings_untrusted(t) and not _failed(r["content"]):
-            return r["content"]
-    return None
+    placeholders = ",".join("?" * len(names))
+    sql = (
+        f"SELECT content FROM messages WHERE session_id = ? AND role = 'tool' AND name IN ({placeholders})"
+        " ORDER BY created_at DESC, rowid DESC LIMIT ? OFFSET ?"
+    )
+    offset = 0
+    while True:
+        try:
+            rows = await ctx.store.fetchall(sql, (ctx.session_id, *names, 50, offset))
+        except Exception:
+            return None
+        if not rows:
+            return None
+        for r in rows:
+            if not _failed(r["content"]):
+                return r["content"]
+        offset += len(rows)
 
 
 def _failed(content: str | None) -> bool:

tests/memory/test_review.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import functools
+import json
 from datetime import UTC, datetime, timedelta
 
 import pytest
@@ -177,6 +178,39 @@ async def test_the_review_snippet_is_the_content_read_not_a_later_error_or_decli
     assert "declined" not in snippet and "NOT DONE" not in snippet and "No email" not in snippet
 
 
+async def test_the_snippet_survives_more_than_a_page_of_failures_after_the_read(chat):
+    """#270: the real snippet must survive any number of failed calls after it.
+
+    One SQL query takes the newest non-failed outside result, so no fixed window buries it. A non-JSON
+    result mixed in among the failures must not raise and must not be mistaken for a failure."""
+    from sentient.tools.builtin.memory_tool import _last_outside_result
+
+    sid = await chat.store.create_session(channel="cli")
+    good = json.dumps({"from": "travel@example.com", "body": EMAIL})
+    await chat.store.add_message(sid, "tool", "Not JSON at all.", name="mail_read")  # predicate must not raise on it
+    await chat.store.add_message(sid, "tool", good, name="mail_read")
+    for _ in range(60):  # far past the old fixed 12-row window and any single page
+        await chat.store.add_message(sid, "tool", json.dumps(DECLINED), name="mail_read")
+    ctx = chat.agent.tool_context(sid, "cli")
+    ctx.untrusted = "Mail"
+    assert await _last_outside_result(ctx) == good
+
+
+async def test_a_falsey_error_field_is_still_a_successful_result(chat):
+    """A successful result may carry ``error`` with a falsey value (``false``/``0``/``{}``); it must still be
+    picked, and a decline whose ``declined`` reads as a non-empty string must still be skipped. Python truthiness
+    decides this, not SQLite's text coercion (which read ``false``/``{}`` as failures and ``"yes"`` as a success)."""
+    from sentient.tools.builtin.memory_tool import _last_outside_result
+
+    sid = await chat.store.create_session(channel="cli")
+    good = json.dumps({"from": "travel@example.com", "body": EMAIL, "error": False})  # falsey error == success
+    await chat.store.add_message(sid, "tool", good, name="mail_read")
+    await chat.store.add_message(sid, "tool", json.dumps({"declined": "yes"}), name="mail_read")  # newest, a decline
+    ctx = chat.agent.tool_context(sid, "cli")
+    ctx.untrusted = "Mail"
+    assert await _last_outside_result(ctx) == good
+
+
 async def test_work_nobody_asked_for_saves_memories_for_review(chat):
     for origin, label in (("proactive", "a proactive check"), ("heartbeat", "a background check")):
         ctx = chat.agent.tool_context(None, origin)