diff --git a/emrg/memory.py b/emrg/memory.py index 9dcaafc4..da14705f 100644 --- a/emrg/memory.py +++ b/emrg/memory.py @@ -271,13 +271,27 @@ class MemoryFile: _fm_lines: ClassVar[tuple[str, ...] | None] = None _fm_parsed: ClassVar[dict[str, object]] = {} + # The file this memory actually lives in, when that is known. ``filename`` + # derives a name from ``type`` + ``title``, which is not the name a memory + # gets when its title collides with one already on disk: the store writes + # the second one to ``-2.md``. Everything keyed by filename — the + # index rows (``MemoryIndex.add_entry`` identifies entries by it), the + # server's memory frames, a load → save — has to name the file the memory + # is really in, or a row lands on the wrong memory and the written file is + # referenced by nothing at all. Not part of the file format: ``to_markdown`` + # renders frontmatter from ``_canonical_frontmatter`` only, and this is not + # in it. + _filename: Optional[str] = field(default=None, repr=False, compare=False) + @property def filename(self) -> str: - """Derive a descriptive filename from title. + """The name of the file this memory is in, or the one its title derives. Does NOT include the id — the id lives in frontmatter only. This keeps filenames human-readable. """ + if self._filename: + return self._filename prefix = f"{self.type}-" if self.type != "reference" else "" slug = slugify(self.title) return f"{prefix}{slug}.md" @@ -403,6 +417,13 @@ def from_text(cls, text: str, _filename: str = "") -> MemoryFile: status=frontmatter.get("status") or "active", title=title, body=body, + # A memory read from a file knows which file it came from — the only + # moment that name is available, and the name every reader that + # builds a path from it (the server's memory frame, an index row, an + # update's load → save) has to agree with. Without it a load + # re-derives the name from the title, which for a counter-suffixed + # file names a different memory's file. + _filename=_filename or None, ) # Provenance for the write side: the lines as read, and the value each # key parsed to, so ``to_markdown`` can tell what it changed. @@ -995,21 +1016,14 @@ def create( filename = self._resolve_filename(mem) filepath = self.directory / filename - # Re-create with resolved filename (so to_markdown + index match) - mem = MemoryFile( - id=mem.id, - event_at=mem.event_at, - created_at=mem.created_at, - updated_at=mem.updated_at, - source_session=mem.source_session, - type=mem.type, - scope=mem.scope, - status=mem.status, - title=mem.title, - body=mem.body, - ) - # Override filename for this instance - object.__setattr__(mem, "_filename", filename) + # Name the file this memory is in, so the index row, this instance and + # every later reader agree with the write. (This used to be an + # ``object.__setattr__(mem, "_filename", filename)`` against a property + # with no setter: a property is a data descriptor, so it won over the + # instance attribute and the override never took effect — the second + # memory of a duplicate title got a ``-2.md`` file and no index row, + # and reported the first memory's filename.) + mem._filename = filename mem.save(filepath) # Update index @@ -1205,6 +1219,12 @@ def promote_to_project( new_path = project_store.directory / f"{stem}-{counter}.md" counter += 1 + # The copy lives at ``new_path``, not at the name its title derives — + # and on a collision those two differ. The project index keys its rows + # by filename, so a row added under the derived name would attach + # itself to the resident memory of that title and leave this copy + # referenced by nothing. + mem._filename = new_path.name mem.save(new_path) # Update project index diff --git a/tests/test_memory.py b/tests/test_memory.py index c6133d91..f45d5bf1 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -666,3 +666,106 @@ def test_a_row_over_the_cap_is_rewritten_and_its_excess_is_gone(self, session_st assert len(row[0]) <= INDEX_TITLE_MAX_CHARS, "the rewritten row still exceeds the cap" assert "big.md" in row[0], "the rewritten row lost the link to its detail file" assert "Small" in after, "the in-cap sibling row was dropped as well" + + +class TestTwoMemoriesWithTheSameTitle: + """A duplicate title writes a counter-suffixed file; the index has to follow. + + Issue #1801. ``MemoryFile.filename`` derives its name from ``type`` + + ``title``, and ``MemoryIndex.add_entry`` identifies rows *by filename*, so + a memory written to ``-2.md`` must report that name or its row lands + on the resident memory of that title and its own file is referenced by + nothing — permanent, because ``_rebuild_index`` derives the name the same + way. Measured before the fix: two files, one row, one orphan. + """ + + def _rows(self, store): + return MemoryIndex.from_text(store.index_path.read_text(encoding="utf-8")).entries + + def _files(self, store): + return sorted(p.name for p in store.directory.glob("*.md") if p.name != "MEMORY.md") + + def test_create_gives_each_memory_its_own_row(self, project_store): + first = project_store.create("project", "Same title", "first") + second = project_store.create("project", "Same title", "second") + + assert self._files(project_store) == ["project-same-title-2.md", "project-same-title.md"] + assert first.filename != second.filename, "both memories claim the same file" + for mem in (first, second): + assert (project_store.directory / mem.filename).exists(), mem.filename + + entries = self._rows(project_store) + assert sorted(e.filename for e in entries) == [ + "project-same-title-2.md", + "project-same-title.md", + ], "each file needs a row of its own; neither may be orphaned" + + def test_each_row_opens_the_memory_that_wrote_it(self, project_store): + first = project_store.create("project", "Same title", "first body") + second = project_store.create("project", "Same title", "second body") + + assert first.id != second.id + for mem in (first, second): + back = project_store.get_by_filename(mem.filename) + assert back is not None and back.id == mem.id, ( + f"{mem.filename} does not open the memory that wrote it" + ) + + def test_a_memory_read_from_disk_reports_its_own_file(self, project_store): + project_store.create("project", "Same title", "first") + second = project_store.create("project", "Same title", "second") + + loaded = {m.id: m for m in project_store.list()} + assert loaded[second.id].filename == second.filename, ( + "a reload re-derived the name from the title, so two memories " + "report one file" + ) + + def test_rebuild_index_keeps_the_counter_suffixed_memory(self, project_store): + project_store.create("project", "Same title", "first") + project_store.create("project", "Same title", "second") + + project_store.rebuild_index() + + assert sorted(e.filename for e in self._rows(project_store)) == [ + "project-same-title-2.md", + "project-same-title.md", + ], "a rebuild re-derived the name and dropped the suffixed memory again" + + def test_update_of_the_suffixed_memory_keeps_its_row(self, project_store): + project_store.create("project", "Same title", "first") + second = project_store.create("project", "Same title", "second") + + updated = project_store.update(second.id, body="second, edited") + + assert updated.filename == second.filename + assert sorted(e.filename for e in self._rows(project_store)) == [ + "project-same-title-2.md", + "project-same-title.md", + ] + + def test_promote_with_a_colliding_title_keeps_its_row(self, session_store, temp_cwd): + project = ProjectMemoryStore(temp_cwd) + project.create("project", "Collide", "already resident") + mem = session_store.create("project", "Collide", "from the session") + + promoted = session_store.promote_to_project(mem.id, project) + + assert promoted is not None + assert promoted.filename != mem.filename, "the copy claims the resident memory's file" + assert promoted.filename.endswith("-collide-1.md"), promoted.filename + assert (project.directory / promoted.filename).exists() + assert sorted(e.filename for e in self._rows(project)) == sorted( + ["project-collide-1.md", "project-collide.md"] + ), "the promoted copy's row must name the file it was written to" + + def test_the_filename_field_stays_out_of_the_file_format(self, project_store): + mem = project_store.create("project", "Same title", "first") + project_store.create("project", "Same title", "second") + + text = (project_store.directory / mem.filename).read_text(encoding="utf-8") + assert "_filename" not in text, "the override leaked into the serialized memory" + + reloaded = MemoryFile.from_file(project_store.directory / mem.filename) + assert reloaded._filename == mem.filename + assert reloaded.to_markdown() == text, "a load → save rewrote the file"