mirror of
https://github.com/MODSetter/SurfSense.git
synced 2026-05-25 19:15:18 +02:00
feat: refine private and team memory protocols
This commit is contained in:
Anish Sarkar
parent
ceedd02353
commit
5247dc7097
16 changed files with 232 additions and 264 deletions
|
|
@ -2,28 +2,12 @@
|
|||
|
||||
import pytest
|
||||
|
||||
from app.agents.new_chat.tools.update_memory import _save_memory
|
||||
from app.services.memory import MemoryScope, save_memory
|
||||
from app.utils.content_utils import extract_text_content
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
class _Recorder:
|
||||
def __init__(self) -> None:
|
||||
self.applied_content: str | None = None
|
||||
self.commit_calls = 0
|
||||
self.rollback_calls = 0
|
||||
|
||||
def apply(self, content: str) -> None:
|
||||
self.applied_content = content
|
||||
|
||||
async def commit(self) -> None:
|
||||
self.commit_calls += 1
|
||||
|
||||
async def rollback(self) -> None:
|
||||
self.rollback_calls += 1
|
||||
|
||||
|
||||
def test_extract_text_content_keeps_no_update_bare_string_from_content_blocks() -> None:
|
||||
content = [
|
||||
{"type": "thinking", "thinking": "No"},
|
||||
|
|
@ -69,21 +53,12 @@ def test_extract_text_content_preserves_plain_string_responses() -> None:
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_memory_rejects_non_string_payload_before_commit() -> None:
|
||||
recorder = _Recorder()
|
||||
|
||||
result = await _save_memory(
|
||||
updated_memory=["NO_UPDATE"], # type: ignore[arg-type]
|
||||
old_memory=None,
|
||||
llm=None,
|
||||
apply_fn=recorder.apply,
|
||||
commit_fn=recorder.commit,
|
||||
rollback_fn=recorder.rollback,
|
||||
label="memory",
|
||||
scope="user",
|
||||
result = await save_memory(
|
||||
scope=MemoryScope.USER,
|
||||
target_id="00000000-0000-0000-0000-000000000000",
|
||||
content=["NO_UPDATE"], # type: ignore[arg-type]
|
||||
session=None, # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
assert result["status"] == "error"
|
||||
assert "must be a string" in result["message"]
|
||||
assert recorder.applied_content is None
|
||||
assert recorder.commit_calls == 0
|
||||
assert recorder.rollback_calls == 0
|
||||
assert result.status == "error"
|
||||
assert "must be a string" in result.message
|
||||
|
|
|
|||
|
|
@ -1,24 +1,24 @@
|
|||
"""Unit tests for memory scope validation and bullet format validation."""
|
||||
"""Unit tests for heading-based memory validation."""
|
||||
|
||||
import pytest
|
||||
|
||||
from app.agents.new_chat.tools.update_memory import (
|
||||
_save_memory,
|
||||
_validate_bullet_format,
|
||||
_validate_memory_scope,
|
||||
from app.services.memory import MemoryScope, save_memory
|
||||
from app.services.memory.validation import (
|
||||
validate_bullet_format,
|
||||
validate_memory_scope,
|
||||
)
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
class _Recorder:
|
||||
class _FakeSession:
|
||||
def __init__(self) -> None:
|
||||
self.applied_content: str | None = None
|
||||
self.added = []
|
||||
self.commit_calls = 0
|
||||
self.rollback_calls = 0
|
||||
|
||||
def apply(self, content: str) -> None:
|
||||
self.applied_content = content
|
||||
def add(self, obj) -> None:
|
||||
self.added.append(obj)
|
||||
|
||||
async def commit(self) -> None:
|
||||
self.commit_calls += 1
|
||||
|
|
@ -27,172 +27,125 @@ class _Recorder:
|
|||
self.rollback_calls += 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_memory_scope — marker-based
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_validate_memory_scope_rejects_pref_marker_in_team_scope() -> None:
|
||||
content = "- (2026-04-10) [pref] Prefers dark mode\n"
|
||||
result = _validate_memory_scope(content, "team")
|
||||
def test_validate_memory_scope_rejects_new_personal_heading_in_team() -> None:
|
||||
content = "## Preferences\n- 2026-04-10: Prefers dark mode\n"
|
||||
result, _warnings = validate_memory_scope(content, "team")
|
||||
assert result is not None
|
||||
assert result["status"] == "error"
|
||||
assert "[pref]" in result["message"]
|
||||
assert "preferences" in result["message"]
|
||||
|
||||
|
||||
def test_validate_memory_scope_rejects_instr_marker_in_team_scope() -> None:
|
||||
content = "- (2026-04-10) [instr] Always respond in Spanish\n"
|
||||
result = _validate_memory_scope(content, "team")
|
||||
assert result is not None
|
||||
assert result["status"] == "error"
|
||||
assert "[instr]" in result["message"]
|
||||
def test_validate_memory_scope_allows_old_marker_payload_in_team_scope() -> None:
|
||||
content = "- (2026-04-10) [pref] Legacy personal marker remains readable\n"
|
||||
result, _warnings = validate_memory_scope(content, "team")
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_validate_memory_scope_rejects_both_personal_markers_in_team() -> None:
|
||||
def test_validate_memory_scope_allows_team_headings() -> None:
|
||||
content = "## Engineering Conventions\n- 2026-04-10: Uses PostgreSQL\n"
|
||||
result, _warnings = validate_memory_scope(content, "team")
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_validate_bullet_format_accepts_new_and_legacy_bullets() -> None:
|
||||
content = (
|
||||
"- (2026-04-10) [pref] Prefers dark mode\n"
|
||||
"- (2026-04-10) [instr] Always respond in Spanish\n"
|
||||
"## Facts\n"
|
||||
"- 2026-04-10: Senior Python developer\n"
|
||||
"- (2026-04-10) [fact] Legacy fact is preserved\n"
|
||||
)
|
||||
result = _validate_memory_scope(content, "team")
|
||||
assert result is not None
|
||||
assert result["status"] == "error"
|
||||
assert "[instr]" in result["message"]
|
||||
assert "[pref]" in result["message"]
|
||||
|
||||
|
||||
def test_validate_memory_scope_allows_fact_in_team_scope() -> None:
|
||||
content = "- (2026-04-10) [fact] Office is in downtown Seattle\n"
|
||||
result = _validate_memory_scope(content, "team")
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_validate_memory_scope_allows_all_markers_in_user_scope() -> None:
|
||||
content = (
|
||||
"- (2026-04-10) [fact] Python developer\n"
|
||||
"- (2026-04-10) [pref] Prefers concise answers\n"
|
||||
"- (2026-04-10) [instr] Always use bullet points\n"
|
||||
)
|
||||
result = _validate_memory_scope(content, "user")
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_validate_memory_scope_allows_any_heading_in_team() -> None:
|
||||
content = "## Architecture\n- (2026-04-10) [fact] Uses PostgreSQL for persistence\n"
|
||||
result = _validate_memory_scope(content, "team")
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_validate_memory_scope_allows_any_heading_in_user() -> None:
|
||||
content = "## My Projects\n- (2026-04-10) [fact] Working on SurfSense\n"
|
||||
result = _validate_memory_scope(content, "user")
|
||||
assert result is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_bullet_format
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_validate_bullet_format_passes_valid_bullets() -> None:
|
||||
content = (
|
||||
"## Work\n"
|
||||
"- (2026-04-10) [fact] Senior Python developer\n"
|
||||
"- (2026-04-10) [pref] Prefers dark mode\n"
|
||||
"- (2026-04-10) [instr] Always respond in bullet points\n"
|
||||
)
|
||||
warnings = _validate_bullet_format(content)
|
||||
warnings = validate_bullet_format(content)
|
||||
assert warnings == []
|
||||
|
||||
|
||||
def test_validate_bullet_format_warns_on_missing_marker() -> None:
|
||||
content = "- (2026-04-10) Senior Python developer\n"
|
||||
warnings = _validate_bullet_format(content)
|
||||
def test_validate_bullet_format_warns_on_nonstandard_bullet() -> None:
|
||||
content = "## Facts\n- Senior Python developer\n"
|
||||
warnings = validate_bullet_format(content)
|
||||
assert len(warnings) == 1
|
||||
assert "Malformed bullet" in warnings[0]
|
||||
|
||||
|
||||
def test_validate_bullet_format_warns_on_missing_date() -> None:
|
||||
content = "- [fact] Senior Python developer\n"
|
||||
warnings = _validate_bullet_format(content)
|
||||
assert len(warnings) == 1
|
||||
assert "Malformed bullet" in warnings[0]
|
||||
|
||||
|
||||
def test_validate_bullet_format_warns_on_unknown_marker() -> None:
|
||||
content = "- (2026-04-10) [context] Working on project X\n"
|
||||
warnings = _validate_bullet_format(content)
|
||||
assert len(warnings) == 1
|
||||
assert "Malformed bullet" in warnings[0]
|
||||
|
||||
|
||||
def test_validate_bullet_format_ignores_non_bullet_lines() -> None:
|
||||
content = "## Some Heading\nSome paragraph text\n"
|
||||
warnings = _validate_bullet_format(content)
|
||||
assert warnings == []
|
||||
|
||||
|
||||
def test_validate_bullet_format_warns_on_old_format_without_marker() -> None:
|
||||
content = "## About the user\n- (2026-04-10) Likes cats\n"
|
||||
warnings = _validate_bullet_format(content)
|
||||
assert len(warnings) == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _save_memory — end-to-end with marker scope check
|
||||
# ---------------------------------------------------------------------------
|
||||
assert "Non-standard memory bullet" in warnings[0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_memory_blocks_pref_in_team_before_commit() -> None:
|
||||
recorder = _Recorder()
|
||||
result = await _save_memory(
|
||||
updated_memory="- (2026-04-10) [pref] Prefers dark mode\n",
|
||||
old_memory=None,
|
||||
llm=None,
|
||||
apply_fn=recorder.apply,
|
||||
commit_fn=recorder.commit,
|
||||
rollback_fn=recorder.rollback,
|
||||
label="team memory",
|
||||
scope="team",
|
||||
async def test_save_memory_blocks_new_personal_heading_in_team_before_commit(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
target = type("Target", (), {"shared_memory_md": ""})()
|
||||
session = _FakeSession()
|
||||
|
||||
async def fake_load_target(**_kwargs):
|
||||
return target
|
||||
|
||||
monkeypatch.setattr("app.services.memory.service._load_target", fake_load_target)
|
||||
|
||||
result = await save_memory(
|
||||
scope=MemoryScope.TEAM,
|
||||
target_id=1,
|
||||
content="## Preferences\n- 2026-04-10: Prefers dark mode\n",
|
||||
session=session,
|
||||
)
|
||||
assert result["status"] == "error"
|
||||
assert recorder.commit_calls == 0
|
||||
assert recorder.applied_content is None
|
||||
assert result.status == "error"
|
||||
assert session.commit_calls == 0
|
||||
assert target.shared_memory_md == ""
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_memory_allows_fact_in_team_and_commits() -> None:
|
||||
recorder = _Recorder()
|
||||
content = "- (2026-04-10) [fact] Weekly standup on Mondays\n"
|
||||
result = await _save_memory(
|
||||
updated_memory=content,
|
||||
old_memory=None,
|
||||
llm=None,
|
||||
apply_fn=recorder.apply,
|
||||
commit_fn=recorder.commit,
|
||||
rollback_fn=recorder.rollback,
|
||||
label="team memory",
|
||||
scope="team",
|
||||
async def test_save_memory_allows_grandfathered_personal_heading_in_team(monkeypatch) -> None:
|
||||
content = "## Preferences\n- 2026-04-10: Prefers dark mode\n"
|
||||
target = type("Target", (), {"shared_memory_md": content})()
|
||||
session = _FakeSession()
|
||||
|
||||
async def fake_load_target(**_kwargs):
|
||||
return target
|
||||
|
||||
monkeypatch.setattr("app.services.memory.service._load_target", fake_load_target)
|
||||
|
||||
result = await save_memory(
|
||||
scope=MemoryScope.TEAM,
|
||||
target_id=1,
|
||||
content=content,
|
||||
session=session,
|
||||
)
|
||||
assert result["status"] == "saved"
|
||||
assert recorder.commit_calls == 1
|
||||
assert recorder.applied_content == content
|
||||
assert result.status == "saved"
|
||||
assert session.commit_calls == 1
|
||||
assert target.shared_memory_md == content.strip()
|
||||
assert result.warnings
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_memory_includes_format_warnings() -> None:
|
||||
recorder = _Recorder()
|
||||
content = "- (2026-04-10) Missing marker text\n"
|
||||
result = await _save_memory(
|
||||
updated_memory=content,
|
||||
old_memory=None,
|
||||
llm=None,
|
||||
apply_fn=recorder.apply,
|
||||
commit_fn=recorder.commit,
|
||||
rollback_fn=recorder.rollback,
|
||||
label="memory",
|
||||
scope="user",
|
||||
async def test_save_memory_strips_preamble_before_heading(monkeypatch) -> None:
|
||||
target = type("Target", (), {"memory_md": ""})()
|
||||
session = _FakeSession()
|
||||
|
||||
async def fake_load_target(**_kwargs):
|
||||
return target
|
||||
|
||||
monkeypatch.setattr("app.services.memory.service._load_target", fake_load_target)
|
||||
|
||||
result = await save_memory(
|
||||
scope=MemoryScope.USER,
|
||||
target_id="00000000-0000-0000-0000-000000000000",
|
||||
content="Sure, here is the update:\n\n## Facts\n- 2026-04-10: Likes cats\n",
|
||||
session=session,
|
||||
)
|
||||
assert result["status"] == "saved"
|
||||
assert "format_warnings" in result
|
||||
assert len(result["format_warnings"]) == 1
|
||||
assert result.status == "saved"
|
||||
assert target.memory_md == "## Facts\n- 2026-04-10: Likes cats"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_memory_rejects_long_no_heading_payload(monkeypatch) -> None:
|
||||
target = type("Target", (), {"memory_md": ""})()
|
||||
session = _FakeSession()
|
||||
|
||||
async def fake_load_target(**_kwargs):
|
||||
return target
|
||||
|
||||
monkeypatch.setattr("app.services.memory.service._load_target", fake_load_target)
|
||||
|
||||
result = await save_memory(
|
||||
scope=MemoryScope.USER,
|
||||
target_id="00000000-0000-0000-0000-000000000000",
|
||||
content="NO_UPDATE because there is nothing durable to remember.",
|
||||
session=session,
|
||||
)
|
||||
assert result.status == "error"
|
||||
assert "## heading" in result.message
|
||||
assert session.commit_calls == 0
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue