emrg: the filename a memory reports is the file it is in

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 ``<slug>-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

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 ``<slug>-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"