mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(core): clean up delete vectors and cloud sync (#733)
Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -124,22 +124,6 @@ def sync_project_command(
|
||||
|
||||
if success:
|
||||
console.print(f"[green]{name} synced successfully[/green]")
|
||||
|
||||
# Trigger database sync if not a dry run
|
||||
if not dry_run:
|
||||
|
||||
async def _trigger_db_sync():
|
||||
async with get_client(project_name=name) as client:
|
||||
return await ProjectClient(client).sync(
|
||||
project_data.external_id, force_full=False
|
||||
)
|
||||
|
||||
try:
|
||||
with force_routing(cloud=True):
|
||||
result = run_with_cleanup(_trigger_db_sync())
|
||||
console.print(f"[dim]Database sync initiated: {result.get('message')}[/dim]")
|
||||
except Exception as e:
|
||||
console.print(f"[yellow]Warning: Could not trigger database sync: {e}[/yellow]")
|
||||
else:
|
||||
console.print(f"[red]{name} sync failed[/red]")
|
||||
raise typer.Exit(1)
|
||||
@@ -202,22 +186,6 @@ def bisync_project_command(
|
||||
sync_entry.last_sync = datetime.now()
|
||||
sync_entry.bisync_initialized = True
|
||||
ConfigManager().save_config(config)
|
||||
|
||||
# Trigger database sync if not a dry run
|
||||
if not dry_run:
|
||||
|
||||
async def _trigger_db_sync():
|
||||
async with get_client(project_name=name) as client:
|
||||
return await ProjectClient(client).sync(
|
||||
project_data.external_id, force_full=False
|
||||
)
|
||||
|
||||
try:
|
||||
with force_routing(cloud=True):
|
||||
result = run_with_cleanup(_trigger_db_sync())
|
||||
console.print(f"[dim]Database sync initiated: {result.get('message')}[/dim]")
|
||||
except Exception as e:
|
||||
console.print(f"[yellow]Warning: Could not trigger database sync: {e}[/yellow]")
|
||||
else:
|
||||
console.print(f"[red]{name} bisync failed[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
@@ -70,6 +70,10 @@ class SearchRepository(Protocol):
|
||||
"""Sync semantic vector chunks for an entity."""
|
||||
...
|
||||
|
||||
async def delete_entity_vector_rows(self, entity_id: int) -> None:
|
||||
"""Delete semantic vector chunks and embeddings for one entity."""
|
||||
...
|
||||
|
||||
async def sync_entity_vectors_batch(
|
||||
self,
|
||||
entity_ids: list[int],
|
||||
|
||||
@@ -454,6 +454,15 @@ class SearchRepositoryBase(ABC):
|
||||
logger.debug(f"Query executed successfully in {elapsed_time:.2f}s.")
|
||||
return result
|
||||
|
||||
async def delete_entity_vector_rows(self, entity_id: int) -> None:
|
||||
"""Delete one entity's derived vector rows using the backend's cleanup path."""
|
||||
await self._ensure_vector_tables()
|
||||
|
||||
async with db.scoped_session(self.session_maker) as session:
|
||||
await self._prepare_vector_session(session)
|
||||
await self._delete_entity_chunks(session, entity_id)
|
||||
await session.commit()
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Shared semantic search: guard, text processing, chunking
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
@@ -565,21 +565,6 @@ class SQLiteSearchRepository(SearchRepositoryBase):
|
||||
stale_params,
|
||||
)
|
||||
|
||||
async def delete_entity_vector_rows(self, entity_id: int) -> None:
|
||||
"""Delete one entity's vec rows on a sqlite-vec-enabled connection."""
|
||||
await self._ensure_vector_tables()
|
||||
|
||||
async with db.scoped_session(self.session_maker) as session:
|
||||
await self._ensure_sqlite_vec_loaded(session)
|
||||
|
||||
# Constraint: sqlite-vec virtual tables are only visible after vec0 is
|
||||
# loaded on this exact connection.
|
||||
# Why: generic repository sessions can reach search_vector_chunks but still
|
||||
# fail with "no such module: vec0" when touching embeddings.
|
||||
# Outcome: service-level cleanup routes vec-table deletes through this helper.
|
||||
await self._delete_entity_chunks(session, entity_id)
|
||||
await session.commit()
|
||||
|
||||
async def delete_project_vector_rows(self) -> None:
|
||||
"""Delete all vector rows for this project on a sqlite-vec-enabled connection."""
|
||||
await self._ensure_vector_tables()
|
||||
|
||||
@@ -660,7 +660,6 @@ class SearchService:
|
||||
async def _clear_entity_vectors(self, entity_id: int) -> None:
|
||||
"""Delete derived vector rows for one entity."""
|
||||
from basic_memory.repository.search_repository_base import SearchRepositoryBase
|
||||
from basic_memory.repository.sqlite_search_repository import SQLiteSearchRepository
|
||||
|
||||
# Trigger: semantic indexing is disabled for this repository instance.
|
||||
# Why: repositories only create vector tables when semantic search is enabled.
|
||||
@@ -671,17 +670,7 @@ class SearchService:
|
||||
):
|
||||
return
|
||||
|
||||
params = {"project_id": self.repository.project_id, "entity_id": entity_id}
|
||||
if isinstance(self.repository, SQLiteSearchRepository):
|
||||
await self.repository.delete_entity_vector_rows(entity_id)
|
||||
else:
|
||||
await self.repository.execute_query(
|
||||
text(
|
||||
"DELETE FROM search_vector_chunks "
|
||||
"WHERE project_id = :project_id AND entity_id = :entity_id"
|
||||
),
|
||||
params,
|
||||
)
|
||||
await self.repository.delete_entity_vector_rows(entity_id)
|
||||
|
||||
async def index_entity_file(
|
||||
self,
|
||||
@@ -889,7 +878,7 @@ class SearchService:
|
||||
await self.repository.delete_by_entity_id(entity_id)
|
||||
|
||||
async def handle_delete(self, entity: Entity):
|
||||
"""Handle complete entity deletion from search index including observations and relations.
|
||||
"""Handle complete entity deletion from search and semantic index state.
|
||||
|
||||
This replicates the logic from sync_service.handle_delete() to properly clean up
|
||||
all search index entries for an entity and its related data.
|
||||
@@ -916,3 +905,8 @@ class SearchService:
|
||||
await self.delete_by_permalink(permalink)
|
||||
else:
|
||||
await self.delete_by_entity_id(entity.id)
|
||||
|
||||
# Trigger: entity deletion removes the source rows for this note.
|
||||
# Why: semantic chunks/embeddings are stored separately from search_index rows.
|
||||
# Outcome: deleting an entity clears both full-text and vector-derived search state.
|
||||
await self._clear_entity_vectors(entity.id)
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Tests for cloud sync and bisync command behavior."""
|
||||
|
||||
import importlib
|
||||
from contextlib import asynccontextmanager
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
@@ -20,11 +19,10 @@ runner = CliRunner()
|
||||
["cloud", "bisync", "--name", "research"],
|
||||
],
|
||||
)
|
||||
def test_cloud_sync_commands_use_incremental_db_sync(monkeypatch, argv, config_manager):
|
||||
"""Cloud sync commands should not force a full database re-index after file sync."""
|
||||
def test_cloud_sync_commands_skip_explicit_cloud_project_sync(monkeypatch, argv, config_manager):
|
||||
"""Cloud sync commands should not trigger an extra explicit cloud project sync."""
|
||||
project_sync_command = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
|
||||
seen: dict[str, object] = {}
|
||||
config = config_manager.load_config()
|
||||
config.set_project_mode("research", ProjectMode.CLOUD)
|
||||
config_manager.save_config(config)
|
||||
@@ -50,30 +48,10 @@ def test_cloud_sync_commands_use_incremental_db_sync(monkeypatch, argv, config_m
|
||||
monkeypatch.setattr(project_sync_command, "project_sync", lambda *args, **kwargs: True)
|
||||
monkeypatch.setattr(project_sync_command, "project_bisync", lambda *args, **kwargs: True)
|
||||
|
||||
@asynccontextmanager
|
||||
async def fake_get_client(*, project_name=None, workspace=None):
|
||||
seen["project_name"] = project_name
|
||||
seen["workspace"] = workspace
|
||||
yield object()
|
||||
|
||||
class FakeProjectClient:
|
||||
def __init__(self, _client):
|
||||
pass
|
||||
|
||||
async def sync(self, external_id: str, force_full: bool = False):
|
||||
seen["external_id"] = external_id
|
||||
seen["force_full"] = force_full
|
||||
return {"message": "queued"}
|
||||
|
||||
monkeypatch.setattr(project_sync_command, "get_client", fake_get_client)
|
||||
monkeypatch.setattr(project_sync_command, "ProjectClient", FakeProjectClient)
|
||||
|
||||
result = runner.invoke(app, argv)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert seen["project_name"] == "research"
|
||||
assert seen["external_id"] == "external-project-id"
|
||||
assert seen["force_full"] is False
|
||||
assert "Database sync initiated" not in result.output
|
||||
|
||||
|
||||
def test_cloud_bisync_fails_fast_when_sync_entry_disappears(monkeypatch, config_manager):
|
||||
|
||||
@@ -6,8 +6,10 @@ from textwrap import dedent
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from sqlalchemy import text
|
||||
|
||||
from basic_memory.config import ProjectConfig, BasicMemoryConfig
|
||||
from basic_memory import db
|
||||
from basic_memory.config import ProjectConfig, BasicMemoryConfig, DatabaseBackend
|
||||
from basic_memory.markdown import EntityParser
|
||||
from basic_memory.models import Entity as EntityModel
|
||||
from basic_memory.repository import EntityRepository
|
||||
@@ -19,6 +21,98 @@ from basic_memory.services.search_service import SearchService
|
||||
from basic_memory.utils import generate_permalink
|
||||
|
||||
|
||||
class _DeleteTestEmbeddingProvider:
|
||||
"""Deterministic embedding provider for entity delete cleanup tests."""
|
||||
|
||||
model_name = "delete-test"
|
||||
dimensions = 4
|
||||
|
||||
async def embed_query(self, text: str) -> list[float]:
|
||||
return self._vectorize(text)
|
||||
|
||||
async def embed_documents(self, texts: list[str]) -> list[list[float]]:
|
||||
return [self._vectorize(text) for text in texts]
|
||||
|
||||
@staticmethod
|
||||
def _vectorize(text: str) -> list[float]:
|
||||
normalized = text.lower()
|
||||
if "semantic" in normalized:
|
||||
return [1.0, 0.0, 0.0, 0.0]
|
||||
if "cleanup" in normalized:
|
||||
return [0.0, 1.0, 0.0, 0.0]
|
||||
return [0.0, 0.0, 1.0, 0.0]
|
||||
|
||||
|
||||
async def _count_entity_search_state(
|
||||
session_maker,
|
||||
app_config: BasicMemoryConfig,
|
||||
project_id: int,
|
||||
entity_id: int,
|
||||
) -> tuple[int, int, int]:
|
||||
"""Return counts for all derived search rows tied to one entity."""
|
||||
embedding_join = (
|
||||
"e.chunk_id = c.id"
|
||||
if app_config.database_backend == DatabaseBackend.POSTGRES
|
||||
else "e.rowid = c.id"
|
||||
)
|
||||
params = {"project_id": project_id, "entity_id": entity_id}
|
||||
|
||||
async with db.scoped_session(session_maker) as session:
|
||||
search_index_rows = await session.execute(
|
||||
text(
|
||||
"SELECT COUNT(*) FROM search_index "
|
||||
"WHERE project_id = :project_id AND entity_id = :entity_id"
|
||||
),
|
||||
params,
|
||||
)
|
||||
vector_chunk_rows = await session.execute(
|
||||
text(
|
||||
"SELECT COUNT(*) FROM search_vector_chunks "
|
||||
"WHERE project_id = :project_id AND entity_id = :entity_id"
|
||||
),
|
||||
params,
|
||||
)
|
||||
vector_embedding_rows = await session.execute(
|
||||
text(
|
||||
"SELECT COUNT(*) FROM search_vector_embeddings e "
|
||||
"JOIN search_vector_chunks c ON "
|
||||
f"{embedding_join} "
|
||||
"WHERE c.project_id = :project_id AND c.entity_id = :entity_id"
|
||||
),
|
||||
params,
|
||||
)
|
||||
|
||||
return (
|
||||
int(search_index_rows.scalar_one()),
|
||||
int(vector_chunk_rows.scalar_one()),
|
||||
int(vector_embedding_rows.scalar_one()),
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def entity_service_with_search(
|
||||
entity_repository: EntityRepository,
|
||||
observation_repository,
|
||||
relation_repository,
|
||||
entity_parser: EntityParser,
|
||||
file_service: FileService,
|
||||
link_resolver,
|
||||
search_service: SearchService,
|
||||
app_config: BasicMemoryConfig,
|
||||
) -> EntityService:
|
||||
"""Create EntityService with a real attached search service."""
|
||||
return EntityService(
|
||||
entity_parser=entity_parser,
|
||||
entity_repository=entity_repository,
|
||||
observation_repository=observation_repository,
|
||||
relation_repository=relation_repository,
|
||||
file_service=file_service,
|
||||
link_resolver=link_resolver,
|
||||
search_service=search_service,
|
||||
app_config=app_config,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_entity(
|
||||
entity_service: EntityService, file_service: FileService, project_config: ProjectConfig
|
||||
@@ -227,6 +321,61 @@ async def test_delete_entity_by_id(entity_service: EntityService):
|
||||
await entity_service.get_by_permalink(entity_data.permalink)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_entity_removes_search_and_vector_state(
|
||||
entity_service_with_search: EntityService,
|
||||
search_service: SearchService,
|
||||
session_maker,
|
||||
app_config: BasicMemoryConfig,
|
||||
):
|
||||
"""Deleting an entity should clear all of its full-text and semantic search state."""
|
||||
if app_config.database_backend == DatabaseBackend.SQLITE:
|
||||
pytest.importorskip("sqlite_vec")
|
||||
|
||||
repository = search_service.repository
|
||||
repository._semantic_enabled = True
|
||||
repository._embedding_provider = _DeleteTestEmbeddingProvider()
|
||||
repository._vector_dimensions = repository._embedding_provider.dimensions
|
||||
repository._vector_tables_initialized = False
|
||||
await search_service.init_search_index()
|
||||
|
||||
entity = await entity_service_with_search.create_entity(
|
||||
EntitySchema(
|
||||
title="Semantic Delete Target",
|
||||
directory="test",
|
||||
note_type="note",
|
||||
content=dedent("""
|
||||
# Semantic Delete Target
|
||||
|
||||
- [note] Semantic cleanup should remove every derived row
|
||||
- references [[Cleanup Target]]
|
||||
""").strip(),
|
||||
)
|
||||
)
|
||||
|
||||
await search_service.index_entity(entity)
|
||||
await search_service.sync_entity_vectors(entity.id)
|
||||
|
||||
search_rows, chunk_rows, embedding_rows = await _count_entity_search_state(
|
||||
session_maker,
|
||||
app_config,
|
||||
search_service.repository.project_id,
|
||||
entity.id,
|
||||
)
|
||||
assert search_rows >= 3
|
||||
assert chunk_rows > 0
|
||||
assert embedding_rows > 0
|
||||
|
||||
assert await entity_service_with_search.delete_entity(entity.id) is True
|
||||
|
||||
assert await _count_entity_search_state(
|
||||
session_maker,
|
||||
app_config,
|
||||
search_service.repository.project_id,
|
||||
entity.id,
|
||||
) == (0, 0, 0)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_entity_by_permalink_not_found(entity_service: EntityService):
|
||||
"""Test handling of non-existent entity retrieval."""
|
||||
|
||||
@@ -196,4 +196,3 @@ async def test_initialize_app_no_precedence_warning_when_not_conflicting(
|
||||
"ensure_frontmatter_on_sync=True overrides disable_permalinks=True" in message
|
||||
for message in warnings
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user