Skip to content

Commit 0f67ecd

Browse files
phernandezclaude
andcommitted
fix(sync): re-read file at batch reconcile to avoid reverting newer accepted content
Batch note_content reconciliation reconciled each entity using `indexed.markdown_content`, a scan-time snapshot. Between scan and reconcile a note materialization can rewrite the file to a newer accepted db_version; the batch then promoted the stale snapshot and reverted the newer write. The db_version compare-and-set guard does not help: the stale snapshot promotes cleanly to a fresh version. Add an optional `file_reader` (NoteContentReconcileFileReader) threaded through `reconcile_indexed_note_content_batch`, `IndexedNoteContentReconciliationTask`, `IndexBatchRuntime`, and `build_default_index_batch_runtime`. When set, the reconcile task re-reads the current file: missing content (file gone) skips reconciliation, otherwise the fresh content and its mtime drive reconciliation instead of the scan snapshot. Default None preserves existing behavior so cloud compiles unchanged. Local indexing wires a filesystem-backed reader (FileServiceNoteContentReconcileFileReader) built on FileService. Cloud passes None for now to avoid doubling S3 reads and can opt in later via its S3 reader. Adds regression coverage: a stale scan snapshot no longer reverts a newer accepted note_content version once the reader re-reads the current file, plus a companion test documenting the revert that occurs without the reader. 🐛 Fixes a concurrency defect the version guard cannot catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 5cd42a9 commit 0f67ecd

5 files changed

Lines changed: 359 additions & 6 deletions

File tree

src/basic_memory/index/local_dependencies.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from collections.abc import Mapping, Sequence
66
from dataclasses import dataclass
7+
from datetime import datetime
78
from pathlib import Path
89
from typing import Any, Protocol
910

@@ -68,6 +69,42 @@
6869
from basic_memory.services.search_service import SearchService
6970

7071

