mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
delete relations/observations from search_index on delete entity during sync
This commit is contained in:
@@ -1,10 +1,8 @@
|
||||
"""Knowledge graph models."""
|
||||
|
||||
import re
|
||||
import os
|
||||
from datetime import datetime
|
||||
from typing import Optional
|
||||
from unidecode import unidecode
|
||||
|
||||
from sqlalchemy import (
|
||||
Integer,
|
||||
@@ -22,7 +20,7 @@ from sqlalchemy.orm import Mapped, mapped_column, relationship, validates
|
||||
from basic_memory.models.base import Base
|
||||
from enum import Enum
|
||||
|
||||
|
||||
from basic_memory.utils import generate_permalink
|
||||
|
||||
|
||||
class Entity(Base):
|
||||
@@ -89,10 +87,10 @@ class Entity(Base):
|
||||
def relations(self):
|
||||
return self.incoming_relations + self.outgoing_relations
|
||||
|
||||
@validates('permalink')
|
||||
@validates("permalink")
|
||||
def validate_permalink(self, key, value):
|
||||
"""Validate permalink format.
|
||||
|
||||
|
||||
Requirements:
|
||||
1. Must be valid URI path component
|
||||
2. Only lowercase letters, numbers, and hyphens (no underscores)
|
||||
@@ -101,8 +99,8 @@ class Entity(Base):
|
||||
"""
|
||||
if not value:
|
||||
raise ValueError("Permalink must not be None")
|
||||
|
||||
if not re.match(r'^[a-z0-9][a-z0-9\-/]*[a-z0-9]$', value):
|
||||
|
||||
if not re.match(r"^[a-z0-9][a-z0-9\-/]*[a-z0-9]$", value):
|
||||
raise ValueError(
|
||||
f"Invalid permalink format: {value}. "
|
||||
"Use only lowercase letters, numbers, and hyphens."
|
||||
@@ -159,6 +157,17 @@ class Observation(Base):
|
||||
# Relationships
|
||||
entity = relationship("Entity", back_populates="observations")
|
||||
|
||||
@property
|
||||
def permalink(self) -> str:
|
||||
"""
|
||||
Create synthetic permalink for the observation
|
||||
We can construct these because observations are always
|
||||
defined in and owned by a single entity
|
||||
"""
|
||||
return generate_permalink(
|
||||
f"{self.entity.permalink}/observations/{self.category}/{self.content}"
|
||||
)
|
||||
|
||||
def __repr__(self) -> str:
|
||||
return f"Observation(id={self.id}, entity_id={self.entity_id}, content='{self.content}')"
|
||||
|
||||
@@ -194,5 +203,16 @@ class Relation(Base):
|
||||
)
|
||||
to_entity = relationship("Entity", foreign_keys=[to_id], back_populates="incoming_relations")
|
||||
|
||||
@property
|
||||
def permalink(self) -> str:
|
||||
"""Create relation permalink showing the semantic connection:
|
||||
source/relation_type/target
|
||||
e.g., "specs/search/implements/features/search-ui"
|
||||
"""
|
||||
|
||||
return generate_permalink(
|
||||
f"{self.from_entity.permalink}/{self.relation_type}/{self.to_entity.permalink}"
|
||||
)
|
||||
|
||||
def __repr__(self) -> str:
|
||||
return f"Relation(id={self.id}, from_id={self.from_id}, to_id={self.to_id}, type='{self.relation_type}')"
|
||||
return f"Relation(id={self.id}, from_id={self.from_id}, to_id={self.to_id}, type='{self.relation_type}')"
|
||||
|
||||
@@ -116,7 +116,7 @@ class EntityRepository(Repository[Entity]):
|
||||
|
||||
def get_load_options(self) -> List[LoaderOption]:
|
||||
return [
|
||||
selectinload(Entity.observations),
|
||||
selectinload(Entity.observations).selectinload(Observation.entity),
|
||||
# Load from_relations and both entities for each relation
|
||||
selectinload(Entity.outgoing_relations).selectinload(Relation.from_entity),
|
||||
selectinload(Entity.outgoing_relations).selectinload(Relation.to_entity),
|
||||
|
||||
@@ -152,12 +152,6 @@ class SearchService:
|
||||
|
||||
# Index each observation with synthetic permalink
|
||||
for obs in entity.observations:
|
||||
# Create synthetic permalink for the observation
|
||||
# We can construct these because observations are always
|
||||
# defined in and owned by a single entity
|
||||
observation_permalink = (
|
||||
generate_permalink(f"{entity.permalink}/observations/{obs.category}/{obs.content}")
|
||||
)
|
||||
|
||||
# Index with parent entity's file path since that's where it's defined
|
||||
await self._do_index(
|
||||
@@ -166,7 +160,7 @@ class SearchService:
|
||||
type=SearchItemType.OBSERVATION.value,
|
||||
title=f"{obs.category}: {obs.content[:50]}...",
|
||||
content=obs.content,
|
||||
permalink=observation_permalink,
|
||||
permalink=obs.permalink,
|
||||
file_path=entity.file_path,
|
||||
category=obs.category,
|
||||
entity_id=entity.id,
|
||||
@@ -182,13 +176,6 @@ class SearchService:
|
||||
|
||||
# Only index outgoing relations (ones defined in this file)
|
||||
for rel in entity.outgoing_relations:
|
||||
# Create relation permalink showing the semantic connection:
|
||||
# source/relation_type/target
|
||||
# e.g., "specs/search/implements/features/search-ui"
|
||||
relation_permalink = (
|
||||
generate_permalink(f"{rel.from_entity.permalink}/{rel.relation_type}/{rel.to_entity.permalink}")
|
||||
)
|
||||
|
||||
# Create descriptive title showing the relationship
|
||||
relation_title = f"{rel.from_entity.title} → {rel.to_entity.title}"
|
||||
|
||||
@@ -197,7 +184,7 @@ class SearchService:
|
||||
id=rel.id,
|
||||
title=relation_title,
|
||||
content=rel.context or "",
|
||||
permalink=relation_permalink,
|
||||
permalink=rel.permalink,
|
||||
file_path=entity.file_path,
|
||||
type=SearchItemType.RELATION.value,
|
||||
from_id=rel.from_id,
|
||||
|
||||
@@ -35,6 +35,29 @@ class SyncService:
|
||||
self.entity_repository = entity_repository
|
||||
self.search_service = search_service
|
||||
|
||||
async def handle_entity_deletion(self, file_path: str):
|
||||
"""Handle complete entity deletion including search index cleanup."""
|
||||
# First get entity to get permalink before deletion
|
||||
entity = await self.entity_repository.get_by_file_path(file_path)
|
||||
if entity:
|
||||
logger.debug(f"Deleting entity and cleaning up search index: {file_path}")
|
||||
|
||||
# Delete from db (this cascades to observations/relations)
|
||||
await self.entity_sync_service.delete_entity_by_file_path(file_path)
|
||||
|
||||
# Clean up search index
|
||||
permalinks = (
|
||||
[entity.permalink]
|
||||
+ [o.permalink for o in entity.observations]
|
||||
+ [r.permalink for r in entity.relations]
|
||||
)
|
||||
logger.debug(f"Deleting from search index: {permalinks}")
|
||||
for permalink in permalinks:
|
||||
await self.search_service.delete_by_permalink(permalink)
|
||||
|
||||
else:
|
||||
logger.debug(f"No entity found to delete: {file_path}")
|
||||
|
||||
async def sync(self, directory: Path) -> SyncReport:
|
||||
"""Sync knowledge files with database."""
|
||||
changes = await self.scanner.find_knowledge_changes(directory)
|
||||
@@ -47,15 +70,13 @@ class SyncService:
|
||||
if entity:
|
||||
# Update file_path but keep the same permalink for link stability
|
||||
await self.entity_repository.update(
|
||||
entity.id,
|
||||
{"file_path": new_path, "checksum": changes.checksums[new_path]}
|
||||
entity.id, {"file_path": new_path, "checksum": changes.checksums[new_path]}
|
||||
)
|
||||
|
||||
|
||||
# Handle deletions next
|
||||
# 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.entity_sync_service.delete_entity_by_file_path(file_path)
|
||||
await self.handle_entity_deletion(file_path)
|
||||
|
||||
# Parse files that need updating
|
||||
parsed_entities: Dict[str, EntityMarkdown] = {}
|
||||
@@ -83,11 +104,13 @@ class SyncService:
|
||||
# Second pass
|
||||
for file_path, entity_markdown in parsed_entities.items():
|
||||
logger.debug(f"Updating relations for: {file_path}")
|
||||
|
||||
|
||||
# Process relations
|
||||
checksum = changes.checksums[file_path]
|
||||
entity = await self.entity_sync_service.update_entity_relations(file_path, entity_markdown)
|
||||
|
||||
entity = await self.entity_sync_service.update_entity_relations(
|
||||
file_path, entity_markdown
|
||||
)
|
||||
|
||||
# add to search index
|
||||
await self.search_service.index_entity(entity)
|
||||
|
||||
|
||||
@@ -7,7 +7,10 @@ import pytest
|
||||
|
||||
from basic_memory.config import ProjectConfig
|
||||
from basic_memory.models import Entity
|
||||
from basic_memory.services import EntityService
|
||||
from basic_memory.repository import EntityRepository
|
||||
from basic_memory.schemas.search import SearchQuery
|
||||
from basic_memory.services import EntityService, ObservationService
|
||||
from basic_memory.services.search_service import SearchService
|
||||
from basic_memory.sync.sync_service import SyncService
|
||||
|
||||
|
||||
@@ -420,21 +423,24 @@ modified: 2024-01-01
|
||||
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_permalink_formatting(sync_service: SyncService, test_config: ProjectConfig, entity_service: EntityService):
|
||||
async def test_permalink_formatting(
|
||||
sync_service: SyncService, test_config: ProjectConfig, entity_service: EntityService
|
||||
):
|
||||
"""Test that permalinks are properly formatted during sync."""
|
||||
|
||||
|
||||
# Test cases with different filename formats
|
||||
test_files = {
|
||||
# filename -> expected permalink
|
||||
"my_awesome_feature.md": "my-awesome-feature",
|
||||
"MIXED_CASE_NAME.md": "mixed-case-name",
|
||||
"spaces and_underscores.md": "spaces-and-underscores",
|
||||
"design/model_refactor.md": "design/model-refactor",
|
||||
"test/multiple_word_directory/feature_name.md": "test/multiple-word-directory/feature-name",
|
||||
# filename -> expected permalink
|
||||
"my_awesome_feature.md": "my-awesome-feature",
|
||||
"MIXED_CASE_NAME.md": "mixed-case-name",
|
||||
"spaces and_underscores.md": "spaces-and-underscores",
|
||||
"design/model_refactor.md": "design/model-refactor",
|
||||
"test/multiple_word_directory/feature_name.md": "test/multiple-word-directory/feature-name",
|
||||
}
|
||||
|
||||
|
||||
# Create test files
|
||||
for filename, _ in test_files.items():
|
||||
content: str = """
|
||||
@@ -448,16 +454,48 @@ modified: 2024-01-01
|
||||
Testing permalink generation.
|
||||
"""
|
||||
await create_test_file(test_config.home / filename, content)
|
||||
|
||||
|
||||
# Run sync
|
||||
await sync_service.sync(test_config.home)
|
||||
|
||||
|
||||
# Verify permalinks
|
||||
entities = await entity_service.repository.find_all()
|
||||
for filename, expected_permalink in test_files.items():
|
||||
# Find entity for this file
|
||||
entity = next(e for e in entities if e.file_path == filename)
|
||||
assert entity.permalink == expected_permalink, f"File {filename} should have permalink {expected_permalink}"
|
||||
assert (
|
||||
entity.permalink == expected_permalink
|
||||
), f"File {filename} should have permalink {expected_permalink}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handle_entity_deletion(
|
||||
test_graph,
|
||||
sync_service: SyncService,
|
||||
test_config: ProjectConfig,
|
||||
entity_repository: EntityRepository,
|
||||
observation_service: ObservationService,
|
||||
search_service: SearchService,
|
||||
):
|
||||
"""Test deletion of entity cleans up search index."""
|
||||
|
||||
root_entity = test_graph["root"]
|
||||
# Delete the entity
|
||||
await sync_service.handle_entity_deletion(root_entity.file_path)
|
||||
|
||||
# Verify entity is gone from db
|
||||
assert await entity_repository.get_by_permalink(root_entity.permalink) is None
|
||||
|
||||
# Verify entity is gone from search index
|
||||
entity_results = await search_service.search(SearchQuery(text=root_entity.title))
|
||||
assert len(entity_results) == 0
|
||||
|
||||
obs_results = await search_service.search(SearchQuery(text="Root note 1"))
|
||||
assert len(obs_results) == 0
|
||||
|
||||
rel_results = await search_service.search(SearchQuery(text="connects_to"))
|
||||
assert len(rel_results) == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_null_checksum_cleanup(
|
||||
|
||||
Reference in New Issue
Block a user