From 61d62a3cb8890c436b0b9fb16ab6cade62b7c5ca Mon Sep 17 00:00:00 2001 From: Mubashir R <112580905+Mubashirrrr@users.noreply.github.com> Date: Wed, 3 Jun 2026 09:41:46 +0500 Subject: [PATCH] Fix memory bullet extraction in service copy Fix services.memory bullet-list extraction by grouping the bullet/number regex before the capture, and cover both memory manager copies in the regression test. --- services/memory/memory.py | 8 ++++++-- tests/test_memory_bullet_extraction.py | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/services/memory/memory.py b/services/memory/memory.py index 374961b..0f092db 100644 --- a/services/memory/memory.py +++ b/services/memory/memory.py @@ -59,8 +59,12 @@ class MemoryManager: line = line.strip() # Look for bullet points or numbered lists that might contain memories if re.match(r'^[-*•]|\d+\.', line): - # Extract the text after the bullet/number - text_match = re.match(r'^[-*•]|\d+\.\s*(.*)', line) + # Extract the text after the bullet/number. Group both + # markers so the capture applies to either. The previous + # `^[-*•]|\d+\.\s*(.*)` put the group on the numbered + # branch only, so a bullet line matched with group(1)=None + # and crashed on .strip(). + text_match = re.match(r'^(?:[-*•]|\d+\.)\s*(.*)', line) if text_match: text = text_match.group(1).strip() if text: diff --git a/tests/test_memory_bullet_extraction.py b/tests/test_memory_bullet_extraction.py index 3c871ee..1e5fc2c 100644 --- a/tests/test_memory_bullet_extraction.py +++ b/tests/test_memory_bullet_extraction.py @@ -7,12 +7,22 @@ capture group lives only in the numbered-list branch. A bullet line ("- ...") matches the first branch, so ``group(1)`` is ``None`` and ``.strip()`` raised ``AttributeError``, crashing extraction for any assistant message that contains a bullet list (the dominant case). + +There are two copies of ``MemoryManager``: ``src.memory`` and the +``services.memory`` package that ``routes/memory_routes.py`` actually imports. +The fix first landed only in ``src.memory`` while the live route path kept the +broken copy, and this test imported ``src.memory`` so it stayed green. It now +exercises both copies so the two cannot drift back apart. """ -from src.memory import MemoryManager +import pytest + +from src.memory import MemoryManager as SrcMemoryManager +from services.memory.memory import MemoryManager as ServiceMemoryManager -def test_extract_memory_from_chat_handles_bullets(tmp_path): - mgr = MemoryManager(str(tmp_path)) +@pytest.mark.parametrize("manager_cls", [SrcMemoryManager, ServiceMemoryManager]) +def test_extract_memory_from_chat_handles_bullets(manager_cls, tmp_path): + mgr = manager_cls(str(tmp_path)) chat = [{ "role": "assistant", "content": "- User likes coffee\n* Prefers tea in winter\n1. Wakes at 6am",