Skip to content

Commit 67aac01

Browse files
phernandezclaude
andcommitted
fix(sync): re-verify planned deletes so concurrent writes survive scan reconciliation
Scan delete plans compare a storage snapshot against a later DB read (deleted = all_db_paths - storage_paths). A note accepted and materialized between snapshot capture and the DB read was planned as deleted, and the delete batch destroyed its entity, search, and vector rows with no re-verification. apply_project_index_delete_batch now re-confirms each planned path through a ProjectIndexDeletePathVerifier capability before touching the database. The local runtime injects LocalProjectIndexDeletePathVerifier, which probes the filesystem: a path present again (or whose probe fails) is skipped and logged — the next scan picks the file up as modified. Cloud/S3 runtimes keep current behavior through the explicit TrustPlannedProjectIndexDeleteVerifier pass-through default, since their storage listing is authoritative and they have no cheap per-path probe at apply time. Skipped paths are reported on the batch result and delete run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 0a93d07 commit 67aac01

4 files changed

Lines changed: 307 additions & 3 deletions

File tree

src/basic_memory/index/local_project.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
run_project_index_coordinator,
5858
)
5959
from basic_memory.indexing.project_index_maintenance import (
60+
ProjectIndexDeletePathVerifier,
6061
ProjectIndexMaintenanceRunner,
6162
ProjectIndexMovedEntitySearchRefresher,
6263
RepositoryProjectIndexMaintenanceStore,
@@ -392,6 +393,34 @@ def _reuse_indexed_checksum(
392393
return indexed.checksum
393394

394395

396+
@dataclass(frozen=True, slots=True)
397+
class LocalProjectIndexDeletePathVerifier(ProjectIndexDeletePathVerifier):
398+
"""Probe the local filesystem before applying scan-planned index deletes."""
399+
400+
file_service: FileService
401+
402+
async def confirm_deleted_paths(self, paths: Sequence[str]) -> frozenset[str]:
403+
confirmed_paths: set[str] = set()
404+
for path in paths:
405+
# Trigger: the existence probe itself fails (permission/mount error).
406+
# Why: without a positive confirmation of absence, deleting would
407+
# turn a transient storage error into destroyed entity rows.
408+
# Outcome: leave the path out of the confirmed set; the next scan
409+
# reconciles it once the probe works again.
410+
try:
411+
still_absent = not await self.file_service.exists(path)
412+
except FileOperationError as exc:
413+
logger.warning(
414+
"Skipping planned index delete: existence probe failed",
415+
path=path,
416+
error=str(exc),
417+
)
418+
continue
419+
if still_absent:
420+
confirmed_paths.add(path)
421+
return frozenset(confirmed_paths)
422+
423+
395424
@dataclass(frozen=True, slots=True)
396425
class LocalProjectIndexRuntime:
397426
"""Dependencies for running project-wide local indexing through core fanout."""
@@ -558,6 +587,9 @@ def runtime_from_dependencies(
558587
entity_service=dependencies.entity_service,
559588
file_service=dependencies.file_service,
560589
),
590+
delete_path_verifier=LocalProjectIndexDeletePathVerifier(
591+
file_service=dependencies.file_service,
592+
),
561593
)
562594
return LocalProjectIndexRuntime(
563595
observed_file_source=LocalProjectIndexObservedFileSource(

src/basic_memory/indexing/project_index_maintenance.py

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,34 @@ async def refresh_moved_entities(self, entity_ids: Sequence[int]) -> None:
5959
"""Refresh search rows for moved entity ids."""
6060

6161

62+
class ProjectIndexDeletePathVerifier(Protocol):
63+
"""Capability that re-confirms scan-planned delete paths at apply time.
64+
65+
Delete plans come from a storage snapshot that is stale by the time the
66+
batch applies; a note accepted and materialized after the snapshot would
67+
otherwise be destroyed. Implementations return only the paths whose
68+
absence they can positively confirm right now.
69+
"""
70+
71+
async def confirm_deleted_paths(self, paths: Sequence[str]) -> frozenset[str]:
72+
"""Return the subset of paths confirmed absent from storage."""
73+
...
74+
75+
76+
@dataclass(frozen=True, slots=True)
77+
class TrustPlannedProjectIndexDeleteVerifier(ProjectIndexDeletePathVerifier):
78+
"""Confirm every planned delete without re-probing storage.
79+
80+
Cloud/S3 runtimes treat the scan's storage listing as authoritative and
81+
have no cheap per-path existence probe at apply time, so they keep the
82+
plan's verdict unchanged. Runtimes with a live filesystem (local) inject
83+
a probing verifier instead.
84+
"""
85+
86+
async def confirm_deleted_paths(self, paths: Sequence[str]) -> frozenset[str]:
87+
return frozenset(paths)
88+
89+
6290
@dataclass(frozen=True, slots=True)
6391
class ProjectIndexMovedFile:
6492
"""One indexed file move that may need storage-backed metadata repair."""
@@ -214,6 +242,7 @@ class ProjectIndexDeleteBatchResult:
214242
deleted_entities: int
215243
relation_cleanup_entity_ids: frozenset[int] = frozenset()
216244
missing_paths: tuple[str, ...] = ()
245+
skipped_paths: tuple[str, ...] = ()
217246

218247

219248
@dataclass(frozen=True, slots=True)
@@ -241,6 +270,13 @@ def missing_paths(self) -> tuple[str, ...]:
241270
missing_path for record in self.records for missing_path in record.result.missing_paths
242271
)
243272

273+
@property
274+
def skipped_paths(self) -> tuple[str, ...]:
275+
"""Return every planned delete path skipped because it is present again."""
276+
return tuple(
277+
skipped_path for record in self.records for skipped_path in record.result.skipped_paths
278+
)
279+
244280

245281
DELETE_PROJECT_INDEX_SEARCH_ROWS_SQL = text("""
246282
DELETE FROM search_index
@@ -414,6 +450,7 @@ class RepositoryProjectIndexMaintenanceStore:
414450
session_maker: async_sessionmaker[AsyncSession]
415451
project_id: ProjectId
416452
move_content_updater: ProjectIndexMoveContentUpdater | None = None
453+
delete_path_verifier: ProjectIndexDeletePathVerifier = TrustPlannedProjectIndexDeleteVerifier()
417454

418455
async def apply_project_index_move_batch(
419456
self,
@@ -599,19 +636,51 @@ async def apply_project_index_delete_batch(
599636
if not delete_batch.paths:
600637
return ProjectIndexDeleteBatchResult(deleted_entities=0)
601638

639+
# Trigger: a planned delete path exists in storage again at apply time.
640+
# Why: the plan compares a storage snapshot against a later DB read, so
641+
# a note accepted and materialized in between is planned as deleted;
642+
# applying it would destroy the accepted entity, search, and vector
643+
# rows with no recovery.
644+
# Outcome: only positively re-confirmed absences are deleted; skipped
645+
# paths are reported and the next scan picks the file up as
646+
# modified.
647+
confirmed_paths = await self.delete_path_verifier.confirm_deleted_paths(
648+
delete_batch.paths
649+
)
650+
skipped_paths = tuple(
651+
deleted_path
652+
for deleted_path in delete_batch.paths
653+
if deleted_path not in confirmed_paths
654+
)
655+
if skipped_paths:
656+
logger.warning(
657+
"Skipping planned index deletes for paths present in storage again",
658+
paths=skipped_paths,
659+
)
660+
if not confirmed_paths:
661+
return ProjectIndexDeleteBatchResult(
662+
deleted_entities=0,
663+
skipped_paths=skipped_paths,
664+
)
665+
602666
async with db.scoped_session(self.session_maker) as session:
603667
target_result = await session.execute(
604668
select(Entity.id, Entity.file_path).where(
605669
Entity.project_id == self.project_id,
606-
Entity.file_path.in_(tuple(delete_batch.paths)),
670+
Entity.file_path.in_(tuple(confirmed_paths)),
607671
)
608672
)
609673
target_rows = target_result.mappings().all()
610674

611675
if not target_rows:
612676
return ProjectIndexDeleteBatchResult(
613677
deleted_entities=0,
614-
missing_paths=tuple(delete_batch.paths),
678+
missing_paths=tuple(
679+
deleted_path
680+
for deleted_path in delete_batch.paths
681+
if deleted_path in confirmed_paths
682+
),
683+
skipped_paths=skipped_paths,
615684
)
616685

617686
deleted_entity_ids = tuple(int(row["id"]) for row in target_rows)
@@ -629,8 +698,9 @@ async def apply_project_index_delete_batch(
629698
missing_paths=tuple(
630699
deleted_path
631700
for deleted_path in delete_batch.paths
632-
if deleted_path not in deleted_found_paths
701+
if deleted_path in confirmed_paths and deleted_path not in deleted_found_paths
633702
),
703+
skipped_paths=skipped_paths,
634704
)
635705

636706

tests/index/test_local_project_index.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from basic_memory.index.local_project import (
1919
IndexedFileStat,
2020
LocalProjectIndexBatchEnqueuer,
21+
LocalProjectIndexDeletePathVerifier,
2122
LocalProjectIndexObservedFileSource,
2223
LocalProjectIndexRuntime,
2324
LocalProjectIndexRuntimeFactory,
@@ -666,6 +667,104 @@ async def test_local_project_index_reads_current_file_when_file_changes_after_ob
666667
assert "This is the modified content" in note_content.markdown_content
667668

668669

670+
async def test_local_project_index_delete_path_verifier_confirms_only_probed_absent_paths(
671+
tmp_path: Path,
672+
monkeypatch,
673+
) -> None:
674+
"""Only a positive absence probe confirms a planned delete; probe failures skip."""
675+
(tmp_path / "present.md").write_bytes(b"# Present\n")
676+
677+
file_service = FileService(tmp_path)
678+
original_exists = file_service.exists
679+
680+
async def flaky_exists(path):
681+
if str(path).endswith("flaky.md"):
682+
raise FileOperationError("mount error")
683+
return await original_exists(path)
684+
685+
monkeypatch.setattr(file_service, "exists", flaky_exists)
686+
687+
verifier = LocalProjectIndexDeletePathVerifier(file_service=file_service)
688+
confirmed = await verifier.confirm_deleted_paths(("present.md", "absent.md", "flaky.md"))
689+
690+
assert confirmed == frozenset({"absent.md"})
691+
692+
693+
@dataclass(slots=True)
694+
class RecreatingObservedFileSource:
695+
"""Recreate a file after the scan snapshot, simulating a concurrent accepted write."""
696+
697+
source: ProjectIndexObservedFileSource
698+
file_path: Path
699+
content: bytes
700+
701+
async def list_observed_index_files(self) -> tuple[RuntimeObservedIndexFile, ...]:
702+
observed = await self.source.list_observed_index_files()
703+
self.file_path.write_bytes(self.content)
704+
return observed
705+
706+
707+
async def test_local_project_index_delete_batch_spares_file_recreated_after_snapshot(
708+
test_project: Project,
709+
project_config,
710+
entity_repository,
711+
session_maker: async_sessionmaker[AsyncSession],
712+
config_manager,
713+
monkeypatch,
714+
) -> None:
715+
"""An entity whose file reappears after the scan snapshot must survive delete batches."""
716+
del config_manager
717+
718+
note_path = project_config.home / "late.md"
719+
note_content = b"# Late\n\nAccepted while the scan was running.\n"
720+
note_path.write_bytes(note_content)
721+
722+
first = await run_local_project_index_for_project(
723+
test_project,
724+
runtime_factory=LocalProjectIndexRuntimeFactory(batch_size=10),
725+
force_full=True,
726+
)
727+
assert first.enqueued_files == 1
728+
729+
async with db.scoped_session(session_maker) as session:
730+
entity_before = await entity_repository.get_by_file_path(session, "late.md")
731+
assert entity_before is not None
732+
733+
# The file is gone when the snapshot is captured, then written again before
734+
# the delete batch applies — the write_note accept/materialize race.
735+
note_path.unlink()
736+
runtime_factory = LocalProjectIndexRuntimeFactory(batch_size=10)
737+
runtime = await runtime_factory.runtime_for_project(test_project)
738+
racing_runtime = LocalProjectIndexRuntime(
739+
observed_file_source=RecreatingObservedFileSource(
740+
source=runtime.observed_file_source,
741+
file_path=note_path,
742+
content=note_content,
743+
),
744+
change_detector=runtime.change_detector,
745+
maintenance_runner=runtime.maintenance_runner,
746+
moved_entity_search_refresher=runtime.moved_entity_search_refresher,
747+
batch_enqueuer=runtime.batch_enqueuer,
748+
completion_relation_runtime=runtime.completion_relation_runtime,
749+
batch_size=runtime.batch_size,
750+
)
751+
752+
second = await run_local_project_index(
753+
RuntimeProjectIndexJobRequest(
754+
project=ProjectRuntimeReference.from_project(test_project),
755+
),
756+
runtime=racing_runtime,
757+
)
758+
759+
assert second.deleted_files == 0
760+
761+
async with db.scoped_session(session_maker) as session:
762+
entity_after = await entity_repository.get_by_file_path(session, "late.md")
763+
764+
assert entity_after is not None
765+
assert entity_after.id == entity_before.id
766+
767+
669768
@dataclass(slots=True)
670769
class RecordingObservedFileSource:
671770
observed_files: tuple[RuntimeObservedIndexFile, ...]

0 commit comments

Comments
 (0)