fix(memory): do not fall back to raw entry when strip_think empties it
`append_history` previously used `strip_think(entry) or entry.rstrip()` as a safety net, so if the entire entry was a template-token leak (e.g. `<think>reasoning</think>` or `<channel|>` alone), the raw leaked text was still persisted to history — later re-introducing the very content `strip_think` was meant to scrub, via consolidation / replay. Persist the cleaned content directly. When cleanup empties a non-empty entry, log at debug and store an empty-content record (cursor continuity preserved). Adds 3 regression tests in test_memory_store.py covering: - Well-formed thinking blocks are stripped before persistence. - Pure-leak entries persist as empty, not as raw text. - Malformed prefix leaks (`<channel|>`) also persist as empty.
This commit is contained in:
+18
-2
@@ -221,10 +221,26 @@ class MemoryStore:
|
||||
# -- history.jsonl — append-only, JSONL format ---------------------------
|
||||
|
||||
def append_history(self, entry: str) -> int:
|
||||
"""Append *entry* to history.jsonl and return its auto-incrementing cursor."""
|
||||
"""Append *entry* to history.jsonl and return its auto-incrementing cursor.
|
||||
|
||||
Entries are passed through `strip_think` to drop template-level leaks
|
||||
(e.g. unclosed `<think` prefixes, `<channel|>` markers) before being
|
||||
persisted. If the cleaned content is empty but the raw entry wasn't,
|
||||
the record is persisted with an empty string rather than falling back
|
||||
to the raw leak — otherwise `strip_think`'s guarantees would be
|
||||
undone by history replay / consolidation downstream.
|
||||
"""
|
||||
cursor = self._next_cursor()
|
||||
ts = datetime.now().strftime("%Y-%m-%d %H:%M")
|
||||
record = {"cursor": cursor, "timestamp": ts, "content": strip_think(entry.rstrip()) or entry.rstrip()}
|
||||
raw = entry.rstrip()
|
||||
content = strip_think(raw)
|
||||
if raw and not content:
|
||||
logger.debug(
|
||||
"history entry {} stripped to empty (likely template leak); "
|
||||
"persisting empty content to avoid re-polluting context",
|
||||
cursor,
|
||||
)
|
||||
record = {"cursor": cursor, "timestamp": ts, "content": content}
|
||||
with open(self.history_file, "a", encoding="utf-8") as f:
|
||||
f.write(json.dumps(record, ensure_ascii=False) + "\n")
|
||||
self._cursor_file.write_text(str(cursor), encoding="utf-8")
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
"""Tests for the restructured MemoryStore — pure file I/O layer."""
|
||||
|
||||
from datetime import datetime
|
||||
import json
|
||||
from pathlib import Path
|
||||
from datetime import datetime
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -65,6 +64,34 @@ class TestHistoryWithCursor:
|
||||
cursor = store.append_history("event 3")
|
||||
assert cursor == 3
|
||||
|
||||
def test_append_history_strips_thinking_content(self, store):
|
||||
"""`strip_think` must run before persistence — well-formed thinking
|
||||
blocks shouldn't land in history."""
|
||||
cursor = store.append_history("<think>reasoning</think>final answer")
|
||||
content = store.read_file(store.history_file)
|
||||
data = json.loads(content)
|
||||
assert data["cursor"] == cursor
|
||||
assert data["content"] == "final answer"
|
||||
|
||||
def test_append_history_drops_pure_leak_content(self, store):
|
||||
"""Regression: entries that strip down to empty (pure template-token
|
||||
leak) must NOT fall back to the raw leak. Persisting the raw text
|
||||
would re-pollute context via consolidation / replay, undoing the
|
||||
protection `strip_think` provides."""
|
||||
cursor = store.append_history("<think>nothing user-facing</think>")
|
||||
content = store.read_file(store.history_file)
|
||||
data = json.loads(content)
|
||||
assert data["cursor"] == cursor
|
||||
assert data["content"] == ""
|
||||
|
||||
def test_append_history_drops_malformed_leak_prefix(self, store):
|
||||
"""Channel-marker / malformed opening leaks should not survive."""
|
||||
cursor = store.append_history("<channel|>")
|
||||
content = store.read_file(store.history_file)
|
||||
data = json.loads(content)
|
||||
assert data["cursor"] == cursor
|
||||
assert data["content"] == ""
|
||||
|
||||
def test_read_unprocessed_history(self, store):
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
@@ -134,7 +161,8 @@ class TestLegacyHistoryMigration:
|
||||
"""JSONL entries with cursor=1 are correctly parsed and returned."""
|
||||
store.history_file.write_text(
|
||||
'{"cursor": 1, "timestamp": "2026-03-30 14:30", "content": "Old event"}\n',
|
||||
encoding="utf-8")
|
||||
encoding="utf-8",
|
||||
)
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["cursor"] == 1
|
||||
@@ -218,8 +246,7 @@ class TestLegacyHistoryMigration:
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_content = (
|
||||
"[2026-03-25–2026-04-02] Multi-day summary.\n"
|
||||
"[2026-03-26/27] Cross-day summary.\n"
|
||||
"[2026-03-25–2026-04-02] Multi-day summary.\n[2026-03-26/27] Cross-day summary.\n"
|
||||
)
|
||||
legacy_file.write_text(legacy_content, encoding="utf-8")
|
||||
|
||||
@@ -277,9 +304,7 @@ class TestLegacyHistoryMigration:
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_file.write_bytes(
|
||||
b"[2026-04-01 10:00] Broken \xff data still needs migration.\n\n"
|
||||
)
|
||||
legacy_file.write_bytes(b"[2026-04-01 10:00] Broken \xff data still needs migration.\n\n")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user