test memory_service.delete operations

src/basic_memory/fileio.py

@@ -98,7 +98,7 @@ async def write_entity_file(project_entities_path: Path, entity_id: str, entity:
     except Exception as e:
         raise FileOperationError(f"Failed to finalize entity file: {str(e)}") from e
 
-    logger.debug(f"Wrote entity file: {entity_id}")
+    logger.debug(f"Wrote entity {entity_id} file: {entity_path}")
     return True
 
 

src/basic_memory/repository/relation_repository.py

@@ -11,7 +11,13 @@ class RelationRepository(Repository[Relation]):
     
     def __init__(self, session):
         super().__init__(session, Relation)
-    
+
+    async def find_by_entity(self, from_entity_id: str) -> Sequence[Relation]:
+        """Find all relations from a specific entity."""
+        query = select(Relation).filter(Relation.from_id == from_entity_id)
+        result = await self.execute_query(query)
+        return result.scalars().all()
+
     async def find_by_entities(self, from_id: str, to_id: str) -> Sequence[Relation]:
         """Find all relations between two entities."""
         query = select(Relation).filter(

src/basic_memory/services/memory_service.py

@@ -7,7 +7,7 @@ from basic_memory.models import Entity as EntityModel, Observation as Observatio
 from basic_memory.schemas import (
     AddObservationsRequest, Entity, Relation
 )
-from basic_memory.fileio import write_entity_file, read_entity_file, EntityNotFoundError
+from basic_memory.fileio import write_entity_file, read_entity_file, EntityNotFoundError, get_entity_path
 from basic_memory.services import EntityService, RelationService, ObservationService
 from loguru import logger
 
@@ -32,6 +32,9 @@ class MemoryService:
         self.observation_service = observation_service
         logger.debug(f"Initialized MemoryService with path: {project_path}")
 
+    def get_entity_file_path(self, entity_id: str) -> Path:
+        return get_entity_path(self.entities_path, entity_id)
+
     async def create_entities(self, entities_in: List[Entity]) -> List[EntityModel]:
         """Create multiple entities with their observations."""
         logger.debug(f"Creating {len(entities_in)} entities")
@@ -183,14 +186,111 @@ class MemoryService:
             logger.exception(f"Failed to add observations to entity: {observations_in.entity_id}")
             raise
 
-    async def delete_entities(self, entity_names: List[str]) -> None:
-       pass
+    async def delete_entities(self, entity_ids: List[str]) -> bool:
+        """Delete entities and their files."""
+        logger.debug(f"Deleting entities: {entity_ids}")
+        try:
+            deleted = False
+            for entity_id in entity_ids:
+                # First read the entity to make sure it exists
+                try:
+                    entity = await read_entity_file(self.entities_path, entity_id)
+                except EntityNotFoundError:
+                    logger.debug(f"Entity file not found: {entity_id}")
+                    continue
 
-    async def delete_observations(self, deletions: List[Dict[str, Any]]) -> None:
-        pass
+                # Delete the file first since it's source of truth
+                file_path = self.get_entity_file_path(entity_id)
+                if file_path.exists():
+                    file_path.unlink()
+                    logger.debug(f"Deleted entity {entity_id} file: {file_path}")
+                
+                # Then update database
+                result = await self.entity_service.delete_entity(entity_id)
+                if result:
+                    deleted = True
+                    logger.debug(f"Deleted entity from database: {entity_id}")
 
-    async def delete_relations(self, relations: List[Dict[str, Any]]) -> None:
-        pass
+            return deleted
+        except Exception:
+            logger.exception("Failed to delete entities")
+            raise
+
+    async def delete_observations(self, entity_id: str, contents: List[str]) -> bool:
+        """Delete specific observations from an entity."""
+        logger.debug(f"Deleting observations from entity: {entity_id}")
+        try:
+            # First read the entity
+            entity = await read_entity_file(self.entities_path, entity_id)
+            
+            # Remove observations from entity
+            original_count = len(entity.observations)
+            entity.observations = [
+                obs for obs in entity.observations 
+                if obs not in contents
+            ]
+            
+            # Only write file if we actually removed anything
+            if len(entity.observations) < original_count:
+                # Write updated entity file first (source of truth)
+                await write_entity_file(self.entities_path, entity_id, entity)
+                logger.debug(f"Updated entity file: {entity_id}")
+                
+                # Then update database
+                result = await self.observation_service.delete_observations(entity_id, contents)
+                logger.debug(f"Deleted observations from database: {result}")
+                return True
+
+            return False
+        except Exception:
+            logger.exception("Failed to delete observations")
+            raise
+
+    async def delete_relations(self, relations: List[Dict[str, Any]]) -> bool:
+        """Delete relations between entities."""
+        logger.debug(f"Deleting relations: {relations}")
+        try:
+            deleted = False
+            for relation in relations:
+                # First read the source entity
+                try:
+                    from_entity = await read_entity_file(self.entities_path, relation['from_id'])
+                except EntityNotFoundError:
+                    logger.debug(f"Source entity not found: {relation['from_id']}")
+                    continue
+
+                # Update the entity's relations
+                if hasattr(from_entity, 'relations'):
+                    original_count = len(from_entity.relations)
+                    relation_type = relation.get('relation_type')
+                    
+                    if relation_type:
+                        from_entity.relations = [
+                            r for r in from_entity.relations
+                            if not (r.to_id == relation['to_id'] and r.relation_type == relation_type)
+                        ]
+                    else:
+                        from_entity.relations = [
+                            r for r in from_entity.relations
+                            if r.to_id != relation['to_id']
+                        ]
+
+                    # Only write file if we actually removed any relations
+                    if len(from_entity.relations) < original_count:
+                        # Write updated entity file first (source of truth)
+                        await write_entity_file(self.entities_path, from_entity.id, from_entity)
+                        logger.debug(f"Updated source entity file: {from_entity.id}")
+
+                        # Then update database
+                        result = await self.relation_service.delete_relations([relation])
+                        if result:
+                            deleted = True
+                            logger.debug("Deleted relations from database")
+
+            return deleted
+        except Exception:
+            logger.exception("Failed to delete relations")
+            raise
 
     async def read_graph(self) -> Sequence[EntityModel]:
         """Read the entire knowledge graph."""
@@ -214,32 +314,31 @@ class MemoryService:
             logger.exception(f"Failed to search nodes with query: {query}")
             raise
 
-    async def open_nodes(self, names: List[str]) -> List[Entity]:
+    async def open_nodes(self, entity_ids: List[str]) -> List[Entity]:
         """Get specific nodes and their relationships."""
-        logger.debug(f"Opening nodes: {names}")
+        logger.debug(f"Opening nodes entity_ids: {entity_ids}")
 
-        async def read_node(name: str) -> Optional[Entity]:
+        async def read_node(id: str) -> Optional[Entity]:
             try:
                 # Get ID from name first
-                logger.debug(f"Looking up entity: {name}")
-                db_entity = await self.entity_service.get_entity(name)
+                logger.debug(f"Looking up entity: {id}")
+                db_entity = await self.entity_service.get_entity(id)
                 if db_entity:
                     logger.debug(f"Found entity in DB: {db_entity.id}")
                     entity = await read_entity_file(self.entities_path, db_entity.id)
                     logger.debug(f"Read entity from filesystem: {entity.id}")
                     return entity
-                logger.debug(f"Entity not found: {name}")
+                logger.debug(f"Entity not found: {id}")
                 return None
             except Exception:
-                logger.exception(f"Failed to read node: {name}")
+                logger.exception(f"Failed to read node: {id}")
                 return None
 
         try:
-            entities = [entity for entity in await asyncio.gather(*(read_node(name) for name in names))
+            entities = [entity for entity in await asyncio.gather(*(read_node(id) for id in entity_ids))
                        if entity is not None]
             logger.debug(f"Opened {len(entities)} entities")
             return entities
         except Exception:
             logger.exception("Failed to open nodes")
-            raise
-
+            raise

tests/conftest.py

@@ -121,27 +121,3 @@ async def test_entity(entity_service):
     )
     return await entity_service.create_entity(entity_data)
 
-# Test data fixtures
-@pytest_asyncio.fixture
-def test_entity_data():
-    """Sample data for creating a test entity using camelCase (like MCP will)."""
-    return {
-        "entities": [{
-            "name": "Test Entity",
-            "entityType": "test",
-            "description": "",  # Empty string instead of None
-            "observations": [{"content": "This is a test observation"}]
-        }]
-    }
-
-@pytest_asyncio.fixture
-def test_entity_snake_case():
-    """Same test data but using snake_case to test schema flexibility."""
-    return {
-        "entities": [{
-            "name": "Test Entity",
-            "entity_type": "test",
-            "description": "",  # Empty string instead of None
-            "observations": [{"content": "This is a test observation"}]
-        }]
-    }

tests/services/test_memory_service.py

@@ -1,10 +1,13 @@
-"""Tests for the MemoryService class."""
+"""Tests for MemoryService delete operations."""
+
 import pytest
 from basic_memory.services import MemoryService
 from basic_memory.fileio import read_entity_file
 from basic_memory.schemas import CreateEntityRequest, CreateRelationsRequest, AddObservationsRequest, Relation
 
-test_entities_data = [
+from basic_memory.models import Relation
+
+test_create_entity_input = [
     {
         "name": "Test_Entity_1",
         "entity_type": "test",
@@ -17,11 +20,12 @@ test_entities_data = [
     }
 ]
 
+
 @pytest.mark.asyncio
 async def test_create_entities(memory_service: MemoryService):
     """Should create multiple entities in parallel with their observations."""
 
-    entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data})
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
     entities = await memory_service.create_entities(entity_input.entities)
 
     # Verify the SQLAlchemy models were created
@@ -44,14 +48,20 @@ async def test_create_entities(memory_service: MemoryService):
     # Verify files were created (returns Pydantic Entity)
     file_entity1 = await read_entity_file(memory_service.entities_path, entities[0].id)
     file_entity2 = await read_entity_file(memory_service.entities_path, entities[1].id)
-    
+
     assert file_entity1.name == "Test_Entity_1"
     assert file_entity2.name == "Test_Entity_2"
 
+    # Verify files are present
+    for entity in entities:
+        entity_file_path = memory_service.get_entity_file_path(entity.id)
+        assert entity_file_path.exists()
+
+
 @pytest.mark.asyncio
 async def test_add_observations(memory_service: MemoryService):
     """Should add observations to an existing entity."""
-    entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data})
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
     entities = await memory_service.create_entities([entity_input.entities[0]])
     entity = entities[0]
 
@@ -72,19 +82,20 @@ async def test_add_observations(memory_service: MemoryService):
     assert len(added_observations) == 2
     assert added_observations[0].content == "New observation 1"
     assert added_observations[0].context is None
-    assert added_observations[1].content == "New observation 2" 
+    assert added_observations[1].content == "New observation 2"
     assert added_observations[1].context is None
 
     # Verify file was updated - returns Pydantic Entity
     updated_entity = await read_entity_file(memory_service.entities_path, entity.id)
     assert len(updated_entity.observations) == 4  # 2 original + 2 new
-    #assert updated_entity.observations[2] == "New observation 1"
-    #assert updated_entity.observations[3] == "New observation 2"
+    # assert updated_entity.observations[2] == "New observation 1"
+    # assert updated_entity.observations[3] == "New observation 2"
 
     # Verify database - returns SQLAlchemy Entity
     db_entity = await memory_service.entity_service.get_entity(entity.id)
     assert len(db_entity.observations) == 4
 
+
 @pytest.mark.asyncio
 async def test_add_observations_nonexistent_entity(memory_service: MemoryService):
     """Should raise an appropriate error when adding observations to a non-existent entity."""
@@ -92,15 +103,16 @@ async def test_add_observations_nonexistent_entity(memory_service: MemoryService
         "entity_id": "nonexistent-id",
         "observations": ["Test observation"]
     }
-    
+
     with pytest.raises(Exception) as exc:  # We might want to define a specific error type
         observation_input = AddObservationsRequest.model_validate(observations_data)
         await memory_service.add_observations(observation_input)
 
+
 @pytest.mark.asyncio
 async def test_create_relations(memory_service: MemoryService):
     """Should create relations between entities and update both filesystem and database."""
-    entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data})
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
     entities = await memory_service.create_entities(entity_input.entities)
     entity1, entity2 = entities
 
@@ -179,11 +191,12 @@ async def test_create_relations(memory_service: MemoryService):
     assert len(db_entity2.outgoing_relations) == 1
     assert len(db_entity2.incoming_relations) == 1
 
+
 @pytest.mark.asyncio
 async def test_create_relations_with_invalid_entity_id(memory_service: MemoryService):
     """Should raise an appropriate error when trying to create relations with non-existent entity IDs."""
     # Create one entity - returns SQLAlchemy Entity
-    entity_input = CreateEntityRequest.model_validate({"entities": test_entities_data})
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
     entities = await memory_service.create_entities([entity_input.entities[0]])
     entity1 = entities[0]
 
@@ -193,6 +206,192 @@ async def test_create_relations_with_invalid_entity_id(memory_service: MemorySer
         "to_id": "nonexistent-id",
         "relation_type": "connects_to"
     }
-    
+
     with pytest.raises(Exception) as exc:  # We might want to define a specific error type
-        await memory_service.create_relations([Relation.model_validate(bad_relation)])
+        await memory_service.create_relations([Relation.model_validate(bad_relation)])
+
+
+@pytest.mark.asyncio
+async def test_delete_entities(memory_service: MemoryService):
+    """Test deleting an entity deletes file and database record."""
+    # Write the entity files
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
+    entities = await memory_service.create_entities(entity_input.entities)
+
+    # Verify files are present
+    for entity in entities:
+        entity_file_path = memory_service.get_entity_file_path(entity.id)
+        assert entity_file_path.exists()
+
+    # Delete the entities
+    result = await memory_service.delete_entities([entity.id for entity in entities])
+    assert result is True
+    
+    # Verify files are gone
+    for entity in entities:
+        assert not memory_service.get_entity_file_path(entity.id).exists()
+    
+    # Verify database records are deleted
+    for entity in entities:
+        deleted = await memory_service.entity_service.entity_repo.find_by_id(entity.id)
+        assert deleted is None
+
+@pytest.mark.asyncio
+async def test_delete_entity_cascades(memory_service):
+    """Test deleting an entity cascades to observations."""
+
+    # Write the entity files
+    create_entity_input = test_create_entity_input[0]
+    entity_input = CreateEntityRequest.model_validate({"entities": [create_entity_input]})
+    entities = await memory_service.create_entities(entity_input.entities)
+    assert len(entities) == 1
+    test_entity = entities[0]
+
+    # Delete the entity
+    result = await memory_service.delete_entities([test_entity.id])
+    assert result is True
+    
+    # Verify file is gone
+    assert not memory_service.get_entity_file_path(test_entity.id).exists()
+    
+    # Verify observations are gone from database
+    obsv = await memory_service.observation_service.observation_repo.find_by_entity(test_entity.id)
+    assert len(obsv) == 0
+
+    # Verify observations are gone from database
+    rels = await memory_service.relation_service.relation_repo.find_by_entity(test_entity.id)
+    assert len(rels) == 0
+
+@pytest.mark.asyncio
+async def test_delete_observations(memory_service):
+    """Test deleting specific observations."""
+
+    # Set up entity file with observations
+    create_entity_input = test_create_entity_input[0]
+    create_entity_input['observations'] = ["First observation", "Second observation", "Third observation"]
+    create_entity = CreateEntityRequest.model_validate({"entities": [create_entity_input]})
+
+    entities = await memory_service.create_entities(create_entity.entities)
+    assert len(entities) == 1
+    test_entity = entities[0]
+
+    # Delete two observations
+    to_delete = ["First observation", "Second observation"]
+    result = await memory_service.delete_observations(test_entity.id, to_delete)
+    assert result is True
+    
+    # Verify file updated
+    updated_entities = await memory_service.open_nodes([test_entity.id])
+    assert len(updated_entities) == 1
+    updated_entity = updated_entities[0]
+    
+    assert len(updated_entity.observations) == 1
+    assert updated_entity.observations[0] == "Third observation"
+    
+    # Verify database updated
+    remaining_db = await memory_service.observation_service.observation_repo.find_by_entity(test_entity.id)
+    assert len(remaining_db) == 1
+    assert remaining_db[0].content == "Third observation"
+
+@pytest.mark.asyncio
+async def test_delete_relations(memory_service):
+    """Test deleting relations between entities."""
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
+    entities = await memory_service.create_entities(entity_input.entities)
+    entity1, entity2 = entities
+
+    # Create test relations data using actual entity IDs
+    test_relations_data = [
+        {
+            "from_id": entity1.id,
+            "to_id": entity2.id,
+            "relation_type": "connects_to"
+        },
+        {
+            "from_id": entity2.id,
+            "to_id": entity1.id,
+            "relation_type": "references",
+            "context": "test context"
+        }
+    ]
+
+    # Create relations - returns List[models.Relation]
+    input_args = CreateRelationsRequest.model_validate({"relations": test_relations_data})
+    relations = await memory_service.create_relations(input_args.relations)
+    
+    # Delete the relation
+    to_delete = [{
+        "from_id": entity1.id,
+        "to_id": entity2.id,
+        "relation_type": "connects_to"
+    }]
+    result = await memory_service.delete_relations(to_delete)
+    assert result is True
+    
+    # Verify relation removed from source entity file
+    # Verify file updated
+    updated_entities = await memory_service.open_nodes([entity1.id])
+    assert len(updated_entities) == 1
+    updated_entity = updated_entities[0]
+
+    assert len(updated_entity.relations) == 0
+    
+    # Verify relation removed from database
+    relations = await memory_service.relation_service.relation_repo.find_by_entities(entity1.id, entity2.id)
+    assert len(relations) == 0
+
+@pytest.mark.asyncio
+async def test_delete_nonexistent_entity(memory_service):
+    """Test deleting an entity that doesn't exist."""
+    result = await memory_service.delete_entities(["nonexistent/id"])
+    assert result is False
+
+@pytest.mark.asyncio
+async def test_delete_nonexistent_observations(memory_service):
+    """Test deleting observations that don't exist."""
+
+    # Set up entity file with observations
+    create_entity_input = test_create_entity_input[0]
+    create_entity_input['observations'] = ["First observation", "Second observation", "Third observation"]
+    create_entity = CreateEntityRequest.model_validate({"entities": [create_entity_input]})
+
+    entities = await memory_service.create_entities(create_entity.entities)
+    assert len(entities) == 1
+    test_entity = entities[0]
+    
+    result = await memory_service.delete_observations(test_entity.id, ["Nonexistent"])
+    assert result is False
+
+@pytest.mark.asyncio
+async def test_delete_nonexistent_relations(memory_service):
+    """Test deleting relations that don't exist."""
+    entity_input = CreateEntityRequest.model_validate({"entities": test_create_entity_input})
+    entities = await memory_service.create_entities(entity_input.entities)
+    entity1, entity2 = entities
+
+    # Create test relations data using actual entity IDs
+    test_relations_data = [
+        {
+            "from_id": entity1.id,
+            "to_id": entity2.id,
+            "relation_type": "connects_to"
+        },
+        {
+            "from_id": entity2.id,
+            "to_id": entity1.id,
+            "relation_type": "references",
+            "context": "test context"
+        }
+    ]
+
+    # Create relations - returns List[models.Relation]
+    input_args = CreateRelationsRequest.model_validate({"relations": test_relations_data})
+    relations = await memory_service.create_relations(input_args.relations)
+    
+    to_delete = [{
+        "from_id": entity1.id,
+        "to_id": entity2.id,
+        "relation_type": "nonexistent"
+    }]
+    result = await memory_service.delete_relations(to_delete)
+    assert result is False

