mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-14 08:18:36 +00:00
Feature: handle renaming of references to memories
This commit is contained in:
1 parent
4120359154
commit
437bddd7a4
3 files changed
+231
-23
No files matched your search
+48
-2
@@ -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"(?<!{name_char}){re.escape(ref_old)}(?!{name_char})"
|
||||
|
||||
# use a callable replacement to avoid backreference interpretation of characters in ref_new
|
||||
return re.subn(pattern, lambda _m: ref_new, content)
|
||||
|
||||
def get_memory_file_path(self, name: str) -> 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)
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user