Skip to content

Commit f13ad97

Browse files
phernandezclaude
andcommitted
fix(sync): defer moved-file frontmatter writes until the move batch commits
apply_project_index_move_batch called the move content updater inside the open db.scoped_session, and the local updater wrote the moved file's permalink frontmatter to disk via update_frontmatter_with_result before the transaction committed. If the batch rolled back (e.g. an intra-batch permalink collision), the database reverted but the on-disk rewrites persisted, leaving files ahead of their indexed state. The ProjectIndexMoveContentUpdater capability is now split into plan_moved_file_content — which runs inside the transaction, resolves the permalink, and builds the updated markdown in memory without touching storage — and write_moved_file_content, which the store calls only after a successful commit. The database rows are stamped from the planned content, whose checksum matches the exact bytes the post-commit write persists, so DB and file agree when the write lands; a failed write (or formatter or platform newline divergence) is logged and surfaces on the next scan as a checksum-mismatch modified file, which re-indexes from disk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent d2bbb05 commit f13ad97

5 files changed

Lines changed: 484 additions & 31 deletions

File tree

src/basic_memory/index/local_moves.py

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,24 @@
22

33
from __future__ import annotations
44

5-
from collections.abc import Sequence
5+
from collections.abc import Mapping, Sequence
66
from dataclasses import dataclass
77
from pathlib import Path
88
from typing import Protocol
99

10+
import yaml
11+
from loguru import logger
1012
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
1113

