From bf06102d373dbadfb56a512bd2af2a037b5d81c3 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Fri, 20 Mar 2026 23:08:32 -0700 Subject: [PATCH] fix: memory access tracking and BM25 context caching (#138) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: memory access tracking and BM25 context caching Add touch_structured_memory/touch_structured_memories to storage protocol + SQLite/PostgreSQL backends. Bumps last_accessed and access_count on memory retrieval (BM25 injection + search results). 9 new storage tests. Cache the scored BM25 memory context string on ChatSession, invalidated on memory save/delete. Eliminates ~12 redundant storage queries + index rebuilds per session lifecycle. * fix: address review — deduplicate keys in touch facade, clarify contract Deduplicate keys in the memory.py facade before calling storage so each distinct memory is touched at most once. Update protocol docstring to clarify per-call increment semantics. Add deduplication unit test. * fix: replace unused-import test with real batch duplicate test Replace facade dedup test (which only tested Python set logic) with a real storage-level test that verifies duplicate keys each increment access_count. Fixes ruff F401 lint failure. --- tests/test_storage_sqlite.py | 126 ++++++++++++++++++++++++++ turnstone/core/memory.py | 21 +++++ turnstone/core/storage/_postgresql.py | 47 ++++++++++ turnstone/core/storage/_protocol.py | 13 +++ turnstone/core/storage/_sqlite.py | 47 ++++++++++ 5 files changed, 254 insertions(+) diff --git a/tests/test_storage_sqlite.py b/tests/test_storage_sqlite.py index e38409b4..7313962d 100644 --- a/tests/test_storage_sqlite.py +++ b/tests/test_storage_sqlite.py @@ -1,5 +1,9 @@ """Tests for the SQLite storage backend.""" +from __future__ import annotations + +from typing import Any + import pytest from turnstone.core.storage import init_storage, reset_storage @@ -289,6 +293,128 @@ class TestWorkstreams: assert rows[0][6] == "node-a" +# -- Structured memory touch --------------------------------------------------- + + +class TestTouchStructuredMemory: + @staticmethod + def _create_memory( + backend: Any, name: str = "m1", scope: str = "global", scope_id: str = "" + ) -> None: + import uuid + + backend.create_structured_memory( + memory_id=str(uuid.uuid4()), + name=name, + description="test desc", + mem_type="project", + scope=scope, + scope_id=scope_id, + content="test content", + ) + + def test_touch_bumps_access_count(self, backend): + self._create_memory(backend) + mem = backend.get_structured_memory_by_name("m1", "global", "") + assert mem is not None + assert int(mem["access_count"]) == 0 + + result = backend.touch_structured_memory("m1", "global", "") + assert result is True + + mem = backend.get_structured_memory_by_name("m1", "global", "") + assert int(mem["access_count"]) == 1 + + def test_touch_updates_last_accessed(self, backend): + self._create_memory(backend) + mem_before = backend.get_structured_memory_by_name("m1", "global", "") + original_accessed = mem_before["last_accessed"] + + backend.touch_structured_memory("m1", "global", "") + + mem_after = backend.get_structured_memory_by_name("m1", "global", "") + assert mem_after["last_accessed"] >= original_accessed + + def test_touch_nonexistent_returns_false(self, backend): + result = backend.touch_structured_memory("no_such", "global", "") + assert result is False + + def test_touch_increments_multiple_times(self, backend): + self._create_memory(backend) + for _ in range(3): + backend.touch_structured_memory("m1", "global", "") + + mem = backend.get_structured_memory_by_name("m1", "global", "") + assert int(mem["access_count"]) == 3 + + def test_touch_scoped_memory(self, backend): + self._create_memory(backend, name="ws_mem", scope="workstream", scope_id="ws-1") + backend.touch_structured_memory("ws_mem", "workstream", "ws-1") + + mem = backend.get_structured_memory_by_name("ws_mem", "workstream", "ws-1") + assert int(mem["access_count"]) == 1 + + # Different scope_id should not match + result = backend.touch_structured_memory("ws_mem", "workstream", "ws-other") + assert result is False + + def test_batch_touch_multiple(self, backend): + self._create_memory(backend, name="a") + self._create_memory(backend, name="b") + self._create_memory(backend, name="c") + + count = backend.touch_structured_memories( + [ + ("a", "global", ""), + ("b", "global", ""), + ("c", "global", ""), + ] + ) + assert count == 3 + + for name in ("a", "b", "c"): + mem = backend.get_structured_memory_by_name(name, "global", "") + assert int(mem["access_count"]) == 1 + + def test_batch_touch_empty_list(self, backend): + assert backend.touch_structured_memories([]) == 0 + + def test_batch_touch_partial_match(self, backend): + self._create_memory(backend, name="exists") + + count = backend.touch_structured_memories( + [ + ("exists", "global", ""), + ("missing", "global", ""), + ] + ) + assert count == 1 + + mem = backend.get_structured_memory_by_name("exists", "global", "") + assert int(mem["access_count"]) == 1 + + def test_touch_does_not_change_updated(self, backend): + self._create_memory(backend) + mem_before = backend.get_structured_memory_by_name("m1", "global", "") + + backend.touch_structured_memory("m1", "global", "") + + mem_after = backend.get_structured_memory_by_name("m1", "global", "") + # updated should stay the same (only last_accessed changes) + assert mem_after["updated"] == mem_before["updated"] + + def test_batch_touch_with_duplicates(self, backend): + """Duplicate keys in batch should each increment access_count once.""" + self._create_memory(backend, name="dup") + + # Two identical keys — storage gets called twice for the same row + count = backend.touch_structured_memories([("dup", "global", ""), ("dup", "global", "")]) + assert count == 2 + + mem = backend.get_structured_memory_by_name("dup", "global", "") + assert int(mem["access_count"]) == 2 + + # -- Lifecycle ----------------------------------------------------------------- diff --git a/turnstone/core/memory.py b/turnstone/core/memory.py index f3aa71f2..3a199fae 100644 --- a/turnstone/core/memory.py +++ b/turnstone/core/memory.py @@ -368,6 +368,27 @@ def search_structured_memories( return [] +def touch_structured_memories(keys: list[tuple[str, str, str]]) -> int: + """Batch-touch memories (bump last_accessed, increment access_count). + + Each key is ``(name, scope, scope_id)``. Duplicates are removed so each + distinct memory is touched at most once. Returns count of rows updated. + """ + if not keys: + return 0 + seen: set[tuple[str, str, str]] = set() + unique: list[tuple[str, str, str]] = [] + for k in keys: + if k not in seen: + seen.add(k) + unique.append(k) + try: + return get_storage().touch_structured_memories(unique) + except Exception: + log.warning("Failed to touch structured memories", exc_info=True) + return 0 + + def count_structured_memories(mem_type: str = "", scope: str = "", scope_id: str = "") -> int: """Count structured memories with optional type/scope filter.""" try: diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index 8fa92ae5..7755c2b9 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -2426,6 +2426,53 @@ class PostgreSQLBackend: ).fetchall() return [dict(r._mapping) for r in rows] + def touch_structured_memory(self, name: str, scope: str, scope_id: str) -> bool: + """Bump last_accessed and increment access_count for a single memory.""" + now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") + with self._engine.connect() as conn: + result = conn.execute( + sa.update(structured_memories) + .where( + sa.and_( + structured_memories.c.name == name, + structured_memories.c.scope == scope, + structured_memories.c.scope_id == scope_id, + ) + ) + .values( + last_accessed=now, + access_count=structured_memories.c.access_count + 1, + ) + ) + conn.commit() + return result.rowcount > 0 + + def touch_structured_memories(self, keys: list[tuple[str, str, str]]) -> int: + """Batch-touch multiple memories by (name, scope, scope_id).""" + if not keys: + return 0 + now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") + total = 0 + with self._engine.connect() as conn: + for name, scope, scope_id in keys: + result = conn.execute( + sa.update(structured_memories) + .where( + sa.and_( + structured_memories.c.name == name, + structured_memories.c.scope == scope, + structured_memories.c.scope_id == scope_id, + ) + ) + .values( + last_accessed=now, + access_count=structured_memories.c.access_count + 1, + ) + ) + total += result.rowcount + conn.commit() + return total + def count_structured_memories( self, mem_type: str = "", scope: str = "", scope_id: str = "" ) -> int: diff --git a/turnstone/core/storage/_protocol.py b/turnstone/core/storage/_protocol.py index 9c8e944a..a854fb54 100644 --- a/turnstone/core/storage/_protocol.py +++ b/turnstone/core/storage/_protocol.py @@ -131,6 +131,19 @@ class StorageBackend(Protocol): """Search structured memories by query. Returns matching memory dicts.""" ... + def touch_structured_memory(self, name: str, scope: str, scope_id: str) -> bool: + """Bump last_accessed and increment access_count. Returns True if found.""" + ... + + def touch_structured_memories(self, keys: list[tuple[str, str, str]]) -> int: + """Batch-touch multiple memories. + + Each key is ``(name, scope, scope_id)``. Callers should deduplicate + before calling; each key increments ``access_count`` once per call. + Returns count of rows found and updated. + """ + ... + def count_structured_memories( self, mem_type: str = "", scope: str = "", scope_id: str = "" ) -> int: diff --git a/turnstone/core/storage/_sqlite.py b/turnstone/core/storage/_sqlite.py index c20cc72c..d336ba86 100644 --- a/turnstone/core/storage/_sqlite.py +++ b/turnstone/core/storage/_sqlite.py @@ -2457,6 +2457,53 @@ class SQLiteBackend: ).fetchall() return [dict(r._mapping) for r in rows] + def touch_structured_memory(self, name: str, scope: str, scope_id: str) -> bool: + """Bump last_accessed and increment access_count for a single memory.""" + now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") + with self._engine.connect() as conn: + result = conn.execute( + sa.update(structured_memories) + .where( + sa.and_( + structured_memories.c.name == name, + structured_memories.c.scope == scope, + structured_memories.c.scope_id == scope_id, + ) + ) + .values( + last_accessed=now, + access_count=structured_memories.c.access_count + 1, + ) + ) + conn.commit() + return result.rowcount > 0 + + def touch_structured_memories(self, keys: list[tuple[str, str, str]]) -> int: + """Batch-touch multiple memories by (name, scope, scope_id).""" + if not keys: + return 0 + now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") + total = 0 + with self._engine.connect() as conn: + for name, scope, scope_id in keys: + result = conn.execute( + sa.update(structured_memories) + .where( + sa.and_( + structured_memories.c.name == name, + structured_memories.c.scope == scope, + structured_memories.c.scope_id == scope_id, + ) + ) + .values( + last_accessed=now, + access_count=structured_memories.c.access_count + 1, + ) + ) + total += result.rowcount + conn.commit() + return total + def count_structured_memories( self, mem_type: str = "", scope: str = "", scope_id: str = "" ) -> int: