mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(sync): preserve canonical markdown in single-file sync (#746)
Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -12,6 +12,7 @@ from basic_memory.indexing.models import (
|
||||
IndexingBatchResult,
|
||||
IndexInputFile,
|
||||
IndexProgress,
|
||||
SyncedMarkdownFile,
|
||||
)
|
||||
|
||||
__all__ = [
|
||||
@@ -25,5 +26,6 @@ __all__ = [
|
||||
"IndexingBatchResult",
|
||||
"IndexInputFile",
|
||||
"IndexProgress",
|
||||
"SyncedMarkdownFile",
|
||||
"build_index_batches",
|
||||
]
|
||||
|
||||
@@ -12,7 +12,7 @@ from loguru import logger
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
|
||||
from basic_memory.config import BasicMemoryConfig
|
||||
from basic_memory.file_utils import compute_checksum, has_frontmatter
|
||||
from basic_memory.file_utils import compute_checksum, has_frontmatter, remove_frontmatter
|
||||
from basic_memory.markdown.schemas import EntityMarkdown
|
||||
from basic_memory.indexing.models import (
|
||||
IndexedEntity,
|
||||
@@ -43,12 +43,19 @@ class _PreparedMarkdownFile:
|
||||
class _PreparedEntity:
|
||||
path: str
|
||||
entity_id: int
|
||||
permalink: str | None
|
||||
checksum: str
|
||||
content_type: str | None
|
||||
search_content: str | None
|
||||
markdown_content: str | None = None
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class _PersistedMarkdownFile:
|
||||
prepared: _PreparedMarkdownFile
|
||||
entity: Entity
|
||||
|
||||
|
||||
class BatchIndexer:
|
||||
"""Index already-loaded files without assuming where they came from."""
|
||||
|
||||
@@ -118,6 +125,9 @@ class BatchIndexer:
|
||||
)
|
||||
error_by_path.update(markdown_errors)
|
||||
prepared_entities.update(markdown_upserts)
|
||||
if existing_permalink_by_path is not None:
|
||||
for path, prepared_entity in markdown_upserts.items():
|
||||
existing_permalink_by_path[path] = prepared_entity.permalink
|
||||
|
||||
regular_upserts, regular_errors = await self._run_bounded(
|
||||
regular_paths,
|
||||
@@ -168,6 +178,57 @@ class BatchIndexer:
|
||||
search_indexed=search_indexed,
|
||||
)
|
||||
|
||||
async def index_markdown_file(
|
||||
self,
|
||||
file: IndexInputFile,
|
||||
*,
|
||||
new: bool | None = None,
|
||||
existing_permalink_by_path: dict[str, str | None] | None = None,
|
||||
index_search: bool = True,
|
||||
) -> IndexedEntity:
|
||||
"""Index one markdown file using the same normalization and upsert path as batches."""
|
||||
if not self._is_markdown(file):
|
||||
raise ValueError(f"index_markdown_file requires markdown input: {file.path}")
|
||||
|
||||
prepared = await self._prepare_markdown_file(file)
|
||||
if existing_permalink_by_path is None:
|
||||
existing_permalink_by_path = {
|
||||
path: permalink
|
||||
for path, permalink in (
|
||||
await self.entity_repository.get_file_path_to_permalink_map()
|
||||
).items()
|
||||
}
|
||||
|
||||
reserved_permalinks = {
|
||||
permalink
|
||||
for path, permalink in existing_permalink_by_path.items()
|
||||
if path != file.path and permalink
|
||||
}
|
||||
prepared = await self._normalize_markdown_file(prepared, reserved_permalinks)
|
||||
existing_permalink_by_path[file.path] = prepared.markdown.frontmatter.permalink
|
||||
|
||||
persisted = await self._persist_markdown_file(prepared, is_new=new)
|
||||
existing_permalink_by_path[file.path] = persisted.entity.permalink
|
||||
await self._resolve_batch_relations([persisted.entity.id], max_concurrent=1)
|
||||
|
||||
refreshed = await self.entity_repository.find_by_ids([persisted.entity.id])
|
||||
if len(refreshed) != 1: # pragma: no cover
|
||||
raise ValueError(f"Failed to reload indexed entity for {file.path}")
|
||||
entity = refreshed[0]
|
||||
prepared_entity = self._build_prepared_entity(persisted.prepared, entity)
|
||||
|
||||
if index_search:
|
||||
return await self._refresh_search_index(prepared_entity, entity)
|
||||
|
||||
return IndexedEntity(
|
||||
path=prepared_entity.path,
|
||||
entity_id=entity.id,
|
||||
permalink=entity.permalink,
|
||||
checksum=prepared_entity.checksum,
|
||||
content_type=prepared_entity.content_type,
|
||||
markdown_content=prepared_entity.markdown_content,
|
||||
)
|
||||
|
||||
# --- Preparation ---
|
||||
|
||||
async def _prepare_markdown_file(self, file: IndexInputFile) -> _PreparedMarkdownFile:
|
||||
@@ -320,34 +381,8 @@ class BatchIndexer:
|
||||
# --- Persistence ---
|
||||
|
||||
async def _upsert_markdown_file(self, prepared: _PreparedMarkdownFile) -> _PreparedEntity:
|
||||
existing = await self.entity_repository.get_by_file_path(
|
||||
prepared.file.path,
|
||||
load_relations=False,
|
||||
)
|
||||
entity = await self.entity_service.upsert_entity_from_markdown(
|
||||
Path(prepared.file.path),
|
||||
prepared.markdown,
|
||||
is_new=existing is None,
|
||||
)
|
||||
updated = await self.entity_repository.update(
|
||||
entity.id,
|
||||
self._entity_metadata_updates(prepared.file, prepared.final_checksum),
|
||||
)
|
||||
if updated is None:
|
||||
raise ValueError(f"Failed to update markdown entity metadata for {prepared.file.path}")
|
||||
|
||||
return _PreparedEntity(
|
||||
path=prepared.file.path,
|
||||
entity_id=updated.id,
|
||||
checksum=prepared.final_checksum,
|
||||
content_type=prepared.file.content_type,
|
||||
search_content=(
|
||||
prepared.markdown.content
|
||||
if prepared.markdown.content is not None
|
||||
else prepared.content
|
||||
),
|
||||
markdown_content=prepared.content,
|
||||
)
|
||||
persisted = await self._persist_markdown_file(prepared)
|
||||
return self._build_prepared_entity(persisted.prepared, persisted.entity)
|
||||
|
||||
async def _upsert_regular_file(self, file: IndexInputFile) -> _PreparedEntity:
|
||||
checksum = await self._resolve_checksum(file)
|
||||
@@ -405,6 +440,7 @@ class BatchIndexer:
|
||||
return _PreparedEntity(
|
||||
path=file.path,
|
||||
entity_id=updated.id,
|
||||
permalink=updated.permalink,
|
||||
checksum=checksum,
|
||||
content_type=file.content_type,
|
||||
search_content=None,
|
||||
@@ -495,6 +531,92 @@ class BatchIndexer:
|
||||
|
||||
# --- Helpers ---
|
||||
|
||||
async def _persist_markdown_file(
|
||||
self,
|
||||
prepared: _PreparedMarkdownFile,
|
||||
*,
|
||||
is_new: bool | None = None,
|
||||
) -> _PersistedMarkdownFile:
|
||||
existing = await self.entity_repository.get_by_file_path(
|
||||
prepared.file.path,
|
||||
load_relations=False,
|
||||
)
|
||||
if is_new is None:
|
||||
is_new = existing is None
|
||||
entity = await self.entity_service.upsert_entity_from_markdown(
|
||||
Path(prepared.file.path),
|
||||
prepared.markdown,
|
||||
is_new=is_new,
|
||||
)
|
||||
prepared = await self._reconcile_persisted_permalink(prepared, entity)
|
||||
updated = await self.entity_repository.update(
|
||||
entity.id,
|
||||
self._entity_metadata_updates(prepared.file, prepared.final_checksum),
|
||||
)
|
||||
if updated is None:
|
||||
raise ValueError(f"Failed to update markdown entity metadata for {prepared.file.path}")
|
||||
return _PersistedMarkdownFile(prepared=prepared, entity=updated)
|
||||
|
||||
async def _reconcile_persisted_permalink(
|
||||
self,
|
||||
prepared: _PreparedMarkdownFile,
|
||||
entity: Entity,
|
||||
) -> _PreparedMarkdownFile:
|
||||
# Trigger: the source file started without frontmatter and sync is configured
|
||||
# to leave frontmatterless files alone.
|
||||
# Why: upsert may still assign a DB permalink even when disk content should stay untouched.
|
||||
# Outcome: skip reconciliation writes that would silently inject frontmatter.
|
||||
if (
|
||||
self.app_config.disable_permalinks
|
||||
or (
|
||||
not prepared.file_contains_frontmatter
|
||||
and not self.app_config.ensure_frontmatter_on_sync
|
||||
)
|
||||
or entity.permalink is None
|
||||
or entity.permalink == prepared.markdown.frontmatter.permalink
|
||||
):
|
||||
return prepared
|
||||
|
||||
logger.debug(
|
||||
"Updating permalink after upsert conflict resolution",
|
||||
path=prepared.file.path,
|
||||
old_permalink=prepared.markdown.frontmatter.permalink,
|
||||
new_permalink=entity.permalink,
|
||||
)
|
||||
prepared.markdown.frontmatter.metadata["permalink"] = entity.permalink
|
||||
write_result = await self.file_writer.write_frontmatter(
|
||||
IndexFrontmatterUpdate(
|
||||
path=prepared.file.path,
|
||||
metadata={"permalink": entity.permalink},
|
||||
)
|
||||
)
|
||||
return _PreparedMarkdownFile(
|
||||
file=prepared.file,
|
||||
content=write_result.content,
|
||||
final_checksum=write_result.checksum,
|
||||
markdown=prepared.markdown,
|
||||
file_contains_frontmatter=prepared.file_contains_frontmatter,
|
||||
)
|
||||
|
||||
def _build_prepared_entity(
|
||||
self,
|
||||
prepared: _PreparedMarkdownFile,
|
||||
entity: Entity,
|
||||
) -> _PreparedEntity:
|
||||
return _PreparedEntity(
|
||||
path=prepared.file.path,
|
||||
entity_id=entity.id,
|
||||
permalink=entity.permalink,
|
||||
checksum=prepared.final_checksum,
|
||||
content_type=prepared.file.content_type,
|
||||
search_content=(
|
||||
prepared.markdown.content
|
||||
if prepared.markdown.content is not None
|
||||
else remove_frontmatter(prepared.content)
|
||||
),
|
||||
markdown_content=prepared.content,
|
||||
)
|
||||
|
||||
async def _resolve_checksum(self, file: IndexInputFile) -> str:
|
||||
if file.checksum is not None:
|
||||
return file.checksum
|
||||
|
||||
@@ -4,7 +4,10 @@ from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime
|
||||
from typing import Any, Protocol
|
||||
from typing import Any, Protocol, TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING: # pragma: no cover
|
||||
from basic_memory.models import Entity
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
@@ -75,6 +78,19 @@ class IndexedEntity:
|
||||
markdown_content: str | None = None
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class SyncedMarkdownFile:
|
||||
"""Canonical result for syncing one markdown file end-to-end."""
|
||||
|
||||
entity: Entity
|
||||
checksum: str
|
||||
markdown_content: str
|
||||
file_path: str
|
||||
content_type: str
|
||||
updated_at: datetime
|
||||
size: int
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class IndexingBatchResult:
|
||||
"""Outcome for one batch execution."""
|
||||
|
||||
@@ -273,6 +273,9 @@ class FileService:
|
||||
logger.warning("File not found", operation="read_file_content", path=str(full_path))
|
||||
raise
|
||||
except Exception as e:
|
||||
if isinstance(e, FileNotFoundError):
|
||||
logger.warning("File not found", operation="read_file", path=str(full_path))
|
||||
raise
|
||||
logger.exception("File read error", path=str(full_path), error=str(e))
|
||||
raise FileOperationError(f"Failed to read file: {e}")
|
||||
|
||||
@@ -366,6 +369,9 @@ class FileService:
|
||||
)
|
||||
return content, checksum
|
||||
|
||||
except FileNotFoundError as e:
|
||||
logger.warning("File not found", operation="read_file", path=str(full_path))
|
||||
raise FileOperationError(f"Failed to read file: {e}") from e
|
||||
except Exception as e:
|
||||
logger.exception("File read error", path=str(full_path), error=str(e))
|
||||
raise FileOperationError(f"Failed to read file: {e}")
|
||||
|
||||
@@ -18,8 +18,14 @@ from sqlalchemy.exc import IntegrityError
|
||||
from basic_memory import telemetry
|
||||
from basic_memory import db
|
||||
from basic_memory.config import BasicMemoryConfig, ConfigManager
|
||||
from basic_memory.file_utils import compute_checksum, has_frontmatter
|
||||
from basic_memory.indexing import BatchIndexer, IndexFileMetadata, IndexInputFile, IndexProgress
|
||||
from basic_memory.file_utils import ParseError, compute_checksum, remove_frontmatter
|
||||
from basic_memory.indexing import (
|
||||
BatchIndexer,
|
||||
IndexFileMetadata,
|
||||
IndexInputFile,
|
||||
IndexProgress,
|
||||
SyncedMarkdownFile,
|
||||
)
|
||||
from basic_memory.indexing.batching import build_index_batches
|
||||
from basic_memory.indexing.models import (
|
||||
IndexedEntity,
|
||||
@@ -1052,91 +1058,103 @@ class SyncService:
|
||||
Returns:
|
||||
Tuple of (entity, checksum)
|
||||
"""
|
||||
# Parse markdown first to get any existing permalink
|
||||
synced = await self.sync_one_markdown_file(path, new=new, index_search=False)
|
||||
return synced.entity, synced.checksum
|
||||
|
||||
async def sync_one_markdown_file(
|
||||
self,
|
||||
path: str,
|
||||
*,
|
||||
new: bool = True,
|
||||
index_search: bool = True,
|
||||
) -> SyncedMarkdownFile:
|
||||
"""Sync one markdown file and return the final canonical file state.
|
||||
|
||||
This method is the fail-fast single-file primitive for callers such as
|
||||
cloud workers. It does not swallow unexpected exceptions.
|
||||
"""
|
||||
logger.debug(f"Parsing markdown file, path: {path}, new: {new}")
|
||||
|
||||
file_content = await self.file_service.read_file_content(path)
|
||||
file_contains_frontmatter = has_frontmatter(file_content)
|
||||
|
||||
# Get file timestamps for tracking modification times
|
||||
try:
|
||||
initial_markdown_bytes = await self.file_service.read_file_bytes(path)
|
||||
except FileOperationError as exc:
|
||||
# Trigger: FileService wraps binary read failures in FileOperationError.
|
||||
# Why: sync_file() treats bare FileNotFoundError as a deletion race and cleans up the DB row.
|
||||
# Outcome: preserve that contract while still hashing the exact bytes we loaded.
|
||||
if isinstance(exc.__cause__, FileNotFoundError):
|
||||
raise exc.__cause__ from exc
|
||||
raise
|
||||
initial_markdown_content = initial_markdown_bytes.decode("utf-8")
|
||||
file_metadata = await self.file_service.get_file_metadata(path)
|
||||
created = file_metadata.created_at
|
||||
modified = file_metadata.modified_at
|
||||
|
||||
# Parse markdown content with file metadata (avoids redundant file read/stat)
|
||||
# This enables cloud implementations (S3FileService) to provide metadata from head_object
|
||||
abs_path = self.file_service.base_path / path
|
||||
entity_markdown = await self.entity_parser.parse_markdown_content(
|
||||
file_path=abs_path,
|
||||
content=file_content,
|
||||
mtime=file_metadata.modified_at.timestamp(),
|
||||
ctime=file_metadata.created_at.timestamp(),
|
||||
initial_checksum = await compute_checksum(initial_markdown_bytes)
|
||||
indexed = await self.batch_indexer.index_markdown_file(
|
||||
IndexInputFile(
|
||||
path=path,
|
||||
size=file_metadata.size,
|
||||
checksum=initial_checksum,
|
||||
content_type=self.file_service.content_type(path),
|
||||
last_modified=file_metadata.modified_at,
|
||||
created_at=file_metadata.created_at,
|
||||
content=initial_markdown_bytes,
|
||||
),
|
||||
new=new,
|
||||
index_search=False,
|
||||
)
|
||||
|
||||
# Trigger: markdown file has no frontmatter and frontmatter enforcement is enabled
|
||||
# Why: watch/sync consumers rely on normalized metadata and stable permalinks
|
||||
# Outcome: file is updated in-place with derived title/type/permalink metadata
|
||||
if not file_contains_frontmatter and self.app_config.ensure_frontmatter_on_sync:
|
||||
permalink = await self.entity_service.resolve_permalink(
|
||||
path, markdown=entity_markdown, skip_conflict_check=True
|
||||
)
|
||||
frontmatter_updates = {
|
||||
"title": entity_markdown.frontmatter.title,
|
||||
"type": entity_markdown.frontmatter.type,
|
||||
"permalink": permalink,
|
||||
}
|
||||
await self.file_service.update_frontmatter(path, frontmatter_updates)
|
||||
entity_markdown.frontmatter.metadata.update(frontmatter_updates)
|
||||
|
||||
# if the file contains frontmatter, resolve a permalink (unless disabled)
|
||||
if file_contains_frontmatter and not self.app_config.disable_permalinks:
|
||||
# Resolve permalink - skip conflict checks during bulk sync for performance
|
||||
permalink = await self.entity_service.resolve_permalink(
|
||||
path, markdown=entity_markdown, skip_conflict_check=True
|
||||
)
|
||||
|
||||
# If permalink changed, update the file
|
||||
if permalink != entity_markdown.frontmatter.permalink:
|
||||
logger.debug(
|
||||
f"Updating permalink for path: {path}, old_permalink: {entity_markdown.frontmatter.permalink}, new_permalink: {permalink}"
|
||||
)
|
||||
|
||||
entity_markdown.frontmatter.metadata["permalink"] = permalink
|
||||
await self.file_service.update_frontmatter(path, {"permalink": permalink})
|
||||
|
||||
# Create/update entity and relations in one path
|
||||
logger.debug(f"{'Creating' if new else 'Updating'} entity from markdown, path={path}")
|
||||
entity = await self.entity_service.upsert_entity_from_markdown(
|
||||
Path(path), entity_markdown, is_new=new
|
||||
final_markdown_content = (
|
||||
indexed.markdown_content
|
||||
if indexed.markdown_content is not None
|
||||
else initial_markdown_content
|
||||
)
|
||||
|
||||
# After updating relations, we need to compute the checksum again
|
||||
# This is necessary for files with wikilinks to ensure consistent checksums
|
||||
# after relation processing is complete
|
||||
final_checksum = await self.file_service.compute_checksum(path)
|
||||
|
||||
# Update checksum, timestamps, and file metadata from file system
|
||||
# Store mtime/size for efficient change detection in future scans
|
||||
# This ensures temporal ordering in search and recent activity uses actual file modification times
|
||||
await self.entity_repository.update(
|
||||
entity.id,
|
||||
file_metadata = await self.file_service.get_file_metadata(path)
|
||||
refreshed_entities = await self.entity_repository.find_by_ids([indexed.entity_id])
|
||||
if len(refreshed_entities) != 1: # pragma: no cover
|
||||
raise ValueError(f"Failed to reload synced markdown entity for {path}")
|
||||
# Trigger: markdown sync may have rewritten frontmatter after the initial file metadata load.
|
||||
# Why: the batch indexer persisted checksum/path data from the pre-rewrite IndexInputFile.
|
||||
# Outcome: refresh size and mtime from the file as it actually exists on disk now.
|
||||
updated_entity = await self.entity_repository.update(
|
||||
refreshed_entities[0].id,
|
||||
{
|
||||
"checksum": final_checksum,
|
||||
"created_at": created,
|
||||
"updated_at": modified,
|
||||
"checksum": indexed.checksum,
|
||||
"created_at": file_metadata.created_at,
|
||||
"updated_at": file_metadata.modified_at,
|
||||
"mtime": file_metadata.modified_at.timestamp(),
|
||||
"size": file_metadata.size,
|
||||
},
|
||||
)
|
||||
if updated_entity is None: # pragma: no cover
|
||||
raise ValueError(f"Failed to update markdown entity metadata for {path}")
|
||||
|
||||
if index_search:
|
||||
# Trigger: markdown may start with '---' as a thematic break or malformed
|
||||
# frontmatter that the parser already treated as plain content.
|
||||
# Why: one-file sync should not fail after the entity upsert just because
|
||||
# strict frontmatter stripping rejects that exact text shape.
|
||||
# Outcome: fall back to indexing the raw markdown content for these cases.
|
||||
try:
|
||||
search_content = remove_frontmatter(final_markdown_content)
|
||||
except ParseError:
|
||||
search_content = final_markdown_content
|
||||
await self.search_service.index_entity_data(
|
||||
updated_entity,
|
||||
content=search_content,
|
||||
)
|
||||
|
||||
logger.debug(
|
||||
f"Markdown sync completed: path={path}, entity_id={entity.id}, "
|
||||
f"observation_count={len(entity.observations)}, relation_count={len(entity.relations)}, "
|
||||
f"checksum={final_checksum[:8]}"
|
||||
f"Markdown sync completed: path={path}, entity_id={updated_entity.id}, "
|
||||
f"observation_count={len(updated_entity.observations)}, "
|
||||
f"relation_count={len(updated_entity.relations)}, checksum={indexed.checksum[:8]}"
|
||||
)
|
||||
|
||||
# Return the final checksum to ensure everything is consistent
|
||||
return entity, final_checksum
|
||||
return SyncedMarkdownFile(
|
||||
entity=updated_entity,
|
||||
checksum=indexed.checksum,
|
||||
markdown_content=final_markdown_content,
|
||||
file_path=path,
|
||||
content_type=self.file_service.content_type(path),
|
||||
updated_at=file_metadata.modified_at,
|
||||
size=file_metadata.size,
|
||||
)
|
||||
|
||||
async def sync_regular_file(self, path: str, new: bool = True) -> Tuple[Optional[Entity], str]:
|
||||
"""Sync a non-markdown file with basic tracking.
|
||||
|
||||
Reference in New Issue
Block a user