tests/services/test_observation_service.py

@@ -6,7 +6,6 @@ from pathlib import Path
 from basic_memory.models import Entity, Observation
 from basic_memory.repository.observation_repository import ObservationRepository
 from basic_memory.services.observation_service import ObservationService
-from basic_memory.services import DatabaseSyncError
 
 
 @pytest_asyncio.fixture
@@ -30,6 +29,60 @@ async def test_entity(session):
     return entity
 
 
+@pytest_asyncio.fixture
+async def test_add_observation_success(observation_service, test_entity):
+    """Test successful observation addition."""
+
+    # Act
+    observations = await observation_service.add_observations(test_entity.id, ["New observation"])
+
+    # Assert
+    assert len(observations) == 1
+    assert isinstance(observations[0], Observation)
+    assert observations[0].content == "New observation"
+
+    # Verify database index
+    db_observations = await observation_service.observation_repo.find_by_entity(test_entity.id)
+    assert len(db_observations) == 1
+    assert any(obs.content == "New observation"
+               for obs in db_observations)
+
+
+@pytest_asyncio.fixture
+async def test_search_observations(observation_service, test_entity):
+    """Test searching observations across entities."""
+    # Arrange
+    await observation_service.add_observations(
+        test_entity.id,
+        ["Unique test content", "Other content"]
+    )
+
+    # Act
+    results = await observation_service.search_observations("unique")
+
+    # Assert
+    assert len(results) == 1
+    assert results[0].content == "Unique test content"
+
+
+@pytest_asyncio.fixture
+async def test_observation_with_special_characters(observation_service, test_entity):
+    """Test handling observations with special characters."""
+    content = "Test & observation with @#$% special chars!"
+
+    observations = await observation_service.add_observations(test_entity.id, [content])
+    assert observations[0].content == content
+
+
+@pytest_asyncio.fixture
+async def test_very_long_observation(observation_service, test_entity):
+    """Test handling very long observation content."""
+    long_content = "Very long observation " * 100  # ~1800 characters
+
+    observations = await observation_service.add_observations(test_entity.id, [long_content])
+    assert observations[0].content == long_content
+
+
 @pytest_asyncio.fixture
 async def test_observations(session, test_entity):
     """Create test observations."""
