From e2a4fa63e5631870ed4268ce0898666dc7890ea4 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jan 2025 22:38:19 -0600 Subject: [PATCH] fix context_service tests --- src/basic_memory/models/search.py | 2 +- .../repository/search_repository.py | 14 +- src/basic_memory/schemas/search.py | 41 +++- src/basic_memory/services/context_service.py | 31 +-- src/basic_memory/services/search_service.py | 31 ++- tests/conftest.py | 2 +- tests/services/test_context_service.py | 110 +++++---- tests/services/test_search_service.py | 214 ++++++------------ 8 files changed, 208 insertions(+), 237 deletions(-) diff --git a/src/basic_memory/models/search.py b/src/basic_memory/models/search.py index fa6c9f70..1dd6bdfa 100644 --- a/src/basic_memory/models/search.py +++ b/src/basic_memory/models/search.py @@ -28,7 +28,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS search_index USING fts5( updated_at UNINDEXED, -- Last update -- Configuration - tokenize=\"unicode61 separators '/'\", -- Treat / as part of tokens + tokenize='unicode61 tokenchars 0x2F', -- Hex code for / prefix='1,2,3,4' -- Support longer prefixes for paths ); """) \ No newline at end of file diff --git a/src/basic_memory/repository/search_repository.py b/src/basic_memory/repository/search_repository.py index ad23ca41..ac8ea59b 100644 --- a/src/basic_memory/repository/search_repository.py +++ b/src/basic_memory/repository/search_repository.py @@ -74,8 +74,10 @@ class SearchRepository: await session.commit() def _quote_search_term(self, term: str) -> str: - """Add quotes if term contains special characters.""" - if any(c in term for c in "/-"): + """Add quotes if term contains special characters or /. + For FTS5, phrases with / need to be quoted to be treated as a single token. + """ + if '/' in term or '*' in term or any(c in term for c in "-"): return f'"{term}"' return term @@ -83,6 +85,7 @@ class SearchRepository: self, search_text: Optional[str] = None, permalink: Optional[str] = None, + permalink_match: Optional[str] = None, types: List[SearchItemType] = None, after_date: datetime = None, entity_types: List[str] = None, @@ -97,9 +100,14 @@ class SearchRepository: params["text"] = f"{search_text}*" conditions.append("(title MATCH :text OR content MATCH :text)") - # Handle permalink search + # Handle permalink exact search if permalink: params["permalink"] = permalink + conditions.append("permalink = :permalink") + + # Handle permalink match search, supports * + if permalink_match: + params["permalink"] = self._quote_search_term(permalink_match) conditions.append("permalink MATCH :permalink") # Handle type filter diff --git a/src/basic_memory/schemas/search.py b/src/basic_memory/schemas/search.py index 52cdc1ea..ebf203cb 100644 --- a/src/basic_memory/schemas/search.py +++ b/src/basic_memory/schemas/search.py @@ -14,6 +14,7 @@ from pydantic import BaseModel, field_validator class SearchItemType(str, Enum): """Types of searchable items.""" + ENTITY = "entity" OBSERVATION = "observation" RELATION = "relation" @@ -21,21 +22,23 @@ class SearchItemType(str, Enum): class SearchQuery(BaseModel): """Search query parameters. - + Use ONE of these primary search modes: - permalink: Exact permalink match - permalink_pattern: Path pattern with * - text: Full-text search of title/content - + Optionally filter results by: - types: Limit to specific item types - entity_types: Limit to specific entity types - after_date: Only items after date """ + # Primary search modes (use ONE of these) permalink: Optional[str] = None # Exact permalink match + permalink_match: Optional[str] = None # Exact permalink match text: Optional[str] = None # Full-text search - + # Optional filters types: Optional[List[SearchItemType]] = None # Filter by item type entity_types: Optional[List[str]] = None # Filter by entity type @@ -51,31 +54,43 @@ class SearchQuery(BaseModel): return v.isoformat() return v + def no_criteria(self) -> bool: + return ( + self.permalink is None + and self.permalink_match is None + and self.text is None + and self.after_date is None + and self.types is None + and self.entity_types is None + ) + class SearchResult(BaseModel): """Search result with score and metadata.""" - id: int + + id: int type: SearchItemType score: Optional[float] = None metadata: Optional[dict] = None - + # Common fields permalink: Optional[str] = None file_path: Optional[str] = None - - # Type-specific fields + + # Type-specific fields entity_id: Optional[int] = None # For observations - category: Optional[str] = None # For observations - from_id: Optional[int] = None # For relations - to_id: Optional[int] = None # For relations + category: Optional[str] = None # For observations + from_id: Optional[int] = None # For relations + to_id: Optional[int] = None # For relations relation_type: Optional[str] = None # For relations + class RelatedResult(BaseModel): type: SearchItemType id: int title: str permalink: str - depth:int + depth: int root_id: int created_at: datetime from_id: Optional[int] = None @@ -88,13 +103,15 @@ class RelatedResult(BaseModel): class SearchResponse(BaseModel): """Wrapper for search results.""" + results: List[SearchResult] # Schema for future advanced search endpoint class AdvancedSearchQuery(BaseModel): """Advanced full-text search with explicit FTS5 syntax.""" + query: str # Raw FTS5 query (e.g., "foo AND bar") types: Optional[List[SearchItemType]] = None entity_types: Optional[List[str]] = None - after_date: Optional[Union[datetime, str]] = None \ No newline at end of file + after_date: Optional[Union[datetime, str]] = None diff --git a/src/basic_memory/services/context_service.py b/src/basic_memory/services/context_service.py index 1a7d453e..b1289b98 100644 --- a/src/basic_memory/services/context_service.py +++ b/src/basic_memory/services/context_service.py @@ -8,7 +8,7 @@ from loguru import logger from sqlalchemy import text from basic_memory.repository.entity_repository import EntityRepository -from basic_memory.repository.search_repository import SearchRepository +from basic_memory.repository.search_repository import SearchRepository, SearchIndexRow from basic_memory.schemas.memory import MemoryUrl from basic_memory.schemas.search import SearchQuery, SearchItemType @@ -64,19 +64,24 @@ class ContextService: # Special mode for finding related content target = memory_url.params["target"] logger.debug(f"Finding related content for '{target}'") + + # start by looking at direct relations primary = await self.find_related_1(target) + + # Pattern matching - use search + elif '*' in memory_url.relative_path(): + logger.debug(f"Pattern search for '{memory_url.relative_path()}'") + primary = await self.search_repository.search(permalink_match=memory_url.relative_path()) + + # Direct lookup for exact path else: - # Direct permalink lookup - logger.debug(f"Direct permalink lookup for '{memory_url.relative_path()}'") - primary = await self.find_by_permalink(memory_url.relative_path()) - - logger.debug(f"Found {len(primary)} primary entities") - for p in primary: - logger.debug(f"Found primary entity: {p}") - + logger.debug(f"Direct lookup for '{memory_url.relative_path()}'") + primary = await self.search_repository.search(permalink=memory_url.relative_path()) + # Get type_id pairs for traversal + type_id_pairs = [(r.type, r.id) for r in primary] if primary else [] - logger.debug(f"type_id_pairs: {type_id_pairs}") + logger.debug(f"primary type_id_pairs: {type_id_pairs}") # Find connected content related = await self.find_connected(type_id_pairs, max_depth=depth, since=since) @@ -99,14 +104,10 @@ class ContextService: }, } - async def find_by_permalink(self, permalink: str): - """Find an entity by exact permalink.""" - return await self.search_repository.search(permalink=permalink) - async def find_related_1(self, permalink: str): """Find entities related to a given permalink.""" # First find the target entity - target = await self.find_by_permalink(permalink) + target = await self.search_repository.search(permalink=permalink) if not target: return [] diff --git a/src/basic_memory/services/search_service.py b/src/basic_memory/services/search_service.py index 4bb3b41c..41371148 100644 --- a/src/basic_memory/services/search_service.py +++ b/src/basic_memory/services/search_service.py @@ -57,22 +57,21 @@ class SearchService: 2. Pattern match: handles * wildcards in paths 3. Text search: full-text search across title/content """ + if query.no_criteria(): + logger.debug("no criteria passed to query") + return [] + logger.debug(f"Searching with query: {query}") - # Determine search mode based on provided parameters - if query.permalink: - # Exact permalink lookup - results = await self.repository.search(permalink=query.permalink) - # elif query.permalink_pattern: - # # Pattern matching with * - # results = await self.repository.search( - # SearchQuery(permalink_pattern=query.permalink_pattern) - # ) - elif query.text: - # Full-text search - results = await self.repository.search(search_text=query.text) - else: - return [] # No search criteria provide + # permalink search + results = await self.repository.search( + search_text=query.text, + permalink=query.permalink, + permalink_match=query.permalink_match, + types=query.types, + entity_types=query.entity_types, + after_date=query.after_date, + ) return results @@ -281,8 +280,8 @@ class SearchService: relation_type=relation_type, entity_id=entity_id, category=category, - created_at= metadata.get("created_at"), - updated_at= metadata.get("updated_at"), + created_at=metadata.get("created_at"), + updated_at=metadata.get("updated_at"), ) ) diff --git a/tests/conftest.py b/tests/conftest.py index 36504d15..931d2f0f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -281,7 +281,7 @@ async def test_graph(entity_repository, search_service): ), Entity( title="Deep Entity", - entity_type="test", + entity_type="deep", permalink="test/deep", file_path="test/deep.md", content_type="text/markdown", diff --git a/tests/services/test_context_service.py b/tests/services/test_context_service.py index f94438b9..e7dd71f7 100644 --- a/tests/services/test_context_service.py +++ b/tests/services/test_context_service.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta, UTC import pytest import pytest_asyncio +from basic_memory.repository.search_repository import SearchIndexRow from basic_memory.schemas.search import SearchItemType from basic_memory.services.context_service import ContextService @@ -16,23 +17,7 @@ async def context_service(search_repository, entity_repository): @pytest.mark.asyncio -async def test_find_by_pattern(context_service, test_graph): - """Test pattern matching.""" - results = await context_service.find_by_permalink("test/*") - assert len(results) > 0 - assert all("test/" in r.permalink for r in results) - - -@pytest.mark.asyncio -async def test_find_by_permalink(context_service, test_graph): - """Test exact permalink lookup.""" - results = await context_service.find_by_permalink("test/root") - assert len(results) == 1 - assert results[0].permalink == "test/root" - - -@pytest.mark.asyncio -async def test_find_related(context_service, test_graph): +async def test_find_related_1(context_service, test_graph): """Test finding related content.""" results = await context_service.find_related_1("test/root") # Should get immediate connections @@ -106,38 +91,49 @@ async def test_find_connected_timeframe(context_service, test_graph, search_repo # Index root and its relation as old await search_repository.index_item( - id=test_graph["root"].id, - title=test_graph["root"].title, - content="Root content", - permalink=test_graph["root"].permalink, - file_path=test_graph["root"].file_path, - type=SearchItemType.ENTITY, - metadata={"created_at": old_date.isoformat()}, + SearchIndexRow( + id=test_graph["root"].id, + title=test_graph["root"].title, + content="Root content", + permalink=test_graph["root"].permalink, + file_path=test_graph["root"].file_path, + type=SearchItemType.ENTITY, + metadata={"created_at": old_date.isoformat()}, + created_at=old_date.isoformat(), + updated_at=old_date.isoformat() + ) ) await search_repository.index_item( - id=test_graph["relations"][0].id, - title="Root Entity → Connected Entity 1", - content="", - permalink=f"{test_graph['root'].permalink}/connects_to/{test_graph['connected1'].permalink}", - file_path=test_graph["root"].file_path, - type=SearchItemType.RELATION, - from_id=test_graph["root"].id, - to_id=test_graph["connected1"].id, - relation_type="connects_to", - metadata={"created_at": old_date.isoformat()}, + SearchIndexRow( + id=test_graph["relations"][0].id, + title="Root Entity → Connected Entity 1", + content="", + permalink=f"{test_graph['root'].permalink}/connects_to/{test_graph['connected1'].permalink}", + file_path=test_graph["root"].file_path, + type=SearchItemType.RELATION, + from_id=test_graph["root"].id, + to_id=test_graph["connected1"].id, + relation_type="connects_to", + metadata={"created_at": old_date.isoformat()}, + created_at=old_date.isoformat(), + updated_at=old_date.isoformat() + ) ) # Index connected1 as recent await search_repository.index_item( - id=test_graph["connected1"].id, - title=test_graph["connected1"].title, - content="Connected 1 content", - permalink=test_graph["connected1"].permalink, - file_path=test_graph["connected1"].file_path, - type=SearchItemType.ENTITY, - metadata={"created_at": recent_date.isoformat()}, + SearchIndexRow( + id=test_graph["connected1"].id, + title=test_graph["connected1"].title, + content="Connected 1 content", + permalink=test_graph["connected1"].permalink, + file_path=test_graph["connected1"].file_path, + type=SearchItemType.ENTITY, + metadata={"created_at": recent_date.isoformat()}, + created_at=recent_date.isoformat(), + updated_at=recent_date.isoformat() + ) ) - type_id_pairs = [("entity", test_graph["root"].id)] # Search with a 7-day cutoff @@ -150,13 +146,33 @@ async def test_find_connected_timeframe(context_service, test_graph, search_repo assert len(entity_ids) == 0 # No accessible entities within timeframe +@pytest.mark.asyncio +async def test_build_context(context_service, test_graph): + """Test exact permalink lookup.""" + url = "memory://not_used/test/root" + results = await context_service.build_context(url) + matched_entities = results["metadata"]["matched_entities"] + primary_entities = results["primary_entities"] + related_entities = results["related_entities"] + total_entities = results["metadata"]["total_entities"] + + assert results["metadata"]["uri"] == url + assert results["metadata"]["depth"] == 2 + assert matched_entities == 1 + assert len(primary_entities) == 1 + assert len(related_entities) == 10 + assert total_entities == len(primary_entities) + len(related_entities) + + @pytest.mark.asyncio async def test_build_context_pattern(context_service, test_graph): - """Test building context from pattern.""" - context = await context_service.build_context("memory://project/test/*") - assert len(context["primary_entities"]) > 0 - assert "uri" in context["metadata"] - assert "total_entities" in context["metadata"] + """Test exact permalink lookup.""" + url = "memory://not_used/test/connected*" + results = await context_service.build_context(url) + matched_entities = results["metadata"]["matched_entities"] + primary_entities = results["primary_entities"] + related_entities = results["related_entities"] + total_entities = results["metadata"]["total_entities"] @pytest.mark.asyncio diff --git a/tests/services/test_search_service.py b/tests/services/test_search_service.py index 2b0d42ce..63948c17 100644 --- a/tests/services/test_search_service.py +++ b/tests/services/test_search_service.py @@ -1,203 +1,133 @@ """Tests for search service.""" + from datetime import datetime import pytest -import pytest_asyncio from sqlalchemy import text from basic_memory import db -from basic_memory.models import Entity from basic_memory.schemas.search import SearchQuery, SearchItemType -@pytest_asyncio.fixture -async def test_entities(entity_repository): - """Create a set of test entities with various naming patterns.""" - entities = [ - Entity( - title="Core Service", - entity_type="component", - permalink="components/core-service", # Updated to use path-style permalinks - summary="The core service implementation", - file_path="components/core-service.md", - content_type="text/markdown", - ), - Entity( - title="Service Config", - entity_type="config", - permalink="config/service-config", - summary="Configuration for services", - file_path="config/service-config.md", - content_type="text/markdown", - ), - Entity( - title="Auth Service", - entity_type="component", - permalink="components/auth/service", # Nested path - summary="Authentication service implementation", - file_path="components/auth/service.md", - content_type="text/markdown", - ), - Entity( - title="Core Features", - entity_type="specs", - permalink="specs/features/core", - summary="Core feature specifications", - file_path="specs/features/core.md", - content_type="text/markdown", - ), - Entity( - title="API Documentation", - entity_type="docs", - permalink="docs/api/documentation", - summary="API documentation and examples", - file_path="docs/api/documentation.md", - content_type="text/markdown", - ), - ] - - return await entity_repository.add_all(entities) - - -@pytest_asyncio.fixture -async def indexed_search(search_service, test_entities): - """Create SearchService instance with indexed test data.""" - # Index all test entities - for entity in test_entities: - await search_service.index_entity(entity) - return search_service - - @pytest.mark.asyncio -async def test_search_permalink(indexed_search): +async def test_search_permalink(search_service, test_graph): """Exact permalink""" - results = await indexed_search.search(SearchQuery(permalink="components/core-service")) + results = await search_service.search(SearchQuery(permalink="test/root")) assert len(results) == 1 - assert results[0].permalink == "components/core-service" + + for r in results: + assert "test/root" in r.permalink + @pytest.mark.asyncio -async def test_search_wildcard(indexed_search): +async def test_search_permalink_wildcard(search_service, test_graph): """Pattern matching""" - results = await indexed_search.search(SearchQuery(permalink="components/*")) + results = await search_service.search(SearchQuery(permalink_match="test/root/observations/*")) assert len(results) == 2 permalinks = {r.permalink for r in results} - assert "components/core-service" in permalinks - assert "components/auth/service" in permalinks + assert "test/root/observations/1" in permalinks + assert "test/root/observations/2" in permalinks + @pytest.mark.asyncio -async def test_search_text(indexed_search): +async def test_search_text(search_service, test_graph): """Full-text search""" - results = await indexed_search.search(SearchQuery(text="implementation")) + results = await search_service.search(SearchQuery(text="Root Entity", types=[SearchItemType.ENTITY])) assert len(results) >= 1 - assert any("service" in r.permalink for r in results) + assert results[0].permalink == "test/root" @pytest.mark.asyncio -async def test_text_search_features(indexed_search): +async def test_text_search_features(search_service, test_graph): """Test text search functionality.""" # Case insensitive - results = await indexed_search.search(SearchQuery(text="API")) - assert len(results) == 1 - assert results[0].file_path == "docs/api/documentation.md" + results = await search_service.search(SearchQuery(text="ENTITY")) + assert any( "test/root" in r.permalink for r in results) # Partial word match - results = await indexed_search.search(SearchQuery(text="Serv")) + results = await search_service.search(SearchQuery(text="Connect")) assert len(results) > 0 - assert any(r.file_path == "components/core-service.md" for r in results) + assert any(r.file_path == "test/connected2.md" for r in results) # Multiple terms - results = await indexed_search.search(SearchQuery(text="core service")) - assert any("core-service" in r.permalink for r in results) + results = await search_service.search(SearchQuery(text="root connected")) + assert any("test/root" in r.permalink for r in results) @pytest.mark.asyncio -async def test_pattern_matching(indexed_search): +async def test_pattern_matching(search_service, test_graph): """Test pattern matching with various wildcards.""" - # Test nested wildcards - results = await indexed_search.search( - SearchQuery(permalink_pattern="components/*/service") - ) - assert len(results) == 1 # Should match components/auth/service - assert results[0].permalink == "components/auth/service" + # Test wildcards + results = await search_service.search(SearchQuery(permalink="test/*")) + for r in results: + assert "test/" in r.permalink # Test start wildcards - results = await indexed_search.search( - SearchQuery(permalink_pattern="*/service") + results = await search_service.search(SearchQuery(permalink="*/observations")) + for r in results: + assert "/observations" in r.permalink + + +@pytest.mark.asyncio +async def test_filters(search_service, test_graph): + """Test search filters.""" + # Combined filters + results = await search_service.search( + SearchQuery(text="Deep", types=[SearchItemType.ENTITY], entity_types=["deep"]) ) assert len(results) == 1 - assert results[0].permalink == "components/auth/service" - - # Test end wildcards - results = await indexed_search.search( - SearchQuery(permalink_pattern="components/*") - ) - assert len(results) == 2 - assert all("components/" in r.permalink for r in results) - - -@pytest.mark.asyncio -async def test_filters(indexed_search): - """Test search filters.""" - # Filter by type - results = await indexed_search.search( - SearchQuery( - text="service", - types=[SearchItemType.ENTITY] - ) - ) for r in results: assert r.type == SearchItemType.ENTITY - - # Filter by entity type - results = await indexed_search.search( - SearchQuery( - text="service", - entity_types=["component"] - ) - ) - assert all(r.metadata.get("entity_type") == "component" for r in results) - - # Combined filters - results = await indexed_search.search( - SearchQuery( - text="service", - types=[SearchItemType.ENTITY], - entity_types=["component"] - ) - ) - assert len(results) == 2 - assert all(r.type == SearchItemType.ENTITY for r in results) - assert all(r.metadata.get("entity_type") == "component" for r in results) + assert r.metadata.get("entity_type") == "deep" @pytest.mark.asyncio -async def test_after_date(indexed_search): +async def test_after_date(search_service, test_graph): """Test search filters.""" - + # Should find with past date past_date = datetime(2020, 1, 1) - results = await indexed_search.search( + results = await search_service.search( SearchQuery( - text="service", + text="entity", after_date=past_date.isoformat(), ) ) - assert all(datetime.fromisoformat(r.metadata['created_at']) > past_date for r in results) + for r in results: + assert datetime.fromisoformat(r.metadata["created_at"]) > past_date # Should not find with future date future_date = datetime(2030, 1, 1) - results = await indexed_search.search( + results = await search_service.search( SearchQuery( - text="service", + text="entity", after_date=future_date.isoformat(), ) ) assert len(results) == 0 + @pytest.mark.asyncio -async def test_no_criteria(indexed_search): +async def test_search_type(search_service, test_graph): + """Test search filters.""" + + # Should find only type + results = await search_service.search(SearchQuery(types=[SearchItemType.ENTITY])) + assert len(results) > 0 + for r in results: + assert r.type == SearchItemType.ENTITY + + # Should find only types passed in + results = await search_service.search(SearchQuery(types=[SearchItemType.ENTITY])) + assert len(results) > 0 + for r in results: + assert r.type == SearchItemType.ENTITY + + +@pytest.mark.asyncio +async def test_no_criteria(search_service, test_graph): """Test search with no criteria returns empty list.""" - results = await indexed_search.search(SearchQuery()) + results = await search_service.search(SearchQuery()) assert len(results) == 0 @@ -212,14 +142,14 @@ async def test_init_search_index(search_service, session_maker): @pytest.mark.asyncio -async def test_update_index(indexed_search, full_entity): +async def test_update_index(search_service, full_entity): """Test updating indexed content.""" - await indexed_search.index_entity(full_entity) + await search_service.index_entity(full_entity) # Update entity full_entity.summary = "Updated description with new terms" - await indexed_search.index_entity(full_entity) + await search_service.index_entity(full_entity) # Search for new terms - results = await indexed_search.search(SearchQuery(text="new terms")) - assert len(results) == 1 \ No newline at end of file + results = await search_service.search(SearchQuery(text="new terms")) + assert len(results) == 1