72+
@dataclass(frozen=True, slots=True)
73+
class FileServiceReconcileFile:
74+
"""Canonical file state re-read at batch reconcile time."""
75+
76+
content: bytes | None
77+
last_modified: datetime | None
78+
79+
80+
@dataclass(frozen=True, slots=True)
81+
class FileServiceNoteContentReconcileFileReader:
82+
"""Filesystem-backed reader that re-reads canonical markdown at reconcile time.
83+
84+
Batch reconciliation indexes a scan-time snapshot of each file. A note
85+
materialization can rewrite the same file to a newer accepted version between
86+
the scan and reconcile; re-reading here lets reconciliation promote the
87+
current file instead of reverting to the stale snapshot.
88+
"""
89+
90+
file_service: FileService
91+
92+
async def get_file(self, path: RuntimeFilePath) -> FileServiceReconcileFile:
93+
"""Return current file bytes and mtime, or None content when the file is gone."""
94+
try:
95+
content = await self.file_service.read_file_bytes(path)
96+
except FileOperationError as exc:
97+
# Trigger: the file was deleted between scan and reconcile.
98+
# Why: a missing file has no content to promote; surface None so the
99+
# batch task skips reconciliation instead of failing the whole batch.
100+
# Outcome: caller treats None content as "nothing to reconcile".
101+
if isinstance(exc.__cause__, FileNotFoundError):
102+
return FileServiceReconcileFile(content=None, last_modified=None)
103+
raise
104+
metadata = await self.file_service.get_file_metadata(path)
105+
return FileServiceReconcileFile(content=content, last_modified=metadata.modified_at)
106+
107+
71108
class LocalIndexEntityRepository(
72109
IndexMarkdownEntityRepository,
73110
IndexedFileChecksumRepository,
@@ -598,6 +635,7 @@ async def build_local_index_project_dependencies(
598635
frontmatter_storage=file_service,
599636
content_type_provider=file_service,
600637
session_maker=session_maker,
638+
file_reader=FileServiceNoteContentReconcileFileReader(file_service=file_service),
601639
)
602640
file_indexer = build_local_markdown_file_indexer(
603641
project_id=project.id,

src/basic_memory/indexing/index_batch_runtime.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@
3131
IndexedNoteContentTimestampProvider,
3232
reconcile_indexed_note_content_batch,
3333
)
34-
from basic_memory.indexing.note_content_reconciler import NoteContentReconciler
34+
from basic_memory.indexing.note_content_reconciler import (
35+
NoteContentReconcileFileReader,
36+
NoteContentReconciler,
37+
)
3538
from basic_memory.models import Entity
3639
from basic_memory.repository import EntityRepository, NoteContentRepository, RelationRepository
3740
from basic_memory.runtime.storage import ProjectId
@@ -62,6 +65,12 @@ class IndexBatchRuntime[EntityT: IndexedNoteContentEntity, FileInfoT: LoadedInde
6265
note_content_reconciler: IndexedNoteContentReconciler[EntityT]
6366
timestamp_provider: IndexedNoteContentTimestampProvider[FileInfoT]
6467
note_content_source: str = "index"
68+
# Optional canonical-file reader. When set, batch reconciliation re-reads each
69+
# file at reconcile time instead of trusting the scan snapshot, so a note
70+
# materialization that rewrote the file between scan and reconcile is not
71+
# reverted. Local indexing passes a filesystem reader; cloud leaves this None
72+
# to avoid doubling S3 reads (it can opt in later).
73+
file_reader: NoteContentReconcileFileReader | None = None
6574

6675
async def index_loaded_files(
6776
self,
@@ -90,6 +99,7 @@ async def index_loaded_files(
9099
timestamp_provider=self.timestamp_provider,
91100
max_concurrent=metadata_update_max_concurrent or max_concurrent,
92101
source=self.note_content_source,
102+
file_reader=self.file_reader,
93103
)
94104
result.errors.extend(error.as_tuple() for error in note_content_errors)
95105
result.search_indexed = count_search_indexed_entities(result.indexed)
@@ -136,6 +146,7 @@ def build_default_index_batch_runtime[FileInfoT: LoadedIndexFile](
136146
frontmatter_storage: IndexFrontmatterStorage,
137147
content_type_provider: IndexContentTypeProvider,
138148
session_maker: async_sessionmaker[AsyncSession],
149+
file_reader: NoteContentReconcileFileReader | None = None,
139150
) -> DefaultIndexBatchRuntime[FileInfoT]:
140151
"""Compose the default repository-backed batch index runtime.
141152
@@ -170,6 +181,7 @@ def build_default_index_batch_runtime[FileInfoT: LoadedIndexFile](
170181
session_maker=session_maker,
171182
note_content_reconciler=note_content_reconciler,
172183
timestamp_provider=DefaultIndexedNoteContentTimestampProvider(),
184+
file_reader=file_reader,
173185
),
174186
note_content_reconciler=note_content_reconciler,
175187
)

src/basic_memory/indexing/note_content_batch_reconciliation.py

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from basic_memory import db
1616
from basic_memory.indexing.file_index_planning import FileIndexPath
1717
from basic_memory.indexing.models import IndexedEntity
18+
from basic_memory.indexing.note_content_reconciler import NoteContentReconcileFileReader
1819

1920

2021
class IndexedNoteContentEntity(Protocol):
@@ -175,6 +176,7 @@ class IndexedNoteContentReconciliationTask[
175176
note_content_reconciler: IndexedNoteContentReconciler[EntityT]
176177
timestamp_provider: IndexedNoteContentTimestampProvider[FileInfoT]
177178
source: str
179+
file_reader: NoteContentReconcileFileReader | None = None
178180

179181
async def run(self) -> IndexedNoteContentReconciliationError | None:
180182
if self.indexed.markdown_content is None:
@@ -187,14 +189,35 @@ async def run(self) -> IndexedNoteContentReconciliationError | None:
187189
f"Entity {self.indexed.entity_id} not found after indexing",
188190
)
189191

192+
# --- Resolve the markdown version to reconcile ---
193+
# indexed.markdown_content is a SCAN-TIME snapshot. Between the scan and
194+
# this reconcile a note materialization can rewrite the file to a newer
195+
# accepted db_version; promoting the snapshot would revert that write and
196+
# the db_version compare-and-set guard cannot catch it (the stale snapshot
197+
# still promotes cleanly to a fresh version).
198+
if self.file_reader is not None:
199+
# Trigger: a filesystem reader is configured (local indexing path).
200+
# Why: re-read the file so we reconcile the CURRENT accepted content,
201+
# not the possibly-stale scan snapshot.
202+
# Outcome: fresh content + its last_modified drive reconciliation.
203+
fresh = await self.file_reader.get_file(self.indexed.path)
204+
if fresh.content is None:
205+
# File was removed between scan and reconcile; nothing to promote.
206+
return None
207+
markdown_content = fresh.content.decode("utf-8")
208+
observed_at = fresh.last_modified
209+
else:
210+
markdown_content = self.indexed.markdown_content
211+
observed_at = self.timestamp_provider.observed_at(
212+
self.indexed,
213+
self.file_infos.get(self.indexed.path),
214+
)
215+
190216
try:
191217
await self.note_content_reconciler.reconcile(
192218
entity=entity,
193-
markdown_content=self.indexed.markdown_content,
194-
observed_at=self.timestamp_provider.observed_at(
195-
self.indexed,
196-
self.file_infos.get(self.indexed.path),
197-
),
219+
markdown_content=markdown_content,
220+
observed_at=observed_at,
198221
source=self.source,
199222
)
200223
return None
@@ -218,6 +241,7 @@ async def reconcile_indexed_note_content_batch[
218241
timestamp_provider: IndexedNoteContentTimestampProvider[FileInfoT],
219242
max_concurrent: int,
220243
source: str = "index",
244+
file_reader: NoteContentReconcileFileReader | None = None,
221245
) -> tuple[IndexedNoteContentReconciliationError, ...]:
222246
"""Hydrate note_content rows for indexed markdown entities after batch indexing."""
223247
markdown_entities = tuple(
@@ -242,6 +266,7 @@ async def reconcile_indexed_note_content_batch[
242266
note_content_reconciler=note_content_reconciler,
243267
timestamp_provider=timestamp_provider,
244268
source=source,
269+
file_reader=file_reader,
245270
)
246271
for indexed in markdown_entities
247272
],
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
"""Tests for the local filesystem note_content reconcile file reader."""
2+
3+
from __future__ import annotations
4+
5+
from datetime import datetime
6+
from pathlib import Path
7+
from types import SimpleNamespace
8+
from typing import Any, cast
9+
10+
import pytest
11+
12+
from basic_memory.index.local_dependencies import (
13+
FileServiceNoteContentReconcileFileReader,
14+
FileServiceReconcileFile,
15+
)
16+
from basic_memory.services.exceptions import FileOperationError
17+
from basic_memory.services.file_service import FileService
18+
19+
20+
@pytest.mark.asyncio
21+
async def test_reader_returns_current_bytes_and_mtime(file_service: FileService) -> None:
22+
"""The reader returns the current file bytes and modification time."""
23+
path = "note.md"
24+
(Path(file_service.base_path) / path).write_text("# Fresh\n", encoding="utf-8")
25+
26+
reader = FileServiceNoteContentReconcileFileReader(file_service=file_service)
27+
result = await reader.get_file(path)
28+
29+
assert result.content == b"# Fresh\n"
30+
assert isinstance(result.last_modified, datetime)
31+
32+
33+
@pytest.mark.asyncio
34+
async def test_reader_returns_none_content_when_file_missing(file_service: FileService) -> None:
35+
"""A file removed between scan and reconcile yields None content, not an error."""
36+
reader = FileServiceNoteContentReconcileFileReader(file_service=file_service)
37+
38+
result = await reader.get_file("gone.md")
39+
40+
assert result == FileServiceReconcileFile(content=None, last_modified=None)
41+
42+
43+
@pytest.mark.asyncio
44+
async def test_reader_reraises_non_missing_file_errors() -> None:
45+
"""Read failures other than a missing file must propagate."""
46+
47+
async def read_file_bytes(path: str) -> bytes:
48+
raise FileOperationError("boom") from ValueError("disk error")
49+
50+
file_service = SimpleNamespace(read_file_bytes=read_file_bytes)
51+
reader = FileServiceNoteContentReconcileFileReader(file_service=cast(Any, file_service))
52+
53+
with pytest.raises(FileOperationError):
54+
await reader.get_file("broken.md")

0 commit comments

Comments
 (0)