@@ -100,15 +153,6 @@ async def test_delete_observations_invalid_entity(observation_service):
     assert result is False
 
 
-@pytest.mark.asyncio
-async def test_search_observations(observation_service, test_observations):
-    """Test searching observations."""
-    results = await observation_service.search_observations("First")
-    
-    assert len(results) == 1
-    assert results[0].content == "First observation"
-
-
 @pytest.mark.asyncio
 async def test_get_observations_by_context(observation_service, session, test_entity):
     """Test getting observations by context."""

tests/test_observation_service.py

@@ -1,62 +0,0 @@
-"""Tests for ObservationService."""
-import pytest
-
-from basic_memory.models import Observation
-
-pytestmark = pytest.mark.asyncio
-
-
-async def test_add_observation_success(observation_service, test_entity):
-    """Test successful observation addition."""
-
-    # Act
-    observations = await observation_service.add_observations(test_entity.id, ["New observation"])
-    
-    # Assert
-    assert len(observations) == 1
-    assert isinstance(observations[0], Observation)
-    assert observations[0].content == "New observation"
-
-    # Verify database index
-    db_observations = await observation_service.observation_repo.find_by_entity(test_entity.id)
-    assert len(db_observations) == 1
-    assert any(obs.content == "New observation"
-              for obs in db_observations)
-
-
-
-async def test_search_observations(observation_service, test_entity):
-    """Test searching observations across entities."""
-    # Arrange
-    await observation_service.add_observations(
-        test_entity.id,
-        ["Unique test content", "Other content"]
-    )
-
-    # Act
-    results = await observation_service.search_observations("unique")
-    
-    # Assert
-    assert len(results) == 1
-    assert results[0].content == "Unique test content"
-
-
-
-
-# Edge Cases
-
-async def test_observation_with_special_characters(observation_service, test_entity):
-    """Test handling observations with special characters."""
-    content = "Test & observation with @#$% special chars!"
-
-    observations = await observation_service.add_observations(test_entity.id, [content])
-    assert observations[0].content == content
-
-
-async def test_very_long_observation(observation_service, test_entity):
-    """Test handling very long observation content."""
-    long_content = "Very long observation " * 100  # ~1800 characters
-
-    observations = await observation_service.add_observations(test_entity.id, [long_content])
-    assert observations[0].content == long_content
-