Skip to content

Commit 25337f9

Browse files
phernandezclaude
andcommitted
fix(sync): tolerate concurrently deleted entities in moved-entity search refresh
RepositoryProjectIndexMovedEntitySearchRefresher raised RuntimeError when any moved entity id no longer had a database row at refresh time. Move batches commit before the refresh runs, so a concurrent delete (file removed, note deleted via API) between move-batch commit and refresh is a benign race — but the raise aborted the whole coordinator run before delete batches and file indexing executed, stalling the scan. A missing entity at refresh time is now logged and skipped; the surviving moved entities are still refreshed. The vanished entity's search rows were already removed with its row, so there is nothing left to repair. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 7ee7889 commit 25337f9

2 files changed

Lines changed: 79 additions & 5 deletions

File tree

src/basic_memory/indexing/project_index_maintenance.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from dataclasses import dataclass
77
from typing import Protocol
88

9+
from loguru import logger
910
from sqlalchemy import bindparam, case, column, delete, select, table, text, update
1011
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
1112

@@ -685,14 +686,24 @@ async def refresh_moved_entities(self, entity_ids: Sequence[int]) -> None:
685686
missing_entity_ids = [
686687
entity_id for entity_id in unique_entity_ids if entity_id not in entities_by_id
687688
]
689+
# Trigger: a moved entity id has no row by the time the refresh reloads it.
690+
# Why: move batches commit before this refresh runs, so a concurrent delete
691+
# (file removed, note deleted via API) can legitimately retire the row
692+
# in between; failing here would abort the coordinator run before delete
693+
# batches and file indexing, stalling the whole scan over a benign race.
694+
# Outcome: skip the vanished ids (their search rows were removed with the
695+
# entity) and refresh the survivors.
688696
if missing_entity_ids:
689-
raise RuntimeError(
690-
"Moved entities disappeared before search refresh: "
691-
f"{', '.join(str(entity_id) for entity_id in missing_entity_ids)}"
697+
logger.warning(
698+
"Skipping search refresh for moved entities deleted mid-run",
699+
entity_ids=missing_entity_ids,
692700
)
693701

694702
for entity_id in unique_entity_ids:
695-
await self.entity_indexer.index_entity(entities_by_id[entity_id])
703+
entity = entities_by_id.get(entity_id)
704+
if entity is None:
705+
continue
706+
await self.entity_indexer.index_entity(entity)
696707

697708

698709
def build_project_index_move_batch_plan(

tests/indexing/test_project_index_maintenance.py

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Tests for project-index move/delete maintenance."""
22

3-
from collections.abc import AsyncIterator, Iterator
3+
from collections.abc import AsyncIterator, Iterator, Sequence
44
from contextlib import asynccontextmanager
55
from dataclasses import dataclass, field
66
from types import SimpleNamespace
@@ -23,12 +23,14 @@
2323
ProjectIndexMoveRun,
2424
ProjectIndexMoveTarget,
2525
RepositoryProjectIndexMaintenanceStore,
26+
RepositoryProjectIndexMovedEntitySearchRefresher,
2627
StoreProjectIndexMaintenanceRunner,
2728
build_project_index_delete_batch_plan,
2829
build_project_index_move_batch_plan,
2930
run_project_index_delete_batches,
3031
run_project_index_move_batches,
3132
)
33+
from basic_memory.models import Entity
3234

3335

3436
def _stub_move_store(
@@ -172,6 +174,67 @@ async def update_moved_file_content(
172174
return self.updates.get(moved_file.entity_id)
173175

174176

177+
@dataclass(slots=True)
178+
class StaticMovedEntityRepository:
179+
"""Return only the moved entities that still have a database row."""
180+
181+
entities: list[Entity]
182+
183+
async def find_by_ids(
184+
self,
185+
session: AsyncSession,
186+
ids: list[int],
187+
) -> Sequence[Entity]:
188+
del session
189+
return [entity for entity in self.entities if entity.id in ids]
190+
191+
192+
@dataclass(slots=True)
193+
class RecordingMovedEntityIndexer:
194+
"""Record which moved entities were search-refreshed."""
195+
196+
indexed_entity_ids: list[int] = field(default_factory=list)
197+
198+
async def index_entity(self, entity: Entity) -> object:
199+
self.indexed_entity_ids.append(entity.id)
200+
return entity
201+
202+
203+
@pytest.mark.asyncio
204+
async def test_moved_entity_search_refresher_skips_entities_deleted_mid_run(
205+
monkeypatch: pytest.MonkeyPatch,
206+
) -> None:
207+
"""A moved entity deleted between move commit and refresh must not abort the run."""
208+
session_maker = cast(async_sessionmaker[AsyncSession], object())
209+
session = FakeProjectIndexSession(results=[])
210+
211+
@asynccontextmanager
212+
async def fake_scoped_session(
213+
scoped_session_maker: async_sessionmaker[AsyncSession],
214+
) -> AsyncIterator[FakeProjectIndexSession]:
215+
assert scoped_session_maker is session_maker
216+
yield session
217+
218+
monkeypatch.setattr(
219+
project_index_maintenance_module.db,
220+
"scoped_session",
221+
fake_scoped_session,
222+
)
223+
224+
surviving_entity = cast(Entity, SimpleNamespace(id=10))
225+
entity_indexer = RecordingMovedEntityIndexer()
226+
refresher = RepositoryProjectIndexMovedEntitySearchRefresher(
227+
session_maker=session_maker,
228+
entity_repository=StaticMovedEntityRepository(entities=[surviving_entity]),
229+
entity_indexer=entity_indexer,
230+
)
231+
232+
# Entity 5 was deleted concurrently; only entity 10 can still be refreshed.
233+
await refresher.refresh_moved_entities([10, 5, 10])
234+
235+
assert entity_indexer.indexed_entity_ids == [10]
236+
237+
175238
def test_project_index_move_batch_plan_builds_batches_and_progress_metadata() -> None:
176239
plan = build_project_index_move_batch_plan(
177240
moved_files={

0 commit comments

Comments
 (0)