1214
from basic_memory import db
1315
from basic_memory.config import BasicMemoryConfig
16+
from basic_memory.file_utils import (
17+
ParseError,
18+
compute_checksum,
19+
has_frontmatter,
20+
parse_frontmatter,
21+
remove_frontmatter,
22+
)
1423
from basic_memory.markdown import EntityMarkdown
1524
from basic_memory.indexing.project_index_maintenance import (
1625
ProjectIndexMaintenanceRunner,
@@ -63,14 +72,41 @@ async def resolve_permalink(
6372
) -> str: ...
6473

6574

75+
def merged_frontmatter_markdown(content: str, updates: Mapping[str, object]) -> str:
76+
"""Return markdown with frontmatter updates applied, without touching storage.
77+
78+
Mirrors FileService.update_frontmatter_with_result's merge — including its
79+
tolerance for malformed YAML, which keeps the full content and prepends a
80+
fresh frontmatter block — so planned bytes match what a direct frontmatter
81+
rewrite would have produced.
82+
"""
83+
current_frontmatter: dict[str, object] = {}
84+
body = content
85+
if has_frontmatter(content):
86+
try:
87+
current_frontmatter = dict(parse_frontmatter(content))
88+
body = remove_frontmatter(content)
89+
except ParseError as error:
90+
logger.warning(
91+
"Treating file with malformed frontmatter as plain markdown",
92+
error=str(error),
93+
)
94+
current_frontmatter = {}
95+
body = content
96+
97+
merged_frontmatter = {**current_frontmatter, **updates}
98+
yaml_block = yaml.dump(merged_frontmatter, sort_keys=False, allow_unicode=True)
99+
return f"---\n{yaml_block}---\n\n{body.strip()}"
100+
101+
66102
@dataclass(frozen=True, slots=True)
67103
class LocalProjectIndexMoveContentUpdater(ProjectIndexMoveContentUpdater):
68104
"""Apply local markdown permalink policy for moved files."""
69105

70106
entity_service: LocalMoveEntityService
71107
file_service: FileService
72108

73-
async def update_moved_file_content(
109+
async def plan_moved_file_content(
74110
self,
75111
session: AsyncSession,
76112
moved_file: ProjectIndexMovedFile,
@@ -91,16 +127,30 @@ async def update_moved_file_content(
91127
if permalink == moved_file.old_permalink:
92128
return None
93129

94-
update = await self.file_service.update_frontmatter_with_result(
95-
moved_file.new_path,
130+
current_bytes = await self.file_service.read_file_bytes(moved_file.new_path)
131+
planned_content = merged_frontmatter_markdown(
132+
current_bytes.decode("utf-8"),
96133
{"permalink": permalink},
97134
)
135+
# Invariant: the move batch stamps entity/note_content rows from this
136+
# planned content while write_moved_file_content persists exactly these
137+
# bytes after commit, so database and file checksums agree. Divergence
138+
# (a failed write, an on-save formatter, platform newline translation)
139+
# shows up on the next scan as a checksum-mismatch modified file and is
140+
# re-indexed from disk.
98141
return ProjectIndexMovedFileContentUpdate(
99142
permalink=permalink,
100-
checksum=update.checksum,
101-
markdown_content=update.content,
143+
checksum=await compute_checksum(planned_content),
144+
markdown_content=planned_content,
102145
)
103146

147+
async def write_moved_file_content(
148+
self,
149+
moved_file: ProjectIndexMovedFile,
150+
content_update: ProjectIndexMovedFileContentUpdate,
151+
) -> None:
152+
await self.file_service.write_file(moved_file.new_path, content_update.markdown_content)
153+
104154

105155
@dataclass(frozen=True, slots=True)
106156
class LocalWatchMoveProcessingResult:

src/basic_memory/indexing/project_index_maintenance.py

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -99,22 +99,39 @@ class ProjectIndexMovedFile:
9999

100100
@dataclass(frozen=True, slots=True)
101101
class ProjectIndexMovedFileContentUpdate:
102-
"""Storage result after rewriting a moved file's markdown metadata."""
102+
"""Planned markdown metadata rewrite for a moved file.
103+
104+
``checksum`` is computed from ``markdown_content`` — the exact bytes the
105+
post-commit write persists — so the database rows stamped during the batch
106+
transaction agree with the file once the write lands.
107+
"""
103108

104109
permalink: str
105110
checksum: str
106111
markdown_content: str
107112

108113

109114
class ProjectIndexMoveContentUpdater(Protocol):
110-
"""Capability that applies provider-specific moved-file content repair."""
115+
"""Capability that plans and persists provider-specific moved-file content repair.
116+
117+
Planning runs inside the move batch's database transaction and must not
118+
mutate storage: the batch can still roll back (e.g. an intra-batch
119+
permalink collision), and an already-rewritten file would survive that
120+
rollback. The write runs only after the batch commits.
121+
"""
111122

112-
async def update_moved_file_content(
123+
async def plan_moved_file_content(
113124
self,
114125
session: AsyncSession,
115126
moved_file: ProjectIndexMovedFile,
116127
) -> ProjectIndexMovedFileContentUpdate | None: ...
117128

129+
async def write_moved_file_content(
130+
self,
131+
moved_file: ProjectIndexMovedFile,
132+
content_update: ProjectIndexMovedFileContentUpdate,
133+
) -> None: ...
134+
118135

119136
@dataclass(frozen=True, slots=True)
120137
class ProjectIndexMoveTarget:
@@ -543,39 +560,39 @@ async def apply_project_index_move_batch(
543560
replacement_rows = verified_replacement_rows
544561
if dropped_new_paths:
545562
dropped_move_paths = tuple(
546-
sorted(
547-
old_path_by_new_path[new_path] for new_path in dropped_new_paths
548-
)
563+
sorted(old_path_by_new_path[new_path] for new_path in dropped_new_paths)
549564
)
550565
target_rows = [
551566
row
552567
for row in target_rows
553-
if target_paths_by_old_path[str(row["file_path"])]
554-
not in dropped_new_paths
568+
if target_paths_by_old_path[str(row["file_path"])] not in dropped_new_paths
555569
]
556570

557571
updated_old_paths = frozenset(str(row["file_path"]) for row in target_rows)
558572
target_paths_by_entity_id = {
559573
int(row["id"]): target_paths_by_old_path[str(row["file_path"])]
560574
for row in target_rows
561575
}
576+
planned_moved_files_by_entity_id: dict[int, ProjectIndexMovedFile] = {}
562577
if self.move_content_updater is not None:
563578
for row in target_rows:
564579
entity_id = int(row["id"])
565580
old_path = str(row["file_path"])
566-
content_update = await self.move_content_updater.update_moved_file_content(
567-
session,
568-
ProjectIndexMovedFile(
569-
entity_id=entity_id,
570-
old_path=old_path,
571-
new_path=target_paths_by_old_path[old_path],
572-
old_permalink=(
573-
str(row["permalink"]) if row["permalink"] is not None else None
574-
),
581+
moved_file = ProjectIndexMovedFile(
582+
entity_id=entity_id,
583+
old_path=old_path,
584+
new_path=target_paths_by_old_path[old_path],
585+
old_permalink=(
586+
str(row["permalink"]) if row["permalink"] is not None else None
575587
),
576588
)
589+
content_update = await self.move_content_updater.plan_moved_file_content(
590+
session,
591+
moved_file,
592+
)
577593
if content_update is not None:
578594
content_updates_by_entity_id[entity_id] = content_update
595+
planned_moved_files_by_entity_id[entity_id] = moved_file
579596

580597
if updated_old_paths:
581598
replaced_entity_ids = frozenset(int(row["id"]) for row in replacement_rows)
@@ -686,6 +703,28 @@ async def apply_project_index_move_batch(
686703
)
687704
)
688705

706+
# Trigger: the batch committed with entity/note_content rows stamped from
707+
# the planned markdown, and the files still hold their pre-move metadata.
708+
# Why: writing files inside the transaction is not atomic with it — a
709+
# rollback would revert the database while the on-disk frontmatter
710+
# rewrites persisted, leaving files ahead of their indexed state.
711+
# Outcome: writes happen only after a successful commit; a failed write
712+
# leaves the file with a checksum that no longer matches its
713+
# rows, which the next scan reconciles as a modified file.
714+
if self.move_content_updater is not None:
715+
for entity_id, content_update in content_updates_by_entity_id.items():
716+
try:
717+
await self.move_content_updater.write_moved_file_content(
718+
planned_moved_files_by_entity_id[entity_id],
719+
content_update,
720+
)
721+
except Exception as write_error:
722+
logger.error(
723+
"Failed to write moved file content after move batch commit",
724+
path=planned_moved_files_by_entity_id[entity_id].new_path,
725+
error=str(write_error),
726+
)
727+
689728
missing_paths = tuple(
690729
move_target.old_path
691730
for move_target in move_batch.targets
@@ -716,9 +755,7 @@ async def apply_project_index_delete_batch(
716755
# Outcome: only positively re-confirmed absences are deleted; skipped
717756
# paths are reported and the next scan picks the file up as
718757
# modified.
719-
confirmed_paths = await self.delete_path_verifier.confirm_deleted_paths(
720-
delete_batch.paths
721-
)
758+
confirmed_paths = await self.delete_path_verifier.confirm_deleted_paths(delete_batch.paths)
722759
skipped_paths = tuple(
723760
deleted_path
724761
for deleted_path in delete_batch.paths

0 commit comments

Comments
 (0)