diff --git a/src/serena/cli.py b/src/serena/cli.py index ee0bf173..86fc4d5f 100644 --- a/src/serena/cli.py +++ b/src/serena/cli.py @@ -1113,6 +1113,149 @@ class ToolCommands(AutoRegisteringGroup): click.echo(mcp_tool.description) +class MemoryCommands(AutoRegisteringGroup): + """Group for 'memories' subcommands; manage and inspect a project's memory files.""" + + def __init__(self) -> None: + super().__init__( + name="memories", + help="Inspect, read, write, and validate Serena memories. " + "You can run `serena memories --help` for more info on each command.", + ) + + @staticmethod + def _load_memories_manager(project: str): + """Load the active project and return its memories manager.""" + from serena.project import Project + + serena_config = SerenaConfig.from_config_file() + proj = Project.load(os.path.abspath(project), serena_config=serena_config) + return proj.memories_manager + + @staticmethod + @click.command( + "list", + help="List memories of the active project (and global memories).", + context_settings={"max_content_width": _MAX_CONTENT_WIDTH}, + ) + @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) + @click.option("--topic", "-t", type=str, default="", help="Restrict the listing to a single topic (e.g. 'auth' or 'global/style').") + def list(project: str, topic: str) -> None: + manager = MemoryCommands._load_memories_manager(project) + memories = manager.list_memories(topic=topic) + if not len(memories): + click.echo("(no memories found)") + return + if memories.memories: + click.echo("Project memories:") + for name in sorted(memories.memories): + click.echo(f" - {name}") + if memories.read_only_memories: + click.echo("Read-only memories:") + for name in sorted(memories.read_only_memories): + click.echo(f" - {name}") + + @staticmethod + @click.command("read", help="Print the content of a memory to stdout.", context_settings={"max_content_width": _MAX_CONTENT_WIDTH}) + @click.argument("memory_name", type=str) + @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) + def read(memory_name: str, project: str) -> None: + manager = MemoryCommands._load_memories_manager(project) + click.echo(manager.load_memory(memory_name)) + + @staticmethod + @click.command( + "write", + help="Write a memory file. Reads the content from --content, --file, or stdin (in that order of precedence).", + context_settings={"max_content_width": _MAX_CONTENT_WIDTH}, + ) + @click.argument("memory_name", type=str) + @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) + @click.option("--content", type=str, default=None, help="Memory content provided directly on the command line.") + @click.option( + "--file", "file_path", type=click.Path(exists=True, dir_okay=False), default=None, help="Read memory content from the given file." + ) + def write(memory_name: str, project: str, content: str | None, file_path: str | None) -> None: + if content is None and file_path is not None: + with open(file_path, encoding="utf-8") as f: + content = f.read() + if content is None: + content = sys.stdin.read() + manager = MemoryCommands._load_memories_manager(project) + click.echo(manager.save_memory(memory_name, content, is_tool_context=False)) + + @staticmethod + @click.command( + "check", + help=( + "Check referential integrity across all memories of the project (and global memories). " + "Reports stale `mem:` references and unmarked-reference warnings split into high- and " + "low-confidence groups. Read-only and never writes. Always exits 0." + ), + context_settings={"max_content_width": _MAX_CONTENT_WIDTH}, + ) + @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) + def check(project: str) -> None: + manager = MemoryCommands._load_memories_manager(project) + report = manager.validate_referential_integrity() + click.echo(report.format()) + + @staticmethod + @click.command( + "auto-prefix-references", + help=( + "Rewrite exact bare occurrences of existing memory names by adding the `mem:` prefix. " + "This is a heuristic, file-mutating operation: a word that happens to coincide with a memory name " + "will be rewritten as a reference even if it was intended as ordinary prose. " + "Use --dry-run to preview the rewrites without modifying any files. " + "\n\n" + "Scope is narrower than what `serena memories check` reports. Only EXACT bare occurrences are rewritten " + "(the body text must equal an existing memory name verbatim); fuzzy near-miss findings surfaced by `check` " + "are NOT autofixable here, since rewriting them would require substring substitution rather than a prefix " + "addition — they are reported for manual review. " + "By default the rewrite is further restricted to memory names containing `/` or longer than the configured " + "threshold, and skips global and read-only memories; use the --include-* flags below to widen the scope." + ), + context_settings={"max_content_width": _MAX_CONTENT_WIDTH}, + ) + @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) + @click.option( + "--dry-run", + is_flag=True, + default=False, + help="Preview the rewrites this command would apply; do not modify any files.", + ) + @click.option( + "--include-flat-names", + is_flag=True, + default=False, + help="Also rewrite short, flat memory names (no `/`, below the length threshold). Raises false-positive risk significantly.", + ) + @click.option( + "--include-read-only", + is_flag=True, + default=False, + help="Also rewrite occurrences inside read-only memories.", + ) + @click.option( + "--include-global", + is_flag=True, + default=False, + help="Also rewrite occurrences inside global memories (affects every project consuming them).", + ) + def auto_prefix_references( + project: str, dry_run: bool, include_flat_names: bool, include_read_only: bool, include_global: bool + ) -> None: + manager = MemoryCommands._load_memories_manager(project) + report = manager.auto_prefix_bare_references( + include_flat_names=include_flat_names, + include_read_only=include_read_only, + include_global=include_global, + dry_run=dry_run, + ) + click.echo(report.format()) + + class PromptCommands(AutoRegisteringGroup): def __init__(self) -> None: super().__init__(name="prompts", help="Commands related to Serena's prompts that are outside of contexts and modes.") @@ -1244,10 +1387,11 @@ _project = ProjectCommands() _config = SerenaConfigCommands() _tools = ToolCommands() _prompts = PromptCommands() +_memories = MemoryCommands() # Expose so we can use this as an entrypoint top_level = TopLevelCommands() # needed for the help script to work - register all subcommands to the top-level group -for subgroup in (_mode, _context, _project, _config, _tools, _prompts): +for subgroup in (_mode, _context, _project, _config, _tools, _prompts, _memories): top_level.add_command(subgroup) diff --git a/src/serena/project.py b/src/serena/project.py index 54c621d3..dceb65a7 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -3,7 +3,9 @@ import os import re import shutil import threading -from collections.abc import Sequence +from collections.abc import Iterator, Sequence +from dataclasses import dataclass, field +from difflib import SequenceMatcher from pathlib import Path from typing import TYPE_CHECKING, Any, Literal, Optional @@ -118,6 +120,192 @@ class MemoriesManager: # use a callable replacement to avoid backreference interpretation of characters in ref_new return re.subn(pattern, lambda _m: ref_new, content) + NAME_SIMILARITY_THRESHOLD: float = 0.55 + _HIGH_CONFIDENCE_NAME_LENGTH: int = 10 + _SHORT_NAME_FLOOR: int = 3 + # For fuzzy bare-text matching (validate_referential_integrity), an additional safeguard: + # the bare token and the candidate existing name must share at least this fraction of their + # tokenized name parts. Containment / SequenceMatcher signals alone over-match — e.g. a + # generic prose word like "repository" is a substring of "serena_repository_structure" + # but their token Jaccard is only 1/3, so we reject it at this floor. + _FUZZY_BARE_TOKEN_JACCARD_FLOOR: float = 0.5 + _VERSION_SUFFIX_PATTERN = re.compile(r"_(?:v\d+|old|new|legacy|bak)$", re.IGNORECASE) + + @classmethod + def _normalize_for_similarity(cls, name: str) -> str: + """ + :param name: a memory name + :return: a lowercased copy of ``name`` with version/legacy-style trailing suffixes stripped + (e.g. ``"auth_v2"`` -> ``"auth"``); used as the canonical form for similarity scoring. + """ + return cls._VERSION_SUFFIX_PATTERN.sub("", name.lower()) + + @classmethod + def _tokenize_name(cls, name: str) -> set[str]: + """ + :param name: a memory name + :return: the set of lowercase tokens extracted by splitting ``name`` on ``/``, ``_``, ``-`` + and at camelCase boundaries; empty tokens are dropped. + """ + # split on path/word separators and at camelCase boundaries + parts = re.split(r"[/_\-]|(?<=[a-z])(?=[A-Z])", name) + return {p.lower() for p in parts if p} + + @classmethod + def compute_name_similarity(cls, a: str, b: str) -> float: + """ + Computes a similarity score in ``[0, 1]`` between two memory names. Examples + (with default threshold ``0.55``):: + + auth/login == auth/login -> 1.00 (exact) + auth_v1 ~ auth_v2 -> 1.00 (version suffix normalized) + login ~ auth/login -> 1.00 (flat <-> topic move) + auth/login ~ auth/v2/login -> 0.75 (shared prefix token + basename) + auth/login ~ security/login -> 0.50 (basename only; disjoint topics) + auth ~ authentication -> 0.64 (substring containment) + foo ~ for -> 0.00 (short-name floor) + + :param a: first memory name + :param b: second memory name + :return: the similarity score + """ + # normalize: lowercase + strip version/legacy suffixes + norm_a, norm_b = cls._normalize_for_similarity(a), cls._normalize_for_similarity(b) + if norm_a == norm_b: + return 1.0 + + # split into prefix / basename + basename_a = norm_a.rsplit("/", 1)[-1] + basename_b = norm_b.rsplit("/", 1)[-1] + prefix_a = norm_a.rsplit("/", 1)[0] if "/" in norm_a else "" + prefix_b = norm_b.rsplit("/", 1)[0] if "/" in norm_b else "" + + # case 1: basenames match -- score is determined entirely by prefix relationship + if basename_a == basename_b: + if not prefix_a or not prefix_b: + # one side is flat -> canonical topic move, strongly confident + return 1.0 + # both sides carry a topic prefix -> blend basename match (0.5 floor) with prefix token similarity + prefix_tokens_a, prefix_tokens_b = cls._tokenize_name(prefix_a), cls._tokenize_name(prefix_b) + prefix_jaccard = ( + len(prefix_tokens_a & prefix_tokens_b) / len(prefix_tokens_a | prefix_tokens_b) + if (prefix_tokens_a | prefix_tokens_b) + else 0.0 + ) + return 0.5 + 0.5 * prefix_jaccard + + # case 2: basenames differ -- combine generic similarity signals via max + tokens_a, tokens_b = cls._tokenize_name(norm_a), cls._tokenize_name(norm_b) + jaccard = len(tokens_a & tokens_b) / len(tokens_a | tokens_b) if (tokens_a | tokens_b) else 0.0 + seq = SequenceMatcher(None, norm_a, norm_b).ratio() + # containment is a meaningful rename signal on its own (e.g. auth -> authentication), + # so we lift it above the threshold floor when one name fully contains the other, + # with the length-ratio gating how confident we are + contained = 0.0 + if norm_a and norm_b and (norm_a in norm_b or norm_b in norm_a): + length_ratio = min(len(norm_a), len(norm_b)) / max(len(norm_a), len(norm_b)) + contained = 0.5 + 0.5 * length_ratio + + # short-name floor: for very short names, only accept candidates with a strong token signal + if min(len(norm_a), len(norm_b)) <= cls._SHORT_NAME_FLOOR and jaccard < 1.0: + return 0.0 + return max(jaccard, seq, contained) + + @classmethod + def _find_stale_reference_candidates(cls, missing_name: str, existing_names: list[str], threshold: float | None = None) -> list[str]: + """ + :param missing_name: the unresolved reference target (without the ``mem:`` prefix). + :param existing_names: the names of all currently existing memories. + :param threshold: optional override for the minimum similarity required; defaults to + :attr:`NAME_SIMILARITY_THRESHOLD`. + :return: existing memory names whose similarity to ``missing_name`` meets or exceeds + ``threshold``, sorted in descending order of similarity (ties broken alphabetically). + """ + cutoff = cls.NAME_SIMILARITY_THRESHOLD if threshold is None else threshold + scored: list[tuple[float, str]] = [] + for existing in existing_names: + score = cls.compute_name_similarity(missing_name, existing) + if score >= cutoff: + scored.append((score, existing)) + scored.sort(key=lambda pair: (-pair[0], pair[1])) + return [name for _, name in scored] + + # character class delimiting a memory name; used to anchor regex matches so we never + # consume a partial name (kept in sync with the boundary rule in rename_references_to_memory) + _NAME_CHAR_CLASS: str = r"[A-Za-z0-9_\-/]" + + @classmethod + def _iter_referenced_names_in_content(cls, content: str) -> Iterator[str]: + """ + :param content: arbitrary memory content + :return: an iterator over the memory names appearing as ``mem:NAME`` references in + ``content``; duplicate occurrences are yielded once each. + """ + # use the same boundary rule as rename_references_to_memory so the two stay consistent + pattern = rf"(? int: + """ + :param content: arbitrary memory content + :param name: a memory name to look for + :return: the count of bare (i.e. not already ``mem:``-prefixed) word-boundary-anchored + occurrences of ``name`` in ``content``. The match must not be preceded by ``mem:`` + and must not be embedded within a longer memory-name-like run of characters. + """ + # negative lookbehind for the mem: prefix prevents double-counting valid references; + # the additional name-char lookbehind/lookahead prevents matching inside longer names + pattern = ( + rf"(? tuple[str, int]: + """ + :param content: arbitrary memory content + :param name: the memory name whose bare occurrences should be rewritten + :return: ``(new_content, n_replacements)`` after prefixing each bare occurrence of + ``name`` with ``mem:`` (using the same boundary rule as + :meth:`_find_bare_occurrences`). + """ + pattern = ( + rf"(? Iterator[tuple[str, int]]: + """ + :param content: arbitrary memory content + :return: an iterator over ``(token, count)`` pairs for each distinct name-shaped + token in ``content`` whose length meets :attr:`_HIGH_CONFIDENCE_NAME_LENGTH` + and which is not preceded by ``mem:`` (those are already valid references). + The boundaries follow :attr:`_NAME_CHAR_CLASS`, so embedded substrings of + longer name-character runs are not matched. + """ + # require minimum length so we only scan tokens that are unlikely to be coincidental prose + pattern = ( + rf"(? Path: name = self._prepare_name(name) parts = name.split("/") @@ -204,6 +392,212 @@ class MemoriesManager: def get_full_list(self) -> list[str]: return sorted(self.memories + self.read_only_memories) + @dataclass(frozen=True) + class StaleReference: + """ + A ``mem:NAME`` reference whose target memory does not exist. + + :ivar source_memory: the name of the memory whose content contains the broken reference. + :ivar referenced_name: the name following ``mem:`` that did not resolve to an existing memory. + :ivar candidates: existing memory names proposed as likely intended targets, ranked by + decreasing similarity. May be empty if no candidate exceeded the similarity threshold. + :ivar source_is_read_only: whether the source memory is read-only. + """ + + source_memory: str + referenced_name: str + candidates: list[str] + source_is_read_only: bool + + @dataclass(frozen=True) + class UnmarkedReferenceWarning: + """ + A bare occurrence in a memory's content that looks like a forgotten reference to an + existing memory. + + Two flavours of finding share this class: + + * **exact match** — the bare text equals an existing memory name verbatim + (``actual_token`` either equals ``suspected_name`` or is left empty). + * **fuzzy near-miss** — the bare text does not equal any existing memory name, but + a long, distinctive token in the body similarity-matches a high-confidence existing + memory name (``actual_token`` is the bare text actually found, distinct from + ``suspected_name``). Such findings are reported but **not** rewritten by + :meth:`auto_prefix_bare_references`, since they would require substring + substitution rather than a prefix addition. + + :ivar source_memory: the name of the memory whose content contains the bare occurrence. + :ivar suspected_name: the existing memory name proposed as the intended target. + :ivar occurrences: the number of occurrences of ``actual_token`` in the source memory's content. + :ivar is_high_confidence: True when ``suspected_name`` contains a ``/`` separator or + exceeds the configured length threshold; such names are unlikely to coincide with + ordinary prose. False otherwise (a low-confidence warning). + :ivar source_is_read_only: whether the source memory is read-only. + :ivar actual_token: the bare text actually found in the source memory's content. + Defaults to ``""``, which is taken to mean ``suspected_name`` (the exact-match case). + When non-empty and different from ``suspected_name``, this is a fuzzy near-miss. + """ + + source_memory: str + suspected_name: str + occurrences: int + is_high_confidence: bool + source_is_read_only: bool + actual_token: str = "" + + @property + def is_exact_match(self) -> bool: + """:return: True iff the bare text in the body equals ``suspected_name`` (i.e. not a fuzzy near-miss).""" + return self.actual_token == "" or self.actual_token == self.suspected_name + + @dataclass(frozen=True) + class AutofixedReference: + """ + A bare occurrence rewritten to include the ``mem:`` prefix. + + :ivar source_memory: the name of the memory whose content was modified. + :ivar referenced_name: the memory name whose bare occurrences were prefixed. + :ivar n_replacements: the number of bare occurrences replaced in the source memory. + """ + + source_memory: str + referenced_name: str + n_replacements: int + + @dataclass + class ReferentialIntegrityReport: + """ + Outcome of :meth:`MemoriesManager.validate_referential_integrity`. + + :ivar stale_references: ``mem:NAME`` references whose target memory does not exist. + :ivar high_confidence_warnings: bare references whose suspected target name is unlikely + to be coincidental prose (topic-path or sufficiently long). + :ivar low_confidence_warnings: bare references whose suspected target name could plausibly + appear in ordinary prose (short, flat names). + """ + + stale_references: list["MemoriesManager.StaleReference"] = field(default_factory=list) + high_confidence_warnings: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + low_confidence_warnings: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + + def is_clean(self) -> bool: + """:return: True iff no stale references and no warnings of any confidence level were found.""" + return not (self.stale_references or self.high_confidence_warnings or self.low_confidence_warnings) + + def format(self) -> str: + """:return: a human-readable rendering suitable for CLI display.""" + tb = TextBuilder() + if self.is_clean(): + tb.with_line("✓ No referential integrity issues found.") + return tb.build() + + # stale references — grouped by writable / read-only for clarity + if self.stale_references: + tb.with_line(f"Stale references ({len(self.stale_references)}):") + for ref in self.stale_references: + ro_tag = " [read-only source]" if ref.source_is_read_only else "" + tb.with_line(f" - `mem:{ref.referenced_name}` in `{ref.source_memory}`{ro_tag}") + if ref.candidates: + candidates_str = ", ".join(f"`mem:{c}`" for c in ref.candidates) + tb.with_line(f" candidates: {candidates_str}") + else: + tb.with_line(" candidates: (none above similarity threshold)") + tb.with_line("") + + # unmarked-reference warnings, split by confidence + for label, warnings in ( + ("High-confidence unmarked references", self.high_confidence_warnings), + ("Low-confidence unmarked references", self.low_confidence_warnings), + ): + if not warnings: + continue + tb.with_line(f"{label} ({len(warnings)}):") + for w in warnings: + ro_tag = " [read-only source]" if w.source_is_read_only else "" + occ = "occurrence" if w.occurrences == 1 else "occurrences" + if w.is_exact_match: + tb.with_line( + f" - {w.occurrences} {occ} of `{w.suspected_name}` in `{w.source_memory}`{ro_tag}" + f" (suggested: `mem:{w.suspected_name}`)" + ) + else: + # fuzzy near-miss: the bare text differs from the proposed target + tb.with_line( + f" - {w.occurrences} {occ} of `{w.actual_token}` in `{w.source_memory}`{ro_tag}" + f" — near-miss (suggested: `mem:{w.suspected_name}`)" + ) + tb.with_line("") + return tb.build() + + @dataclass + class AutofixReport: + """ + Outcome of :meth:`MemoriesManager.auto_prefix_bare_references`. + + :ivar autofixed: per-(source, target) records of bare references that were rewritten. + When the call was a dry run, these records describe what *would* have been + written; the files themselves are unchanged. + :ivar dry_run: True if the run was a preview that did not write any files. + :ivar skipped_read_only: warnings whose source memory was read-only and therefore not + modified (only populated when ``include_read_only`` is False). + :ivar skipped_flat: warnings skipped because their suspected target had no ``/`` + separator and was not long enough to be high-confidence (only populated when + ``include_flat_names`` is False). + :ivar skipped_global: warnings skipped because their source memory was global and + ``include_global`` was False. + :ivar skipped_fuzzy: warnings whose bare text in the source memory differs from the + suspected target (fuzzy near-misses). These require substring substitution rather + than a prefix addition and are never autofixed; surface them to the user for + manual review instead. + """ + + autofixed: list["MemoriesManager.AutofixedReference"] = field(default_factory=list) + dry_run: bool = False + skipped_read_only: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + skipped_flat: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + skipped_global: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + skipped_fuzzy: list["MemoriesManager.UnmarkedReferenceWarning"] = field(default_factory=list) + + @property + def total_replacements(self) -> int: + return sum(a.n_replacements for a in self.autofixed) + + def format(self) -> str: + """:return: a human-readable rendering suitable for CLI display.""" + tb = TextBuilder() + verb = "Would apply" if self.dry_run else "Applied" + if self.autofixed: + tb.with_line( + f"{verb} {self.total_replacements} replacement(s) across " + f"{len(self.autofixed)} memory/target pair(s)" + (" (dry run; no files were modified)" if self.dry_run else "") + ":" + ) + for a in self.autofixed: + tb.with_line(f" - {a.n_replacements} x `{a.referenced_name}` -> `mem:{a.referenced_name}` in `{a.source_memory}`") + tb.with_line("") + else: + tb.with_line("No replacements" + (" would be" if self.dry_run else "") + " applied.") + tb.with_line("") + + for label, items in ( + ("Skipped (read-only source; pass --include-read-only to override)", self.skipped_read_only), + ("Skipped (flat name; pass --include-flat-names to include)", self.skipped_flat), + ("Skipped (global memory; pass --include-global to include)", self.skipped_global), + ( + "Skipped (fuzzy near-miss; bare text differs from target name — not safe to auto-rewrite, review manually)", + self.skipped_fuzzy, + ), + ): + if not items: + continue + tb.with_line(f"{label}:") + for w in items: + if w.is_exact_match: + tb.with_line(f" - `{w.suspected_name}` in `{w.source_memory}`") + else: + tb.with_line(f" - `{w.actual_token}` in `{w.source_memory}` (suggested: `mem:{w.suspected_name}`)") + tb.with_line("") + return tb.build() + def _list_memories(self, search_dir: Path, base_dir: Path, prefix: str = "") -> MemoriesList: result = self.MemoriesList() if not search_dir.exists(): @@ -309,6 +703,238 @@ class MemoriesManager: f.write(updated_content) return f"Memory {name} edited successfully." + def _is_self_reference(self, source_memory: str, suspected_name: str) -> bool: + """ + :return: True iff ``suspected_name`` would refer to ``source_memory`` itself + (including the case where ``suspected_name`` equals the basename of a topic-path + source memory). + """ + if suspected_name == source_memory: + return True + source_basename = source_memory.rsplit("/", 1)[-1] + return suspected_name == source_basename + + def validate_referential_integrity(self) -> "MemoriesManager.ReferentialIntegrityReport": + """ + Scans every (non-ignored) memory's content for referential integrity issues. + + The scan covers both project-local and global memories. Three kinds of finding are + produced: + + * **stale references** — occurrences of ``mem:NAME`` where ``NAME`` does not resolve + to an existing memory. For each, a list of similarly-named existing memories is + proposed as candidate intended targets (see :meth:`compute_name_similarity`). + * **exact unmarked-reference warnings** — bare occurrences of an existing memory + name that appear without the ``mem:`` prefix. Split into high-confidence (the + suspected name contains a ``/`` or exceeds :attr:`_HIGH_CONFIDENCE_NAME_LENGTH`) + and low-confidence groups. + * **fuzzy near-miss warnings** — long, distinctive bare tokens in a memory body + that *do not* match an existing name exactly but similarity-match a + high-confidence existing name AND share at least :attr:`_FUZZY_BARE_TOKEN_JACCARD_FLOOR` + of their tokenized name parts (e.g. ``adding_new_language_support`` in prose when + the memory is named ``adding_new_language_support_guide``). Always reported as + high-confidence; the actual bare text is preserved on + :attr:`UnmarkedReferenceWarning.actual_token`. + + Self-references (a memory's content mentioning its own name or basename) and empty + memories are skipped silently. + + This method has no side effects. + + :return: a :class:`ReferentialIntegrityReport` summarizing all findings. + """ + report = MemoriesManager.ReferentialIntegrityReport() + + # gather all existing memory names (project + global) with their read-only status + memories_list = self.list_memories() + all_names = sorted(set(memories_list.memories) | set(memories_list.read_only_memories)) + read_only_names = set(memories_list.read_only_memories) + existing_set = set(all_names) + # only existing names that are themselves "memory-name-like" qualify as fuzzy candidates, + # otherwise short flat names would match arbitrary long prose words via containment + high_confidence_names = [n for n in all_names if "/" in n or len(n) >= self._HIGH_CONFIDENCE_NAME_LENGTH] + + for source_name in all_names: + content = self.load_memory(source_name) + if not content: + # skip empty memories silently — no references can exist in empty content + continue + source_is_read_only = source_name in read_only_names + + # stale references: every mem:NAME whose NAME is not an existing memory + stale_seen: set[str] = set() + for referenced in self._iter_referenced_names_in_content(content): + if referenced in existing_set or referenced in stale_seen: + continue + stale_seen.add(referenced) + candidates = self._find_stale_reference_candidates(referenced, all_names) + report.stale_references.append( + MemoriesManager.StaleReference( + source_memory=source_name, + referenced_name=referenced, + candidates=candidates, + source_is_read_only=source_is_read_only, + ) + ) + + # exact unmarked references: bare occurrences of any existing memory name (except self) + for candidate_name in all_names: + if self._is_self_reference(source_name, candidate_name): + continue + n = self._find_bare_occurrences(content, candidate_name) + if n == 0: + continue + is_high_conf = "/" in candidate_name or len(candidate_name) >= self._HIGH_CONFIDENCE_NAME_LENGTH + warning = MemoriesManager.UnmarkedReferenceWarning( + source_memory=source_name, + suspected_name=candidate_name, + occurrences=n, + is_high_confidence=is_high_conf, + source_is_read_only=source_is_read_only, + actual_token=candidate_name, + ) + if is_high_conf: + report.high_confidence_warnings.append(warning) + else: + report.low_confidence_warnings.append(warning) + + # fuzzy near-misses: long bare tokens that similarity-match a high-confidence existing name + for token, n in self._iter_long_bare_tokens(content): + if token in existing_set: + # exact-match path already handled this token (or self-referenced and was skipped) + continue + token_tokens = self._tokenize_name(token) + best_name: str | None = None + best_score = 0.0 + for existing in high_confidence_names: + if self._is_self_reference(source_name, existing): + continue + score = self.compute_name_similarity(token, existing) + if score < self.NAME_SIMILARITY_THRESHOLD: + continue + # safeguard against substring-only matches: require meaningful token overlap so + # generic prose words don't get flagged just because they happen to be substrings + # of an existing memory name + existing_tokens = self._tokenize_name(existing) + union = token_tokens | existing_tokens + if not union: + continue + jaccard = len(token_tokens & existing_tokens) / len(union) + if jaccard < self._FUZZY_BARE_TOKEN_JACCARD_FLOOR: + continue + if score > best_score: + best_score = score + best_name = existing + if best_name is None: + continue + report.high_confidence_warnings.append( + MemoriesManager.UnmarkedReferenceWarning( + source_memory=source_name, + suspected_name=best_name, + occurrences=n, + is_high_confidence=True, + source_is_read_only=source_is_read_only, + actual_token=token, + ) + ) + + return report + + def auto_prefix_bare_references( + self, + include_flat_names: bool = False, + include_read_only: bool = False, + include_global: bool = False, + dry_run: bool = False, + ) -> "MemoriesManager.AutofixReport": + """ + Rewrites *exact* bare occurrences of existing memory names by adding the ``mem:`` prefix. + + .. warning:: + **This is a heuristic, file-mutating operation** (unless ``dry_run`` is True). A + bare word that happens to coincide with a memory name will be rewritten as a + reference, even if it was intended as ordinary prose. Pass ``dry_run=True`` to + preview the rewrites before applying them. + + Scope is intentionally narrower than what :meth:`validate_referential_integrity` + reports: + + * Only **exact** bare occurrences are rewritten — i.e. the bare text in the source + body must equal an existing memory name verbatim. Fuzzy near-miss findings + (where the actual token differs from the suspected target) require substring + substitution rather than a prefix addition and are routed into + :attr:`AutofixReport.skipped_fuzzy` for manual review. + * By default the rewrite is restricted to *high-confidence* findings only — those + whose suspected target name contains a ``/`` separator or exceeds the + configured length threshold — and skips global memories and read-only memories. + The defaults intentionally err toward false negatives over false positives. + + :param include_flat_names: if True, also rewrite low-confidence findings (flat, + short memory names). Increases recall but markedly raises false-positive risk. + :param include_read_only: if True, also rewrite occurrences inside read-only + memories. Use with care, as read-only memories are typically considered + authoritative. + :param include_global: if True, also rewrite occurrences inside global memories. + Modifying a global memory affects every project that consumes it. + :param dry_run: if True, the report describes the rewrites that *would* be applied + but no files are modified. + :return: an :class:`AutofixReport` describing every replacement (made or planned) + and every warning that was deliberately skipped. + """ + report = MemoriesManager.AutofixReport(dry_run=dry_run) + + # build the integrity report first so autofix decisions are made on the same data + validation = self.validate_referential_integrity() + + # combine the warning groups according to the include_flat_names policy + warnings: list[MemoriesManager.UnmarkedReferenceWarning] = list(validation.high_confidence_warnings) + if include_flat_names: + warnings.extend(validation.low_confidence_warnings) + else: + report.skipped_flat.extend(validation.low_confidence_warnings) + + # apply replacements per source memory, grouping warnings to avoid re-reading content + warnings_by_source: dict[str, list[MemoriesManager.UnmarkedReferenceWarning]] = {} + for w in warnings: + # fuzzy near-misses are never auto-rewritten — the bare text differs from the target + if not w.is_exact_match: + report.skipped_fuzzy.append(w) + continue + # filter remaining warnings according to scope flags + if w.source_is_read_only and not include_read_only: + report.skipped_read_only.append(w) + continue + if self._is_global(w.source_memory) and not include_global: + report.skipped_global.append(w) + continue + warnings_by_source.setdefault(w.source_memory, []).append(w) + + for source_memory, source_warnings in warnings_by_source.items(): + content = self.load_memory(source_memory) + total_n = 0 + per_target_records: list[MemoriesManager.AutofixedReference] = [] + + # apply replacements sequentially; each pass uses the (potentially updated) content + for w in source_warnings: + content, n = self._add_bare_occurrences_prefix(content, w.suspected_name) + if n > 0: + per_target_records.append( + MemoriesManager.AutofixedReference( + source_memory=source_memory, + referenced_name=w.suspected_name, + n_replacements=n, + ) + ) + total_n += n + + if total_n > 0: + if not dry_run: + # use is_tool_context=False so we don't trip read-only protection when --include-read-only is set + self.save_memory(source_memory, content, is_tool_context=False) + report.autofixed.extend(per_target_records) + + return report + class Project(ToStringMixin): def __init__( diff --git a/test/serena/test_memories_manager.py b/test/serena/test_memories_manager.py index a862846c..991feb98 100644 --- a/test/serena/test_memories_manager.py +++ b/test/serena/test_memories_manager.py @@ -154,3 +154,350 @@ class TestRenameReferencesToMemory: result, n = manager.rename_references_to_memory(content, "foo", "bar") assert result == f"{left}mem:bar{right}" assert n == 1 + + +class TestNameSimilarity: + @pytest.mark.parametrize( + ("a", "b", "expected"), + [ + # exact match (and case-insensitive) -> 1.0 + pytest.param("foo", "foo", 1.0, id="identical"), + pytest.param("Foo", "foo", 1.0, id="case_insensitive"), + # version-suffix renames are normalized away -> 1.0 + pytest.param("auth_v1", "auth_v2", 1.0, id="version_suffix_rename"), + pytest.param("auth_old", "auth", 1.0, id="old_suffix_normalized"), + pytest.param("auth_legacy", "auth", 1.0, id="legacy_suffix_normalized"), + # flat <-> topic basename move: canonical rename, fully confident + pytest.param("a/b/c/login", "login", 1.0, id="topic_move_to_flat"), + pytest.param("login", "auth/login", 1.0, id="flat_to_topic_move"), + # both sides share a prefix token AND basename: high confidence + pytest.param("auth/login", "auth/v2/login", 0.75, id="basename_match_shared_prefix"), + ], + ) + def test_high_similarity_cases(self, manager: MemoriesManager, a: str, b: str, expected: float) -> None: + assert manager.compute_name_similarity(a, b) == pytest.approx(expected) + + @pytest.mark.parametrize( + ("a", "b"), + [ + # completely unrelated names should be below threshold + pytest.param("authentication", "database", id="unrelated_long"), + pytest.param("api/users", "logging/handlers", id="unrelated_topic_paths"), + # short names without strong token signals must hit the short-name floor + pytest.param("foo", "bar", id="short_names_unrelated"), + pytest.param("foo", "for", id="short_names_typo_distance"), + # IMPORTANT: same basename under unrelated topic prefixes is NOT a rename. + # `auth/login` and `security/login` are two different login flows in different + # subsystems; `topic1/topic2/core` and `topic3/topic5/core` are unrelated. + pytest.param("auth/login", "security/login", id="same_basename_unrelated_prefixes"), + pytest.param("topic1/topic2/core", "topic3/topic5/core", id="same_basename_disjoint_prefix_tokens"), + ], + ) + def test_below_threshold_cases(self, manager: MemoriesManager, a: str, b: str) -> None: + assert manager.compute_name_similarity(a, b) < MemoriesManager.NAME_SIMILARITY_THRESHOLD + + @pytest.mark.parametrize( + ("a", "b"), + [ + # prefix/suffix extensions: caught by sequence ratio / containment, even though Jaccard is 0 + pytest.param("auth", "authentication", id="prefix_extension"), + pytest.param("login", "user_login", id="token_extension"), + # typos on longer names: caught by sequence ratio + pytest.param("authentication", "autentication", id="typo_in_long_name"), + # token reorder / partial overlap + pytest.param("user_login_flow", "login_flow", id="partial_token_overlap"), + ], + ) + def test_above_threshold_cases(self, manager: MemoriesManager, a: str, b: str) -> None: + assert manager.compute_name_similarity(a, b) >= MemoriesManager.NAME_SIMILARITY_THRESHOLD + + def test_symmetry(self, manager: MemoriesManager) -> None: + # similarity is order-independent + assert manager.compute_name_similarity("auth/login", "security/login") == manager.compute_name_similarity( + "security/login", "auth/login" + ) + + def test_find_candidates_ranks_by_descending_similarity(self, manager: MemoriesManager) -> None: + existing = ["auth/login", "security/login", "database", "authentication"] + candidates = manager._find_stale_reference_candidates("login", existing) + # both auth/login and security/login share basename `login` -> tied at 1.0; expect them first, alphabetical tiebreak + assert candidates[:2] == ["auth/login", "security/login"] + # `database` is unrelated and must not appear + assert "database" not in candidates + + def test_find_candidates_returns_empty_below_threshold(self, manager: MemoriesManager) -> None: + existing = ["completely_unrelated", "another_thing"] + assert manager._find_stale_reference_candidates("foo", existing) == [] + + def test_find_candidates_custom_threshold(self, manager: MemoriesManager) -> None: + # raising the threshold filters out borderline matches + existing = ["authentication"] + loose = manager._find_stale_reference_candidates("auth", existing, threshold=0.2) + strict = manager._find_stale_reference_candidates("auth", existing, threshold=0.95) + assert loose == ["authentication"] + assert strict == [] + + +@pytest.fixture +def fs_manager(tmp_path) -> MemoriesManager: + """A MemoriesManager backed by tmp_path with no read-only patterns.""" + return MemoriesManager(serena_data_folder=tmp_path) + + +def _write(manager: MemoriesManager, name: str, content: str) -> None: + manager.save_memory(name, content, is_tool_context=False) + + +class TestValidateReferentialIntegrity: + def test_clean_report_when_all_references_resolve(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth", "# auth notes") + _write(fs_manager, "login_flow", "see `mem:auth` for details") + report = fs_manager.validate_referential_integrity() + assert report.is_clean() + assert "No referential integrity issues" in report.format() + + def test_stale_reference_with_topic_move_candidate(self, fs_manager: MemoriesManager) -> None: + # original memory `login` was moved to `auth/login`; a reference still points to bare `login` + _write(fs_manager, "auth/login", "# moved here") + _write(fs_manager, "docs", "old reference: `mem:login`") + report = fs_manager.validate_referential_integrity() + assert len(report.stale_references) == 1 + stale = report.stale_references[0] + assert stale.source_memory == "docs" + assert stale.referenced_name == "login" + assert "auth/login" in stale.candidates + + def test_stale_reference_with_no_candidate(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "completely_unrelated", "# content") + _write(fs_manager, "docs", "broken: `mem:xyz_unknown_thing`") + report = fs_manager.validate_referential_integrity() + assert len(report.stale_references) == 1 + assert report.stale_references[0].candidates == [] + + def test_high_confidence_unmarked_warning_for_topic_path(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "the auth/login process is documented elsewhere") + report = fs_manager.validate_referential_integrity() + assert len(report.high_confidence_warnings) == 1 + warning = report.high_confidence_warnings[0] + assert warning.suspected_name == "auth/login" + assert warning.is_high_confidence is True + assert warning.occurrences == 1 + assert report.low_confidence_warnings == [] + + def test_low_confidence_unmarked_warning_for_short_flat_name(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth", "# notes") + _write(fs_manager, "docs", "you must use auth here") + report = fs_manager.validate_referential_integrity() + assert report.high_confidence_warnings == [] + assert len(report.low_confidence_warnings) == 1 + assert report.low_confidence_warnings[0].suspected_name == "auth" + assert report.low_confidence_warnings[0].is_high_confidence is False + + def test_long_flat_name_is_high_confidence(self, fs_manager: MemoriesManager) -> None: + # a flat name >= _HIGH_CONFIDENCE_NAME_LENGTH chars is unlikely to be coincidental prose + long_name = "authentication_flow" + assert len(long_name) >= MemoriesManager._HIGH_CONFIDENCE_NAME_LENGTH + _write(fs_manager, long_name, "# notes") + _write(fs_manager, "docs", f"the {long_name} is documented here") + report = fs_manager.validate_referential_integrity() + assert len(report.high_confidence_warnings) == 1 + + def test_fuzzy_near_miss_rejects_substring_only_matches(self, fs_manager: MemoriesManager) -> None: + """ + Regression: fuzzy bare-text matching must require meaningful token overlap, not just + substring containment. Without the token-Jaccard floor, generic words like + ``Repository`` would be flagged as near-misses for ``serena_repository_structure`` + purely because they appear as substrings — a noise flood. The cases below are taken + from a real run on the Serena codebase that produced six false positives. + """ + _write(fs_manager, "serena_repository_structure", "# repo structure notes") + _write(fs_manager, "suggested_commands", "# command notes") + _write( + fs_manager, + "docs", + # generic words ("Repository", "repository") and unrelated code identifiers + # ("_create_launch_command", "create_launch_command", "repository_root_path") + # that happen to be substrings of, or share one token with, an existing memory name + """\ + See _create_launch_command and also create_launch_command. + The Repository pattern is documented elsewhere. + Also repository_root_path is set via env var. + The repository contains many tools. + """, + ) + report = fs_manager.validate_referential_integrity() + assert report.is_clean(), ( + f"Expected no findings; got high={report.high_confidence_warnings}, " + f"low={report.low_confidence_warnings}, stale={report.stale_references}" + ) + + def test_long_flat_name_near_miss_is_high_confidence(self, fs_manager: MemoriesManager) -> None: + """ + Regression: bare text that is a *near-match* of an existing long memory name + (e.g. the same name with a missing suffix) should be flagged as a high-confidence + unmarked-reference warning. Long, distinctive multi-word tokens that appear bare in + prose and similarity-match an existing memory are almost certainly intended references. + + Real-world case: ``serena_repository_structure.md`` mentions + ``adding_new_language_support`` in prose; the actual memory is + ``adding_new_language_support_guide``. Exact-substring matching alone misses this. + """ + existing = "adding_new_language_support_guide" + bare = "adding_new_language_support" # prefix of `existing`; clearly the same concept + assert len(bare) >= MemoriesManager._HIGH_CONFIDENCE_NAME_LENGTH + _write(fs_manager, existing, "# guide content") + _write(fs_manager, "docs", f"See {bare} for details") + + report = fs_manager.validate_referential_integrity() + assert len(report.high_confidence_warnings) == 1, ( + f"expected one high-confidence warning for the near-miss reference, " + f"got {len(report.high_confidence_warnings)} " + f"(high={report.high_confidence_warnings}, low={report.low_confidence_warnings})" + ) + warning = report.high_confidence_warnings[0] + assert warning.suspected_name == existing + assert warning.source_memory == "docs" + + def test_self_reference_is_ignored(self, fs_manager: MemoriesManager) -> None: + # the basename `login` appears in `auth/login`'s own content -> must not be flagged + _write(fs_manager, "auth/login", "this login flow ...") + _write(fs_manager, "auth", "# auth notes") + report = fs_manager.validate_referential_integrity() + # `auth/login` mentioning its own basename `login` should not produce a warning; + # `auth` mentioned in `auth/login` content also shouldn't (it's the prefix), and only + # actual matches matter — assert no warning concerns the source itself + for w in report.high_confidence_warnings + report.low_confidence_warnings: + assert not (w.source_memory == "auth/login" and w.suspected_name in ("login", "auth/login")) + + def test_marked_reference_does_not_produce_unmarked_warning(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "see `mem:auth/login` for details") + report = fs_manager.validate_referential_integrity() + # the marked reference is valid; no warning should fire on that occurrence + assert report.is_clean() + + def test_mixed_marked_and_bare_in_same_content(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "we have `mem:auth/login` plus a bare auth/login mention") + report = fs_manager.validate_referential_integrity() + # only the bare occurrence should be counted + assert len(report.high_confidence_warnings) == 1 + assert report.high_confidence_warnings[0].occurrences == 1 + + def test_empty_memory_skipped(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "") + _write(fs_manager, "docs", "# nothing relevant") + # must not raise even though one memory is empty + report = fs_manager.validate_referential_integrity() + assert report.is_clean() + + def test_read_only_status_propagated(self, tmp_path) -> None: + manager = MemoriesManager(serena_data_folder=tmp_path, read_only_memory_patterns=[r"frozen/.*"]) + _write(manager, "frozen/notes", "see bare auth/login mention") + _write(manager, "auth/login", "# notes") + report = manager.validate_referential_integrity() + warnings = report.high_confidence_warnings + assert len(warnings) == 1 + assert warnings[0].source_memory == "frozen/notes" + assert warnings[0].source_is_read_only is True + + +class TestAutoPrefixBareReferences: + def test_prefixes_high_confidence_warning_in_writable_memory(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "the auth/login process is documented here") + result = fs_manager.auto_prefix_bare_references() + assert result.total_replacements == 1 + assert len(result.autofixed) == 1 + # verify the file content was actually mutated + assert fs_manager.load_memory("docs") == "the mem:auth/login process is documented here" + + def test_skips_low_confidence_by_default(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth", "# notes") + _write(fs_manager, "docs", "use auth here") + result = fs_manager.auto_prefix_bare_references() + assert result.total_replacements == 0 + assert len(result.skipped_flat) == 1 + # content unchanged + assert fs_manager.load_memory("docs") == "use auth here" + + def test_include_flat_names_widens_scope(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth", "# notes") + _write(fs_manager, "docs", "use auth here") + result = fs_manager.auto_prefix_bare_references(include_flat_names=True) + assert result.total_replacements == 1 + assert fs_manager.load_memory("docs") == "use mem:auth here" + + def test_skips_read_only_source_by_default(self, tmp_path) -> None: + manager = MemoriesManager(serena_data_folder=tmp_path, read_only_memory_patterns=[r"frozen/.*"]) + _write(manager, "frozen/notes", "the auth/login process") + _write(manager, "auth/login", "# notes") + result = manager.auto_prefix_bare_references() + assert result.total_replacements == 0 + assert len(result.skipped_read_only) == 1 + # content unchanged + assert manager.load_memory("frozen/notes") == "the auth/login process" + + def test_include_read_only_modifies_read_only_source(self, tmp_path) -> None: + manager = MemoriesManager(serena_data_folder=tmp_path, read_only_memory_patterns=[r"frozen/.*"]) + _write(manager, "frozen/notes", "the auth/login process") + _write(manager, "auth/login", "# notes") + result = manager.auto_prefix_bare_references(include_read_only=True) + assert result.total_replacements == 1 + assert manager.load_memory("frozen/notes") == "the mem:auth/login process" + + def test_multiple_occurrences_in_same_memory(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "auth/login here and auth/login there") + result = fs_manager.auto_prefix_bare_references() + assert result.total_replacements == 2 + assert fs_manager.load_memory("docs") == "mem:auth/login here and mem:auth/login there" + + def test_marked_references_left_alone(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "see `mem:auth/login` and also auth/login bare") + result = fs_manager.auto_prefix_bare_references() + # only the bare occurrence is rewritten; the already-marked one stays as-is + assert result.total_replacements == 1 + assert fs_manager.load_memory("docs") == "see `mem:auth/login` and also mem:auth/login bare" + + def test_dry_run_does_not_modify_files(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "the auth/login process is documented here") + result = fs_manager.auto_prefix_bare_references(dry_run=True) + # report describes what *would* have been done + assert result.dry_run is True + assert result.total_replacements == 1 + assert len(result.autofixed) == 1 + # but the file on disk is unchanged + assert fs_manager.load_memory("docs") == "the auth/login process is documented here" + # running again non-dry actually rewrites + result = fs_manager.auto_prefix_bare_references() + assert result.dry_run is False + assert fs_manager.load_memory("docs") == "the mem:auth/login process is documented here" + + def test_fuzzy_warnings_are_routed_to_skipped_fuzzy(self, fs_manager: MemoriesManager) -> None: + # set up a fuzzy near-miss case (token shares all parts with a longer existing name) + existing = "adding_new_language_support_guide" + bare = "adding_new_language_support" + _write(fs_manager, existing, "# guide content") + _write(fs_manager, "docs", f"See {bare} for details") + result = fs_manager.auto_prefix_bare_references() + # fuzzy match must NOT be auto-rewritten — it would require substring substitution + assert result.total_replacements == 0 + assert len(result.skipped_fuzzy) == 1 + skipped = result.skipped_fuzzy[0] + assert skipped.actual_token == bare + assert skipped.suspected_name == existing + # source unchanged + assert fs_manager.load_memory("docs") == f"See {bare} for details" + + def test_no_double_prefix_on_repeated_runs(self, fs_manager: MemoriesManager) -> None: + _write(fs_manager, "auth/login", "# notes") + _write(fs_manager, "docs", "the auth/login process") + fs_manager.auto_prefix_bare_references() + second = fs_manager.auto_prefix_bare_references() + # idempotent: the second run should not touch anything + assert second.total_replacements == 0 + assert fs_manager.load_memory("docs") == "the mem:auth/login process"