From 09e98e794b0dd7cbddd8c5f895d4fe0d1f1dc2d1 Mon Sep 17 00:00:00 2001 From: phernandez Date: Fri, 3 Jan 2025 15:17:52 -0600 Subject: [PATCH] knowledge sync tests wip --- .../services/sync/file_change_scanner.py | 44 ++-- .../services/sync/sync_service.py | 32 +-- src/basic_memory/services/sync/utils.py | 4 +- tests/cli/test_status.py | 6 +- .../services/sync/test_file_change_scanner.py | 8 +- tests/services/sync/test_sync_service.py | 201 +++++++++++++++--- 6 files changed, 223 insertions(+), 72 deletions(-) diff --git a/src/basic_memory/services/sync/file_change_scanner.py b/src/basic_memory/services/sync/file_change_scanner.py index e736a0d2..23edd120 100644 --- a/src/basic_memory/services/sync/file_change_scanner.py +++ b/src/basic_memory/services/sync/file_change_scanner.py @@ -8,7 +8,7 @@ from loguru import logger from basic_memory.models import Document, Entity from basic_memory.repository.document_repository import DocumentRepository from basic_memory.repository.entity_repository import EntityRepository -from basic_memory.services.sync.utils import FileState, SyncReport, ScanResult +from basic_memory.services.sync.utils import DbState, SyncReport, ScanResult from basic_memory.utils.file_utils import compute_checksum @@ -58,7 +58,7 @@ class FileChangeScanner: checksum = await compute_checksum(content) if checksum: # Only store valid checksums - result.files[rel_path] = FileState( + result.files[rel_path] = DbState( path=rel_path, checksum=checksum ) @@ -81,14 +81,14 @@ class FileChangeScanner: async def find_changes( self, directory: Path, - db_records: Dict[str, FileState] + db_records: Dict[str, DbState] ) -> SyncReport: """ Find changes between filesystem and database. Args: directory: Directory to check - get_records: Function to get database records + db_records: dict mapping file_path to DbState(path_id, checksum) Returns: SyncReport detailing changes @@ -98,34 +98,34 @@ class FileChangeScanner: current_files = scan_result.files logger.debug("Current files from filesystem:") - for path, state in sorted(current_files.items()): - logger.debug(f" {path} ({state.checksum[:8]})") + for file_path, state in sorted(current_files.items()): + logger.debug(f" {file_path} ({state.checksum[:8]})") logger.debug("Files from database:") - for path, state in sorted(db_records.items()): - logger.debug(f" {path} ({state.checksum[:8] if state.checksum else 'no checksum'})") + for file_path, state in sorted(db_records.items()): + logger.debug(f" {file_path} ({state.checksum[:8] if state.checksum else 'no checksum'})") # Build report report = SyncReport() # Add current checksums for display - for path, state in current_files.items(): - report.checksums[path] = state.checksum + for file_path, state in current_files.items(): + report.checksums[file_path] = state.checksum # Find new and modified files - for path, curr_state in current_files.items(): - if path not in db_records: - report.new.add(path) - elif curr_state.checksum != db_records[path].checksum: - report.modified.add(path) + for file_path, curr_state in current_files.items(): + if file_path not in db_records: + report.new.add(file_path) + elif curr_state.checksum != db_records[file_path].checksum: + report.modified.add(file_path) # Find deleted files - need to be deleted from db # either not in current_files, or # db row has no checksum - for db_path, file_state in db_records.items(): - if db_path not in current_files or file_state.checksum is None: - report.deleted.add(db_path) + for db_file_path, db_state in db_records.items(): + if db_file_path not in current_files: + report.deleted.add(db_file_path) # Log summary logger.debug(f"Changes found: {report.total_changes}") @@ -135,12 +135,12 @@ class FileChangeScanner: if scan_result.errors: logger.warning("Files skipped due to errors:") - for path, error in scan_result.errors.items(): - logger.warning(f" {path}: {error}") + for file_path, error in scan_result.errors.items(): + logger.warning(f" {file_path}: {error}") return report - async def get_db_file_paths(self, db_records: Sequence[Document | Entity]) -> Dict[str, FileState]: + async def get_db_file_paths(self, db_records: Sequence[Document | Entity]) -> Dict[str, DbState]: """Get file_path and checksums from database. Args: db_records: database records @@ -148,7 +148,7 @@ class FileChangeScanner: Dict mapping file paths to FileState :param db_records: the data from the db """ - return {r.file_path: FileState(path=r.path_id, checksum=r.checksum) for r in db_records} + return {r.file_path: DbState(path=r.path_id, checksum=r.checksum) for r in db_records} async def find_document_changes(self, directory: Path) -> SyncReport: """Find changes in document directory.""" diff --git a/src/basic_memory/services/sync/sync_service.py b/src/basic_memory/services/sync/sync_service.py index fdb68cbe..87908921 100644 --- a/src/basic_memory/services/sync/sync_service.py +++ b/src/basic_memory/services/sync/sync_service.py @@ -54,29 +54,31 @@ class SyncService: logger.info(f"Found {changes.total_changes} knowledge changes") # Handle deletions first - for path_id in changes.deleted: - logger.debug(f"Deleting entity: {path_id}") - await self.knowledge_sync_service.delete_entity_by_file_path(path_id) + # remove rows from db for files no longer present + for file_path in changes.deleted: + logger.debug(f"Deleting entity from db: {file_path}") + await self.knowledge_sync_service.delete_entity_by_file_path(file_path) # Parse files that need updating parsed_entities = {} - for path_id in [*changes.new, *changes.modified]: - entity = await self.knowledge_parser.parse_file(directory / path_id) - parsed_entities[path_id] = entity + for file_path in [*changes.new, *changes.modified]: + entity_markdown = await self.knowledge_parser.parse_file(directory / file_path) + parsed_entities[file_path] = entity_markdown # First pass: Create/update entities - for path_id, entity in parsed_entities.items(): - if path_id in changes.new: - logger.debug(f"Creating new entity: {path_id}") - await self.knowledge_sync_service.create_entity_and_observations(entity) + for file_path, entity_markdown in parsed_entities.items(): + if file_path in changes.new: + logger.debug(f"Creating new entity_markdown: {file_path}") + await self.knowledge_sync_service.create_entity_and_observations(entity_markdown) else: - logger.debug(f"Updating entity: {path_id}") - await self.knowledge_sync_service.update_entity_and_observations(entity) + path_id = entity_markdown.frontmatter.id + logger.debug(f"Updating entity_markdown: {path_id}") + await self.knowledge_sync_service.update_entity_and_observations(path_id, entity_markdown) # Second pass: Process relations - for path_id, entity in parsed_entities.items(): - logger.debug(f"Updating relations for: {path_id}") - await self.knowledge_sync_service.update_entity_relations(entity, checksum=changes.checksums[path_id]) + for file_path, entity_markdown in parsed_entities.items(): + logger.debug(f"Updating relations for: {file_path}") + await self.knowledge_sync_service.update_entity_relations(entity_markdown, checksum=changes.checksums[file_path]) async def sync(self, root_dir: Path) -> None: """Sync all files with database.""" diff --git a/src/basic_memory/services/sync/utils.py b/src/basic_memory/services/sync/utils.py index 7495f6eb..df902b39 100644 --- a/src/basic_memory/services/sync/utils.py +++ b/src/basic_memory/services/sync/utils.py @@ -5,7 +5,7 @@ from typing import Set, Dict, Optional @dataclass -class FileState: +class DbState: """State of a file including path and checksum info.""" path: str checksum: str @@ -14,7 +14,7 @@ class FileState: @dataclass class ScanResult: """Result of scanning a directory.""" - files: Dict[str, FileState] = field(default_factory=dict) + files: Dict[str, DbState] = field(default_factory=dict) errors: Dict[str, str] = field(default_factory=dict) # path -> error message diff --git a/tests/cli/test_status.py b/tests/cli/test_status.py index 8ee99bf4..f1880a49 100644 --- a/tests/cli/test_status.py +++ b/tests/cli/test_status.py @@ -6,7 +6,7 @@ from rich.console import Console from io import StringIO from basic_memory.cli.commands.status import display_changes, run_status -from basic_memory.services.sync.utils import SyncReport, FileState +from basic_memory.services.sync.utils import SyncReport, DbState from basic_memory.utils.file_utils import compute_checksum @@ -35,7 +35,7 @@ async def test_display_compact_changes(console): modified={"docs/mod.md"}, deleted={"old/deleted.md"}, moved={ - "new/location.md": FileState( + "new/location.md": DbState( path="new/location.md", checksum="abc123", moved_from="old/location.md" ) }, @@ -59,7 +59,7 @@ async def test_display_verbose_changes(console): modified={"docs/mod.md"}, deleted={"old/deleted.md"}, moved={ - "new/location.md": FileState( + "new/location.md": DbState( path="new/location.md", checksum="abc123def", # 8 chars for display moved_from="old/location.md", diff --git a/tests/services/sync/test_file_change_scanner.py b/tests/services/sync/test_file_change_scanner.py index 1db834bf..0471cf38 100644 --- a/tests/services/sync/test_file_change_scanner.py +++ b/tests/services/sync/test_file_change_scanner.py @@ -5,7 +5,7 @@ from typing import AsyncGenerator from basic_memory.repository import DocumentRepository, EntityRepository from basic_memory.services import FileChangeScanner -from basic_memory.services.sync.utils import FileState +from basic_memory.services.sync.utils import DbState from basic_memory.utils.file_utils import compute_checksum from basic_memory.models import Document, Entity @@ -51,7 +51,7 @@ async def test_scan_with_mixed_files( assert len(result.errors) == 0 # Verify FileState objects - assert isinstance(result.files["doc.md"], FileState) + assert isinstance(result.files["doc.md"], DbState) assert result.files["doc.md"].path == "doc.md" assert result.files["doc.md"].checksum is not None @@ -108,7 +108,7 @@ async def test_detect_modified_file( # Create DB state with original checksum original_checksum = await compute_checksum(content) db_records = { - path: FileState(path=path, checksum=original_checksum) + path: DbState(path=path, checksum=original_checksum) } # Modify file @@ -133,7 +133,7 @@ async def test_detect_deleted_files( # Create DB state with file that doesn't exist db_records = { - path: FileState(path=path, checksum="any-checksum") + path: DbState(path=path, checksum="any-checksum") } changes = await file_change_scanner.find_changes( diff --git a/tests/services/sync/test_sync_service.py b/tests/services/sync/test_sync_service.py index 6cf6f5d3..f68b2a02 100644 --- a/tests/services/sync/test_sync_service.py +++ b/tests/services/sync/test_sync_service.py @@ -1,5 +1,6 @@ -"""Test sync service.""" +"""Test general sync behavior.""" +import asyncio from pathlib import Path import pytest import pytest_asyncio @@ -8,7 +9,7 @@ from basic_memory.config import ProjectConfig from basic_memory.services import DocumentService, EntityService, FileChangeScanner from basic_memory.services.sync.knowledge_sync_service import KnowledgeSyncService from basic_memory.services.sync.sync_service import SyncService -from basic_memory.markdown import KnowledgeParser, EntityMarkdown +from basic_memory.markdown import KnowledgeParser from basic_memory.models import Document, Entity, Observation @@ -19,46 +20,194 @@ async def create_test_file(path: Path, content: str = "test content") -> None: @pytest.mark.asyncio -async def test_sync_empty_directories(sync_service: SyncService, test_config: ProjectConfig): +async def test_sync_empty_directories( + sync_service: SyncService, + test_config: ProjectConfig +): """Test syncing empty directories.""" await sync_service.sync(test_config.home) # Should not raise exceptions for empty dirs assert (test_config.documents_dir).exists() assert (test_config.knowledge_dir).exists() - + @pytest.mark.asyncio -async def test_sync_deletes( +async def test_sync_file_modified_during_sync( + sync_service: SyncService, + test_config: ProjectConfig +): + """Test handling of files that change during sync process.""" + # Create initial files + doc_path = test_config.documents_dir / "changing.md" + await create_test_file(doc_path, "Initial content") + + # Setup async modification during sync + async def modify_file(): + await asyncio.sleep(0.1) # Small delay to ensure sync has started + doc_path.write_text("Modified during sync") + + # Run sync and modification concurrently + await asyncio.gather( + sync_service.sync(test_config.home), + modify_file() + ) + + # Verify final state + doc = await sync_service.document_service.repository.find_by_path_id("changing.md") + assert doc is not None + # File should have a checksum, even if it's from either version + assert doc.checksum is not None + + +@pytest.mark.asyncio +async def test_sync_null_checksum_cleanup( sync_service: SyncService, test_config: ProjectConfig, - document_service: DocumentService, entity_service: EntityService ): - """Test sync handles deletions.""" - # Add records to DB that don't exist in filesystem - doc = Document( - path_id="deleted.md", - file_path="deleted.md", - checksum="12345678" - ) - await document_service.repository.add(doc) - + """Test handling of entities with null checksums from incomplete syncs.""" + # Create entity with null checksum (simulating incomplete sync) entity = Entity( - path_id="concept/deleted", - name="Deleted", + path_id="concept/incomplete", + name="Incomplete", entity_type="concept", - file_path="concept/deleted.md", - checksum = "12345678" + file_path="concept/incomplete.md", + checksum=None # Null checksum ) await entity_service.repository.add(entity) - + + # Create corresponding file + content = """ +--- +type: concept +id: concept/incomplete +created: 2024-01-01 +modified: 2024-01-01 +--- +# Incomplete Entity + +## Observations +- Testing cleanup +""" + await create_test_file(test_config.knowledge_dir / "concept/incomplete.md", content) + # Run sync await sync_service.sync(test_config.home) + + # Verify entity was properly synced + updated = await entity_service.get_by_path_id("concept/incomplete") + assert updated.checksum is not None + + +@pytest.mark.asyncio +async def test_sync_mixed_document_types( + sync_service: SyncService, + test_config: ProjectConfig +): + """Test handling documents and knowledge files with similar paths.""" + # Create a document + doc_content = "# Regular Document" + await create_test_file(test_config.documents_dir / "test.md", doc_content) + + # Create a knowledge file + knowledge_content = """ +--- +type: concept +id: concept/test +created: 2024-01-01 +modified: 2024-01-01 +--- +# Knowledge File + +## Observations +- This is a test +""" + await create_test_file(test_config.knowledge_dir / "concept/test.md", knowledge_content) + + # Run sync + await sync_service.sync(test_config.home) + + # Verify both types exist correctly + doc = await sync_service.document_service.repository.find_by_path_id("test.md") + assert doc is not None + + entity = await sync_service.knowledge_sync_service.entity_service.get_by_path_id("concept/test") + assert entity is not None + assert len(entity.observations) == 1 + + +@pytest.mark.asyncio +async def test_sync_performance_large_files( + sync_service: SyncService, + test_config: ProjectConfig +): + """Test sync performance with larger files.""" + # Create a large document with many lines + large_doc = ["Line " + str(i) for i in range(1000)] + await create_test_file( + test_config.documents_dir / "large.md", + "\n".join(large_doc) + ) + + # Create a knowledge file with many observations + observations = [f"- Observation {i}" for i in range(100)] + knowledge_content = f""" +--- +type: concept +id: concept/large +created: 2024-01-01 +modified: 2024-01-01 +--- +# Large Entity + +## Observations +{chr(10).join(observations)} +""" + await create_test_file(test_config.knowledge_dir / "concept/large.md", knowledge_content) + + # Time the sync + start_time = asyncio.get_event_loop().time() + await sync_service.sync(test_config.home) + duration = asyncio.get_event_loop().time() - start_time + + # Verify everything synced + doc = await sync_service.document_service.repository.find_by_path_id("large.md") + assert doc is not None + + entity = await sync_service.knowledge_sync_service.entity_service.get_by_path_id("concept/large") + assert entity is not None + assert len(entity.observations) == 100 + + # Basic performance check - should sync in reasonable time + assert duration < 5 # Should complete in under 5 seconds + + +@pytest.mark.asyncio +async def test_sync_concurrent_updates( + sync_service: SyncService, + test_config: ProjectConfig +): + """Test handling multiple concurrent sync operations.""" + # Create initial files + doc1_path = test_config.documents_dir / "doc1.md" + doc2_path = test_config.documents_dir / "doc2.md" - # Verify deletions - docs = await document_service.repository.find_all() - assert len(docs) == 0 - - entities = await entity_service.repository.find_all() - assert len(entities) == 0 \ No newline at end of file + await create_test_file(doc1_path, "Doc 1 content") + await create_test_file(doc2_path, "Doc 2 content") + + # Run multiple syncs concurrently + results = await asyncio.gather( + sync_service.sync(test_config.home), + sync_service.sync(test_config.home), + return_exceptions=True + ) + + # Check no exceptions were raised + for r in results: + assert not isinstance(r, Exception) + + # Verify final state + docs = await sync_service.document_service.repository.find_all() + assert len(docs) == 2 + assert {d.path_id for d in docs} == {"doc1.md", "doc2.md"} \ No newline at end of file