mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(memory): touch access metadata on composition and tool reads
The touch_structured_memories facade and both storage backends were implemented but had zero call sites, so access_count never moved and last_accessed never advanced past write time on any deployment. Wire two touch points: - proactive composition touches the injected top-k (post-rerank) set, deduped per turn since _init_system_messages recomposes many times within a single turn; - the memory tool's search and get reads touch their returned rows, counted per call. save/delete/list do not touch. Touches are best-effort through the facade, which already swallows storage errors, so a failed touch never breaks composition or a tool call.
This commit is contained in:
@@ -2913,6 +2913,181 @@ class TestMemoryCompositionDeferral:
|
||||
assert session._system_composed_with_context is False
|
||||
|
||||
|
||||
class TestMemoryAccessTouch:
|
||||
"""Access metadata (``access_count`` / ``last_accessed``) moves only when
|
||||
the model actually sees a memory: the injected top-k during composition,
|
||||
and explicit search/get reads via the memory tool. Save/list and the
|
||||
wider candidate pool must NOT bump the counter.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _access_count(name: str, scope: str = "global", scope_id: str = "") -> int:
|
||||
from turnstone.core.storage import get_storage
|
||||
|
||||
mem = get_storage().get_structured_memory_by_name(name, scope, scope_id)
|
||||
assert mem is not None, f"memory {name!r} not found"
|
||||
return int(mem["access_count"])
|
||||
|
||||
@staticmethod
|
||||
def _save(name: str, content: str) -> None:
|
||||
from turnstone.core.memory import save_structured_memory
|
||||
|
||||
save_structured_memory(name, content, scope="global")
|
||||
|
||||
@staticmethod
|
||||
def _empty_session() -> ChatSession:
|
||||
"""A session whose __init__ composed before any memory existed.
|
||||
|
||||
The constructor composes the system prefix once; building it before
|
||||
the memories are saved keeps that first (empty) compose from touching
|
||||
rows, so the tests observe only the turn-driven recompose below.
|
||||
"""
|
||||
return _make_session(ws_id="ws-1", user_id="user-1")
|
||||
|
||||
@staticmethod
|
||||
def _compose_turn(session: ChatSession, query: str) -> None:
|
||||
"""Drive one user turn's worth of composition.
|
||||
|
||||
Mirrors ``send``: a fresh user turn invalidates the per-turn memory
|
||||
caches, then the prefix recomposes against the new query.
|
||||
"""
|
||||
session._invalidate_memory_cache()
|
||||
session.messages.append(turn_from_dict({"role": "user", "content": query}))
|
||||
session._init_system_messages()
|
||||
|
||||
def test_composition_touches_injected_memories(self, tmp_db):
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
self._save("kafka_alerts", "kafka consumer lag alert thresholds")
|
||||
self._compose_turn(session, "how do I restart kafka")
|
||||
# Both query-matching memories were injected, so both got touched once.
|
||||
assert self._access_count("kafka_runbook") == 1
|
||||
assert self._access_count("kafka_alerts") == 1
|
||||
|
||||
def test_composition_skips_unmatched_candidates(self, tmp_db):
|
||||
"""The candidate pool is a superset of the injected set — a memory
|
||||
that loses BM25 ranking (no query overlap) must NOT be touched."""
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
self._save("garden_notes", "tomato watering schedule midsummer")
|
||||
self._compose_turn(session, "restart kafka broker pods status")
|
||||
# The matching memory was injected and touched.
|
||||
assert self._access_count("kafka_runbook") == 1
|
||||
# The non-matching one was a candidate but never injected.
|
||||
assert self._access_count("garden_notes") == 0
|
||||
# Sanity: it really was in the visible candidate pool.
|
||||
visible = {m["name"] for m in session._list_visible_memories()}
|
||||
assert "garden_notes" in visible
|
||||
|
||||
def test_composition_touches_each_memory_once_per_turn(self, tmp_db):
|
||||
"""``_init_system_messages`` runs many times within a turn (tool
|
||||
results, MCP refresh); the injected set must be touched at most once
|
||||
per memory between user turns, not once per recompose."""
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
self._compose_turn(session, "how do I restart kafka")
|
||||
# Several mid-turn recomposes (no new user turn between them).
|
||||
session._init_system_messages()
|
||||
session._init_system_messages()
|
||||
assert self._access_count("kafka_runbook") == 1
|
||||
# A genuinely new turn lets the same memory be counted again.
|
||||
self._compose_turn(session, "kafka again please")
|
||||
assert self._access_count("kafka_runbook") == 2
|
||||
|
||||
def test_composition_touches_exactly_the_injected_keys(self, tmp_db):
|
||||
"""Spy the touch boundary and assert the keys match the names the
|
||||
composer rendered into the ``<memories>`` block — exactly, not the
|
||||
candidate pool."""
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
self._save("garden_notes", "tomato watering schedule midsummer")
|
||||
session._invalidate_memory_cache()
|
||||
session.messages.append(
|
||||
turn_from_dict({"role": "user", "content": "restart kafka broker pods status"})
|
||||
)
|
||||
touched: list[tuple[str, str, str]] = []
|
||||
with patch(
|
||||
"turnstone.core.session.touch_structured_memories",
|
||||
side_effect=lambda keys: touched.extend(keys),
|
||||
):
|
||||
session._init_system_messages()
|
||||
joined = "\n".join(m["content"] for m in session.system_messages if m["role"] == "system")
|
||||
touched_names = {name for name, _, _ in touched}
|
||||
assert touched_names == {"kafka_runbook"}
|
||||
assert '<memory name="kafka_runbook"' in joined
|
||||
assert '<memory name="garden_notes"' not in joined
|
||||
|
||||
def test_composition_survives_touch_storage_error(self, tmp_db):
|
||||
"""A storage blow-up inside the touch must not break composition —
|
||||
the facade swallows it and the memory block still lands."""
|
||||
from turnstone.core.storage import get_storage
|
||||
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
session._invalidate_memory_cache()
|
||||
session.messages.append(
|
||||
turn_from_dict({"role": "user", "content": "how do I restart kafka"})
|
||||
)
|
||||
with patch.object(
|
||||
get_storage(),
|
||||
"touch_structured_memories",
|
||||
side_effect=RuntimeError("storage exploded"),
|
||||
):
|
||||
session._init_system_messages()
|
||||
joined = "\n".join(m["content"] for m in session.system_messages if m["role"] == "system")
|
||||
assert '<memory name="kafka_runbook"' in joined
|
||||
|
||||
def test_search_action_touches_returned_hits(self, tmp_db):
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
item = session._prepare_memory("call_1", {"action": "search", "query": "kafka"})
|
||||
assert "error" not in item
|
||||
session._exec_memory(item)
|
||||
assert self._access_count("kafka_runbook") == 1
|
||||
|
||||
def test_get_action_touches_fetched_memory(self, tmp_db):
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
item = session._prepare_memory(
|
||||
"call_1", {"action": "get", "name": "kafka_runbook", "scope": "global"}
|
||||
)
|
||||
assert "error" not in item
|
||||
session._exec_memory(item)
|
||||
assert self._access_count("kafka_runbook") == 1
|
||||
|
||||
def test_get_miss_touches_nothing(self, tmp_db):
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
item = session._prepare_memory(
|
||||
"call_1", {"action": "get", "name": "no_such_mem", "scope": "global"}
|
||||
)
|
||||
_, msg = session._exec_memory(item)
|
||||
assert "not found" in msg
|
||||
# The existing row must not be collaterally touched by a miss.
|
||||
assert self._access_count("kafka_runbook") == 0
|
||||
|
||||
def test_list_action_does_not_touch(self, tmp_db):
|
||||
session = self._empty_session()
|
||||
self._save("kafka_runbook", "restart the kafka broker pods")
|
||||
item = session._prepare_memory("call_1", {"action": "list"})
|
||||
session._exec_memory(item)
|
||||
assert self._access_count("kafka_runbook") == 0
|
||||
|
||||
def test_save_action_does_not_touch_access_count(self, tmp_db):
|
||||
"""The save action handler itself must not bump ``access_count`` —
|
||||
that counter is read traffic only. (The recompose a save triggers
|
||||
may surface the row via the composition path; that is exercised by
|
||||
the composition tests. Suppressed here to isolate the handler.)"""
|
||||
session = self._empty_session()
|
||||
item = session._prepare_memory(
|
||||
"call_1",
|
||||
{"action": "save", "name": "kafka_runbook", "content": "x", "scope": "global"},
|
||||
)
|
||||
with patch.object(session, "_init_system_messages"):
|
||||
session._exec_memory(item)
|
||||
assert self._access_count("kafka_runbook") == 0
|
||||
|
||||
|
||||
class TestMetacognitiveBuffers:
|
||||
"""Nudges drain through advisory channels, not the system message."""
|
||||
|
||||
|
||||
@@ -86,6 +86,7 @@ from turnstone.core.memory import (
|
||||
search_visible_structured_memories,
|
||||
set_message_attachments,
|
||||
set_workstream_alias,
|
||||
touch_structured_memories,
|
||||
update_workstream_title,
|
||||
)
|
||||
from turnstone.core.memory_relevance import (
|
||||
@@ -1031,6 +1032,10 @@ class ChatSession:
|
||||
# tool results) and the recent-context string is identical across
|
||||
# them. Invalidated on user-turn append and on memory write/delete.
|
||||
self._mem_search_cache: dict[tuple[str, str, int], list[dict[str, str]]] = {}
|
||||
# Per-turn dedup for composition touches: ``_init_system_messages`` runs
|
||||
# many times within a turn, so the injected set is touched at most once
|
||||
# per memory per turn. Cleared alongside the search cache.
|
||||
self._touched_memory_keys: set[tuple[str, str, str]] = set()
|
||||
self._ws_id = ws_id or uuid.uuid4().hex
|
||||
self._title_generated = False
|
||||
self._read_files: set[str] = set()
|
||||
@@ -2873,6 +2878,9 @@ class ChatSession:
|
||||
candidates=len(visible_mems),
|
||||
injected=len(relevant),
|
||||
)
|
||||
# Access metadata tracks what the model actually saw — touch the
|
||||
# injected top-k, not the candidate pool.
|
||||
self._touch_injected_memories(relevant)
|
||||
if relevant:
|
||||
dev_parts.append("")
|
||||
dev_parts.append(build_memory_context(relevant))
|
||||
@@ -7439,6 +7447,7 @@ class ChatSession:
|
||||
def _invalidate_memory_cache(self) -> None:
|
||||
"""Drop the per-turn search cache; call on user-turn append + memory writes."""
|
||||
self._mem_search_cache.clear()
|
||||
self._touched_memory_keys.clear()
|
||||
|
||||
def _select_memory_candidates(self, context: str) -> tuple[list[dict[str, str]], str]:
|
||||
"""Pick the candidate set fed into BM25 ranking.
|
||||
@@ -7476,6 +7485,38 @@ class ChatSession:
|
||||
return extra, "recency"
|
||||
return search_hits + extra, ("union" if extra else "search")
|
||||
|
||||
@staticmethod
|
||||
def _memory_keys(rows: list[dict[str, str]]) -> list[tuple[str, str, str]]:
|
||||
"""Build ``(name, scope, scope_id)`` touch keys from memory rows.
|
||||
|
||||
The storage read helpers return ``SELECT *`` rows, so all three
|
||||
columns are present.
|
||||
"""
|
||||
return [(r.get("name", ""), r.get("scope", ""), r.get("scope_id", "")) for r in rows]
|
||||
|
||||
def _touch_injected_memories(self, rows: list[dict[str, str]]) -> None:
|
||||
"""Touch the memories injected into the system prefix this turn.
|
||||
|
||||
``_init_system_messages`` recomposes many times per turn; gate on the
|
||||
per-turn touched-key set so each surfaced memory is counted at most
|
||||
once between user turns. Best-effort: the facade swallows storage
|
||||
errors, so a failed touch never breaks composition.
|
||||
"""
|
||||
fresh = [k for k in self._memory_keys(rows) if k not in self._touched_memory_keys]
|
||||
if not fresh:
|
||||
return
|
||||
self._touched_memory_keys.update(fresh)
|
||||
touch_structured_memories(fresh)
|
||||
|
||||
def _touch_read_memories(self, rows: list[dict[str, str]]) -> None:
|
||||
"""Touch memories returned by an explicit memory-tool read.
|
||||
|
||||
A search/get is a distinct user-driven access each time it runs, so
|
||||
these are counted unconditionally (not subject to the composition
|
||||
per-turn dedup). Best-effort via the facade.
|
||||
"""
|
||||
touch_structured_memories(self._memory_keys(rows))
|
||||
|
||||
def _check_metacognitive_nudge(self, user_message: str) -> tuple[str, str] | None:
|
||||
"""Check if a metacognitive nudge should fire for *user_message*.
|
||||
|
||||
@@ -11489,6 +11530,7 @@ class ChatSession:
|
||||
found_scope = scope
|
||||
break
|
||||
if mem:
|
||||
self._touch_read_memories([mem])
|
||||
content = mem.get("content", "")
|
||||
desc = mem.get("description", "")
|
||||
mem_type = mem.get("type", "")
|
||||
@@ -11566,6 +11608,7 @@ class ChatSession:
|
||||
result_count=len(rows),
|
||||
query=item["query"][:120],
|
||||
)
|
||||
self._touch_read_memories(rows)
|
||||
if rows:
|
||||
lines = []
|
||||
for m in rows:
|
||||
|
||||
Reference in New Issue
Block a user