From 437bddd7a47390021d9985644249443c2bf1691c Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 12 May 2026 17:11:43 +0200 Subject: [PATCH] Feature: handle renaming of references to memories --- src/serena/project.py | 50 ++++++++- src/serena/tools/memory_tools.py | 48 +++++---- test/serena/test_memories_manager.py | 156 +++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 23 deletions(-) create mode 100644 test/serena/test_memories_manager.py diff --git a/src/serena/project.py b/src/serena/project.py index ae062420..54c621d3 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -33,6 +33,7 @@ log = logging.getLogger(__name__) class MemoriesManager: GLOBAL_TOPIC = "global" _global_memory_dir = SerenaPaths().global_memories_path + _MEMORY_REF_PREFIX = "mem:" def __init__( self, @@ -77,9 +78,48 @@ class MemoriesManager: def _is_global(self, name: str) -> bool: return name == self.GLOBAL_TOPIC or name.startswith(self.GLOBAL_TOPIC + "/") + @classmethod + def _prepare_name(cls, name: str) -> str: + """Corrects the name for common mistakes made by LLMs (``mem:`` prefix, ``.md`` suffix, OS-specific separators).""" + name = name.removeprefix(cls._MEMORY_REF_PREFIX) + if name.endswith(".md"): + name = name[:-3] + return name.replace(os.sep, "/") + + @classmethod + def _add_reference_prefix(cls, name: str) -> str: + name = cls._prepare_name(name) + return cls._MEMORY_REF_PREFIX + name + + def rename_references_to_memory(self, content: str, old_name: str, new_name: str) -> tuple[str, int]: + r""" + Replaces all occurrences of a memory reference (e.g. ``mem:foo``) in ``content`` with + the reference to ``new_name``. + + Matches only references whose name is exactly ``old_name``: the match must not be + embedded in a longer memory name. A memory name consists of the character class + ``[A-Za-z0-9_\\-/]`` (alphanumerics, underscore, hyphen, slash for topic separation), + which determines the boundary of the match. The surrounding delimiters (backticks, + quotes, parentheses, whitespace, etc.) are intentionally unconstrained. + + :param content: the text to search through + :param old_name: the memory name being renamed away from (without the ``mem:`` prefix) + :param new_name: the memory name being renamed to (without the ``mem:`` prefix) + :return: a tuple of (updated content, number of replacements made) + """ + # define the character class that constitutes a memory name; matches inside such a run are not real references + name_char = r"[A-Za-z0-9_\-/]" + ref_old = self._add_reference_prefix(old_name) + ref_new = self._add_reference_prefix(new_name) + + # build a pattern that anchors the reference on both sides so it cannot be embedded inside a longer name + pattern = rf"(? Path: - # Strip .md extension if present - name = name.replace(".md", "").replace(os.sep, "/") + name = self._prepare_name(name) parts = name.split("/") if ".." in parts: raise ValueError(f"Memory name cannot contain '..' segments for security reasons. Got: {name}") @@ -118,6 +158,7 @@ class MemoriesManager: raise PermissionError(f"Attempted to write to read_only memory: '{name}')") def load_memory(self, name: str) -> str: + name = self._prepare_name(name) self._check_not_ignored(name) memory_file_path = self.get_memory_file_path(name) if not memory_file_path.exists(): @@ -126,6 +167,7 @@ class MemoriesManager: return f.read() def save_memory(self, name: str, content: str, is_tool_context: bool) -> str: + name = self._prepare_name(name) self._check_not_ignored(name) self._check_write_access(name, is_tool_context) memory_file_path = self.get_memory_file_path(name) @@ -208,6 +250,7 @@ class MemoriesManager: return memories def delete_memory(self, name: str, is_tool_context: bool) -> str: + name = self._prepare_name(name) self._check_not_ignored(name) self._check_write_access(name, is_tool_context) memory_file_path = self.get_memory_file_path(name) @@ -221,6 +264,8 @@ class MemoriesManager: Rename or move a memory file. Moving between global and project scope (e.g. "global/foo" -> "bar") is supported. """ + old_name = self._prepare_name(old_name) + new_name = self._prepare_name(new_name) self._check_not_ignored(old_name) self._check_not_ignored(new_name) self._check_write_access(new_name, is_tool_context) @@ -250,6 +295,7 @@ class MemoriesManager: :param mode: "literal" or "regex" :param allow_multiple_occurrences: """ + name = self._prepare_name(name) self._check_not_ignored(name) self._check_write_access(name, is_tool_context) memory_file_path = self.get_memory_file_path(name) diff --git a/src/serena/tools/memory_tools.py b/src/serena/tools/memory_tools.py index 9ecb0c0c..0fd98ec4 100644 --- a/src/serena/tools/memory_tools.py +++ b/src/serena/tools/memory_tools.py @@ -1,7 +1,10 @@ +import logging from typing import Literal from serena.tools import Tool, ToolMarkerCanEdit +log = logging.getLogger(__name__) + class WriteMemoryTool(Tool, ToolMarkerCanEdit): """ @@ -11,13 +14,15 @@ class WriteMemoryTool(Tool, ToolMarkerCanEdit): def apply(self, memory_name: str, content: str, max_chars: int = -1) -> str: """ - Write information (utf-8-encoded) about this project that can be useful for future tasks to a memory in md format. - The memory name should be meaningful and can include "/" to organize into topics (e.g., "auth/login/logic"). - If explicitly instructed, use the "global/" prefix for writing a memory that is shared across projects - (e.g., "global/java/style_guide") + Write information about this project that can be useful for future tasks in md format. + The name should be meaningful and can include "/" to organize into topics. + If explicitly instructed, use the "global/" prefix for writing a memory that is shared across projects. + References to other memories should be inside backticks and prefixed with mem:, + e.g., `mem:auth`. - :param max_chars: the maximum number of characters to write. By default, determined by the config, - change only if instructed to do so. + :param memory_name: memory name + :param content: memory content, utf8-encoded + :param max_chars: see other tools """ # NOTE: utf-8 encoding is configured in the MemoriesManager if max_chars == -1: @@ -32,23 +37,19 @@ class WriteMemoryTool(Tool, ToolMarkerCanEdit): class ReadMemoryTool(Tool): """ - Read the content of a memory file. This tool should only be used if the information - is relevant to the current task. You can infer whether the information - is relevant from the memory file name. - You should not read the same memory file multiple times in the same conversation. + Reads the content of a memory file. """ def apply(self, memory_name: str) -> str: """ - Reads the contents of a memory. Should only be used if the information - is likely to be relevant to the current task, inferring relevance from the memory name. + Use to read a memory that is likely to be relevant to the current task, inferring relevance e.g. from the name. """ return self.memories_manager.load_memory(memory_name) class ListMemoriesTool(Tool): """ - List available memories. Any memory can be read using the `read_memory` tool. + Lists available memories. """ def apply(self, topic: str = "") -> str: @@ -60,9 +61,7 @@ class ListMemoriesTool(Tool): class DeleteMemoryTool(Tool, ToolMarkerCanEdit): """ - Delete a memory file. Should only happen if a user asks for it explicitly, - for example by saying that the information retrieved from a memory file is no longer correct - or no longer relevant for the project. + Delete a memory file. """ def apply(self, memory_name: str) -> str: @@ -74,16 +73,23 @@ class DeleteMemoryTool(Tool, ToolMarkerCanEdit): class RenameMemoryTool(Tool, ToolMarkerCanEdit): """ - Renames or moves a memory. Moving between project and global scope is supported - (e.g., renaming "global/foo" to "bar" moves it from global to project scope). + Renames or moves a memory, updating references that are marked with the `mem:` prefix. """ def apply(self, old_name: str, new_name: str) -> str: """ Rename or move a memory, use "/" in the name to organize into topics. The "global" topic should only be used if explicitly instructed. + References to other memories that are marked with the `mem:` prefix will be updated accordingly. """ - return self.memories_manager.move_memory(old_name, new_name, is_tool_context=True) + renaming_message = self.memories_manager.move_memory(old_name, new_name, is_tool_context=True) + for memory in self.memories_manager.list_memories().get_full_list(): + memory_content = self.memories_manager.load_memory(memory) + updated_content, n_replacements = self.memories_manager.rename_references_to_memory(memory_content, old_name, new_name) + if n_replacements > 0: + log.info(f"Updated {n_replacements} references to memory {old_name} to {new_name}") + self.memories_manager.save_memory(memory, updated_content, is_tool_context=True) + return renaming_message class EditMemoryTool(Tool, ToolMarkerCanEdit): @@ -100,10 +106,10 @@ class EditMemoryTool(Tool, ToolMarkerCanEdit): allow_multiple_occurrences: bool = False, ) -> str: r""" - Replaces content matching a regular expression in a memory. + Replace content matching a regular expression in a memory. :param memory_name: the name of the memory - :param needle: the string or regex pattern to search for. + :param needle: the string or regex pattern to search for. In regex mode, be careful to not replace too much! If `mode` is "literal", this string will be matched exactly. If `mode` is "regex", this string will be treated as a regular expression (syntax of Python's `re` module, with flags DOTALL and MULTILINE enabled). diff --git a/test/serena/test_memories_manager.py b/test/serena/test_memories_manager.py new file mode 100644 index 00000000..a862846c --- /dev/null +++ b/test/serena/test_memories_manager.py @@ -0,0 +1,156 @@ +""" +Unit tests for :class:`serena.project.MemoriesManager` helpers that do not +require touching the filesystem (in particular, :meth:`rename_references_to_memory` +and :meth:`_prepare_name`). +""" + +import os + +import pytest + +from serena.project import MemoriesManager + + +@pytest.fixture +def manager() -> MemoriesManager: + # serena_data_folder=None: the tested helpers do not access the filesystem + return MemoriesManager(serena_data_folder=None) + + +class TestPrepareName: + @pytest.mark.parametrize( + "name", + [ + "memory", + "emergency", + "meeting_notes", + "em-dash", + "ml_notes", + "embedding", + "e", + "m", + "mem", + ":colon_prefixed_unlikely", + ], + ) + def test_does_not_strip_partial_prefix_chars_from_real_names(self, manager: MemoriesManager, name: str) -> None: + """A regression guard: previously used ``str.lstrip("mem:")`` which strips any leading m/e/:.""" + assert manager._prepare_name(name) == name + + @pytest.mark.parametrize( + ("input_name", "expected"), + [ + ("mem:foo", "foo"), + ("mem:foo/bar", "foo/bar"), + ("mem:memory", "memory"), # prefix is removed only once, not greedily + ("mem:", ""), + ], + ) + def test_strips_mem_prefix_only_when_actually_a_prefix(self, manager: MemoriesManager, input_name: str, expected: str) -> None: + assert manager._prepare_name(input_name) == expected + + @pytest.mark.parametrize( + ("input_name", "expected"), + [ + ("foo.md", "foo"), + ("topic/foo.md", "topic/foo"), + ("a.md.notes", "a.md.notes"), # ".md" must only be stripped as trailing extension + ("foo.markdown", "foo.markdown"), + (".md", ""), + ], + ) + def test_strips_md_only_as_suffix(self, manager: MemoriesManager, input_name: str, expected: str) -> None: + assert manager._prepare_name(input_name) == expected + + def test_normalizes_os_separator(self, manager: MemoriesManager) -> None: + assert manager._prepare_name(f"topic{os.sep}sub{os.sep}name") == "topic/sub/name" + + def test_combined_normalizations(self, manager: MemoriesManager) -> None: + assert manager._prepare_name(f"mem:topic{os.sep}foo.md") == "topic/foo" + + +class TestRenameReferencesToMemory: + @pytest.mark.parametrize( + ("content", "old_name", "new_name", "expected_content", "expected_count"), + [ + # simple cases + pytest.param("see mem:foo for details", "foo", "bar", "see mem:bar for details", 1, id="simple"), + pytest.param("nothing to see here", "foo", "bar", "nothing to see here", 0, id="no_match"), + pytest.param( + "mem:foo and mem:foo and `mem:foo`", + "foo", + "bar", + "mem:bar and mem:bar and `mem:bar`", + 3, + id="multiple_occurrences", + ), + # boundaries + pytest.param("mem:foo rest", "foo", "bar", "mem:bar rest", 1, id="start_of_string"), + pytest.param("prefix mem:foo", "foo", "bar", "prefix mem:bar", 1, id="end_of_string"), + pytest.param("mem:foo", "foo", "bar", "mem:bar", 1, id="entire_string"), + # must NOT match: embedded in longer name + pytest.param("`mem:foobar`", "foo", "bar", "`mem:foobar`", 0, id="longer_suffix"), + pytest.param("`mem:foo/sub`", "foo", "bar", "`mem:foo/sub`", 0, id="subtopic"), + pytest.param("mem:foo-extra", "foo", "bar", "mem:foo-extra", 0, id="hyphen_suffix"), + pytest.param("mem:foo_extra", "foo", "bar", "mem:foo_extra", 0, id="underscore_suffix"), + pytest.param("xmem:foo", "foo", "bar", "xmem:foo", 0, id="preceded_by_name_char"), + # boundaries on non-name characters + pytest.param("see `mem:foo.md`", "foo", "bar", "see `mem:bar.md`", 1, id="md_extension_suffix"), + # topic renames + pytest.param("`mem:foo/bar`", "foo/bar", "baz", "`mem:baz`", 1, id="topic_to_flat"), + pytest.param("`mem:foo`", "foo/bar", "baz", "`mem:foo`", 0, id="parent_does_not_match_topic"), + pytest.param("`mem:foo`", "foo", "topic/bar", "`mem:topic/bar`", 1, id="flat_to_topic"), + # regex-meta safety + pytest.param("`mem:a.b`", "a.b", "c", "`mem:c`", 1, id="dot_in_old_name_literal"), + pytest.param("`mem:axb`", "a.b", "c", "`mem:axb`", 0, id="dot_in_old_name_not_wildcard"), + # mixed + pytest.param( + "valid `mem:foo`, embedded mem:foobar, subtopic mem:foo/x, and another `mem:foo`.", + "foo", + "bar", + "valid `mem:bar`, embedded mem:foobar, subtopic mem:foo/x, and another `mem:bar`.", + 2, + id="mixed_matching_and_nonmatching", + ), + # input names that previously broke due to lstrip("mem:") in _add_reference_prefix + pytest.param("see `mem:memory`", "memory", "diary", "see `mem:diary`", 1, id="old_name_starts_with_m"), + pytest.param("see `mem:emergency`", "emergency", "incident", "see `mem:incident`", 1, id="old_name_starts_with_e"), + pytest.param("see `mem:foo`", "foo", "memory", "see `mem:memory`", 1, id="new_name_starts_with_m"), + ], + ) + def test_rename( + self, + manager: MemoriesManager, + content: str, + old_name: str, + new_name: str, + expected_content: str, + expected_count: int, + ) -> None: + result, n = manager.rename_references_to_memory(content, old_name, new_name) + assert result == expected_content + assert n == expected_count + + @pytest.mark.parametrize( + "delimiter_pair", + [ + ("`", "`"), + ('"', '"'), + ("'", "'"), + ("(", ")"), + ("[", "]"), + ("{", "}"), + (" ", " "), + ("\n", "\n"), + ("\t", "\t"), + (",", ","), + (";", ";"), + ("", ""), # raw, no delimiters + ], + ) + def test_replacement_inside_various_delimiters(self, manager: MemoriesManager, delimiter_pair: tuple[str, str]) -> None: + left, right = delimiter_pair + content = f"{left}mem:foo{right}" + result, n = manager.rename_references_to_memory(content, "foo", "bar") + assert result == f"{left}mem:bar{right}" + assert n == 1