fix(search): keep colliding observation permalinks searchable and clean up index rows by entity id

Observation permalinks truncate content to 200 chars, so two distinct
observations sharing a category and 200-char prefix produced identical
synthetic permalinks and the second row was silently dropped from the
search index, making it unfindable.

- index_entity_markdown now disambiguates colliding observation
  permalinks by appending "-{obs.id}" (in a loop, so a suffixed
  permalink that itself collides is re-suffixed until unique)
- SearchService.handle_delete and SyncService.handle_delete now clean
  up by entity_id instead of recomputing synthetic permalinks, which
  would miss disambiguated rows and leave ghost search results;
  incoming-relation rows (indexed under the source entity) are still
  removed by permalink
- regression tests at unit (SearchService), sync (SyncService), and MCP
  integration levels, verified failing against the old code on both
  SQLite and Postgres

Fixes #909

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: groksrc

src/basic_memory/services/search_service.py

@@ -884,9 +884,21 @@ async def index_entity_markdown(
             seen_permalinks: set[str] = {entity.permalink} if entity.permalink else set()
             for obs in entity.observations:
                 obs_permalink = obs.permalink
-                if obs_permalink in seen_permalinks:
-                    logger.debug(f"Skipping duplicate observation permalink: {obs_permalink}")
-                    continue
+                # Trigger: observation permalinks truncate content to 200 chars (Postgres
+                # btree index limit), so distinct observations sharing a category and a
+                # 200-char content prefix produce identical synthetic permalinks.
+                # Why: permalinks must stay unique within the batch — the Postgres bulk
+                # upsert conflicts on (permalink, project_id) — but dropping the row made
+                # the second observation unsearchable (#909).
+                # Outcome: append the observation's unique DB id so every observation is
+                # indexed and remains findable. A while (not if) re-checks the suffixed
+                # permalink too: content ending in "-{id}" can make it collide with
+                # another observation's natural permalink in the same batch. Each pass
+                # appends "-{obs.id}", strictly lengthening the permalink against a
+                # finite seen set, so the loop terminates.
+                while obs_permalink in seen_permalinks:
+                    logger.debug(f"Disambiguating duplicate observation permalink: {obs_permalink}")
+                    obs_permalink = f"{obs_permalink}-{obs.id}"
                 seen_permalinks.add(obs_permalink)
 
                 obs_content_stems = _strip_nul(
@@ -965,23 +977,14 @@ async def handle_delete(self, entity: Entity):
             f"observations={len(entity.observations)}, relations={len(entity.outgoing_relations)}"
         )
 
-        # Clean up search index - same logic as sync_service.handle_delete()
-        permalinks = (
-            [entity.permalink]
-            + [o.permalink for o in entity.observations]
-            + [r.permalink for r in entity.outgoing_relations]
-        )
-
-        logger.debug(
-            f"Deleting search index entries for entity_id={entity.id}, "
-            f"index_entries={len(permalinks)}"
-        )
-
-        for permalink in permalinks:
-            if permalink:
-                await self.delete_by_permalink(permalink)
-            else:
-                await self.delete_by_entity_id(entity.id)
+        # Trigger: every row indexed for this entity (entity, observations, outgoing
+        # relations) carries entity_id, and colliding observation permalinks are
+        # disambiguated at index time (#909 appends "-{obs.id}") so recomputing the
+        # synthetic permalink here would no longer match those rows.
+        # Why: search_index has no FK cascade from entity, so permalink-based cleanup
+        # would leave disambiguated rows behind as ghost search results.
+        # Outcome: delete by entity_id, removing every row regardless of permalink shape.
+        await self.delete_by_entity_id(entity.id)
 
         # Trigger: entity deletion removes the source rows for this note.
         # Why: semantic chunks/embeddings are stored separately from search_index rows.

src/basic_memory/sync/sync_service.py

@@ -1291,23 +1291,26 @@ async def handle_delete(self, file_path: str):
             # Delete from db (this cascades to observations/relations)
             await self.entity_service.delete_entity_by_file_path(file_path)
 
-            # Clean up search index
-            permalinks = (
-                [entity.permalink]
-                + [o.permalink for o in entity.observations]
-                + [r.permalink for r in entity.relations]
-            )
-
             logger.debug(
-                f"Cleaning up search index for entity_id={entity.id}, file_path={file_path}, "
-                f"index_entries={len(permalinks)}"
+                f"Cleaning up search index for entity_id={entity.id}, file_path={file_path}"
             )
 
-            for permalink in permalinks:
-                if permalink:
-                    await self.search_service.delete_by_permalink(permalink)
-                else:
-                    await self.search_service.delete_by_entity_id(entity.id)
+            # Trigger: every row indexed for this entity (entity, observations, outgoing
+            # relations) carries entity_id, and colliding observation permalinks are
+            # disambiguated at index time (#909 appends "-{obs.id}") so recomputing the
+            # synthetic permalink here would no longer match those rows.
+            # Why: search_index has no FK cascade from entity, so permalink-based cleanup
+            # would leave disambiguated rows behind as ghost search results.
+            # Outcome: delete by entity_id, removing every row regardless of permalink shape.
+            await self.search_service.delete_by_entity_id(entity.id)
+
+            # Trigger: incoming relations are indexed under the *source* entity's
+            # entity_id, so the entity-id cleanup above cannot reach their rows.
+            # Why: those rows point at this now-deleted entity; the source entity
+            # re-indexes them as forward references on its next sync.
+            # Outcome: remove them by permalink, preserving prior cleanup behavior.
+            for relation in entity.incoming_relations:
+                await self.search_service.delete_by_permalink(relation.permalink)
 
     async def handle_move(self, old_path, new_path):
         logger.debug("Moving entity", old_path=old_path, new_path=new_path)

test-int/mcp/test_observation_permalink_collision_integration.py

@@ -0,0 +1,136 @@
+"""Regression test for https://github.com/basicmachines-co/basic-memory/issues/909.
+
+Distinct observations that share a category and the first 200 chars of content
+collide on their synthetic permalink (content is truncated to 200 chars for the
+Postgres btree index limit). The second observation was silently dropped from
+the search index, making it unfindable.
+"""
+
+import json
+from typing import Any
+
+import pytest
+from fastmcp import Client
+
+
+def _json_content(tool_result) -> Any:
+    """Parse a FastMCP tool result content block into JSON."""
+    assert len(tool_result.content) == 1
+    assert tool_result.content[0].type == "text"
+    return json.loads(tool_result.content[0].text)  # pyright: ignore [reportAttributeAccessIssue]
+
+
+@pytest.mark.asyncio
+async def test_duplicate_category_content_observations_both_searchable(
+    mcp_server, app, test_project
+):
+    """Both observations must be indexed even when their synthetic permalinks collide."""
+    async with Client(mcp_server) as client:
+        prefix = "x" * 210  # > 200 so truncation makes the permalinks identical
+        content = (
+            "# Dup Obs Note\n\n"
+            "## Observations\n"
+            f"- [note] {prefix} ALPHA_UNIQUE_MARKER\n"
+            f"- [note] {prefix} BETA_UNIQUE_MARKER\n"
+        )
+        await client.call_tool(
+            "write_note",
+            {
+                "project": test_project.name,
+                "title": "Dup Obs Note",
+                "directory": "dup",
+                "content": content,
+            },
+        )
+
+        # Both observations should be independently findable by their unique suffix
+        for marker in ("ALPHA_UNIQUE_MARKER", "BETA_UNIQUE_MARKER"):
+            result = await client.call_tool(
+                "search_notes",
+                {
+                    "project": test_project.name,
+                    "query": marker,
+                    "search_type": "text",
+                    "entity_types": ["observation"],
+                    "output_format": "json",
+                },
+            )
+            data = _json_content(result)
+            snippets = [r.get("content") or "" for r in data["results"]]
+            assert any(marker in s for s in snippets), (
+                f"observation containing {marker} was dropped from the search index "
+                "due to a synthetic-permalink collision (content truncated to 200 chars). "
+                "results=" + json.dumps(data, default=str)[:800]
+            )
+
+
+@pytest.mark.asyncio
+async def test_delete_note_with_colliding_observations_leaves_no_ghost_rows(
+    mcp_server, app, test_project, search_service
+):
+    """Regression test for #909 follow-up: deletion must clean disambiguated rows.
+
+    Colliding observation permalinks are disambiguated at index time by appending
+    "-{obs.id}". Deleting the note must remove those rows too; otherwise the
+    disambiguated observation survives in the search index as a ghost row
+    pointing at the deleted file.
+
+    The post-delete assertions inspect the index through ``search_service``
+    rather than the ``search_notes`` tool: the MCP search pipeline happens to
+    hide rows whose entity is gone, which would mask the orphan.
+    """
+    from basic_memory.schemas.search import SearchQuery
+
+    async with Client(mcp_server) as client:
+        prefix = "y" * 210  # > 200 so truncation makes the permalinks identical
+        content = (
+            "# Ghost Obs Note\n\n"
+            "## Observations\n"
+            f"- [note] {prefix} GHOST_ALPHA_MARKER\n"
+            f"- [note] {prefix} GHOST_BETA_MARKER\n"
+        )
+        await client.call_tool(
+            "write_note",
+            {
+                "project": test_project.name,
+                "title": "Ghost Obs Note",
+                "directory": "ghost",
+                "content": content,
+            },
+        )
+
+        # Both observations are searchable before deletion
+        for marker in ("GHOST_ALPHA_MARKER", "GHOST_BETA_MARKER"):
+            result = await client.call_tool(
+                "search_notes",
+                {
+                    "project": test_project.name,
+                    "query": marker,
+                    "search_type": "text",
+                    "entity_types": ["observation"],
+                    "output_format": "json",
+                },
+            )
+            data = _json_content(result)
+            assert any(marker in (r.get("content") or "") for r in data["results"])
+
+        # Both rows exist in the search index itself, one under a "-{obs.id}" suffix
+        index_rows = await search_service.search(SearchQuery(text="GHOST_BETA_MARKER"))
+        assert any(r.type == "observation" for r in index_rows)
+
+        delete_result = await client.call_tool(
+            "delete_note",
+            {
+                "project": test_project.name,
+                "identifier": "Ghost Obs Note",
+            },
+        )
+        assert "true" in delete_result.content[0].text.lower()  # pyright: ignore [reportAttributeAccessIssue]
+
+        # No ghost rows remain in the index - including the disambiguated row
+        for marker in ("GHOST_ALPHA_MARKER", "GHOST_BETA_MARKER"):
+            index_rows = await search_service.search(SearchQuery(text=marker))
+            assert index_rows == [], (
+                f"search index row containing {marker} survived note deletion as a ghost: "
+                f"{[(r.type, r.permalink) for r in index_rows]}"
+            )