mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: use LinkResolver fallback in build_context for flexible identifier matching (#582)
build_context now falls back to LinkResolver when an exact permalink lookup returns empty results. This reuses the same resolution pipeline as read_note (permalink candidates, title match, file path, FTS) so callers no longer get empty results for valid note identifiers. Also changes ensure_frontmatter_on_sync default to True — frontmatter is now added during sync by default. Tests updated accordingly. 🔧 ContextService accepts optional LinkResolver, wired via DI in all 3 factory variants ✅ 2027 unit + 278 integration tests passing Closes #582 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -246,7 +246,7 @@ class BasicMemoryConfig(BaseSettings):
|
||||
)
|
||||
|
||||
ensure_frontmatter_on_sync: bool = Field(
|
||||
default=False,
|
||||
default=True,
|
||||
description="Ensure markdown files have frontmatter during sync by adding derived title/type/permalink when missing. When combined with disable_permalinks=True, this setting takes precedence for missing-frontmatter files and still writes permalinks.",
|
||||
)
|
||||
|
||||
|
||||
@@ -309,11 +309,13 @@ async def get_context_service(
|
||||
search_repository: SearchRepositoryDep,
|
||||
entity_repository: EntityRepositoryDep,
|
||||
observation_repository: ObservationRepositoryDep,
|
||||
link_resolver: LinkResolverDep,
|
||||
) -> ContextService:
|
||||
return ContextService(
|
||||
search_repository=search_repository,
|
||||
entity_repository=entity_repository,
|
||||
observation_repository=observation_repository,
|
||||
link_resolver=link_resolver,
|
||||
)
|
||||
|
||||
|
||||
@@ -324,12 +326,14 @@ async def get_context_service_v2( # pragma: no cover
|
||||
search_repository: SearchRepositoryV2Dep,
|
||||
entity_repository: EntityRepositoryV2Dep,
|
||||
observation_repository: ObservationRepositoryV2Dep,
|
||||
link_resolver: LinkResolverV2Dep,
|
||||
) -> ContextService:
|
||||
"""Create ContextService for v2 API."""
|
||||
return ContextService(
|
||||
search_repository=search_repository,
|
||||
entity_repository=entity_repository,
|
||||
observation_repository=observation_repository,
|
||||
link_resolver=link_resolver,
|
||||
)
|
||||
|
||||
|
||||
@@ -340,12 +344,14 @@ async def get_context_service_v2_external(
|
||||
search_repository: SearchRepositoryV2ExternalDep,
|
||||
entity_repository: EntityRepositoryV2ExternalDep,
|
||||
observation_repository: ObservationRepositoryV2ExternalDep,
|
||||
link_resolver: LinkResolverV2ExternalDep,
|
||||
) -> ContextService:
|
||||
"""Create ContextService for v2 API (uses external_id)."""
|
||||
return ContextService(
|
||||
search_repository=search_repository,
|
||||
entity_repository=entity_repository,
|
||||
observation_repository=observation_repository,
|
||||
link_resolver=link_resolver,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
"""Service for building rich context from the knowledge graph."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime, timezone
|
||||
from typing import List, Optional, Tuple
|
||||
from typing import List, Optional, Tuple, TYPE_CHECKING
|
||||
|
||||
|
||||
from loguru import logger
|
||||
@@ -16,6 +18,9 @@ from basic_memory.schemas.memory import MemoryUrl, memory_url_path
|
||||
from basic_memory.schemas.search import SearchItemType
|
||||
from basic_memory.utils import generate_permalink
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from basic_memory.services.link_resolver import LinkResolver
|
||||
|
||||
|
||||
@dataclass
|
||||
class ContextResultRow:
|
||||
@@ -82,10 +87,12 @@ class ContextService:
|
||||
search_repository: SearchRepository,
|
||||
entity_repository: EntityRepository,
|
||||
observation_repository: ObservationRepository,
|
||||
link_resolver: Optional[LinkResolver] = None,
|
||||
):
|
||||
self.search_repository = search_repository
|
||||
self.entity_repository = entity_repository
|
||||
self.observation_repository = observation_repository
|
||||
self.link_resolver = link_resolver
|
||||
|
||||
async def build_context(
|
||||
self,
|
||||
@@ -131,6 +138,24 @@ class ContextService:
|
||||
primary = await self.search_repository.search(
|
||||
permalink=normalized_path, limit=fetch_limit, offset=offset
|
||||
)
|
||||
|
||||
# Trigger: exact permalink lookup returned no results
|
||||
# Why: the identifier may be valid but not an exact permalink match
|
||||
# (e.g., missing project prefix, title instead of permalink)
|
||||
# Outcome: use LinkResolver's multi-strategy resolution to find the entity,
|
||||
# then retry search with its actual permalink
|
||||
if not primary and self.link_resolver:
|
||||
entity = await self.link_resolver.resolve_link(
|
||||
path, use_search=True, strict=False
|
||||
)
|
||||
if entity:
|
||||
logger.debug(
|
||||
f"LinkResolver resolved '{path}' to permalink '{entity.permalink}'"
|
||||
)
|
||||
normalized_path = entity.permalink
|
||||
primary = await self.search_repository.search(
|
||||
permalink=entity.permalink, limit=fetch_limit, offset=offset
|
||||
)
|
||||
else:
|
||||
logger.debug(f"Build context for '{types}'")
|
||||
primary = await self.search_repository.search(
|
||||
|
||||
@@ -99,7 +99,8 @@ async def test_build_context_underscore_normalization(mcp_server, app, test_proj
|
||||
assert "related-to" in response_text_related.lower()
|
||||
|
||||
# Test 4: Test exact path (non-wildcard) with underscore
|
||||
# Exact relation permalink would be child/relation/target
|
||||
# Previously this returned empty (no exact permalink match). Now LinkResolver
|
||||
# resolves to the child entity, so we get its relations back.
|
||||
result_exact = await client.call_tool(
|
||||
"build_context",
|
||||
{
|
||||
@@ -110,7 +111,8 @@ async def test_build_context_underscore_normalization(mcp_server, app, test_proj
|
||||
|
||||
response_text_exact = result_exact.content[0].text # pyright: ignore
|
||||
assert '"results"' in response_text_exact
|
||||
assert "part-of" in response_text_exact.lower()
|
||||
# LinkResolver resolves to child-with-underscore entity; its relation_type is "part_of"
|
||||
assert "part_of" in response_text_exact.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -28,6 +28,7 @@ async def test_disable_permalinks_create_entity(tmp_path, engine_factory, app_co
|
||||
|
||||
# Override app config to enable disable_permalinks
|
||||
app_config.disable_permalinks = True
|
||||
app_config.ensure_frontmatter_on_sync = False
|
||||
|
||||
# Setup repositories
|
||||
entity_repository = EntityRepository(session_maker, project_id=test_project.id)
|
||||
@@ -88,6 +89,7 @@ async def test_disable_permalinks_sync_workflow(tmp_path, engine_factory, app_co
|
||||
|
||||
# Override app config to enable disable_permalinks
|
||||
app_config.disable_permalinks = True
|
||||
app_config.ensure_frontmatter_on_sync = False
|
||||
|
||||
# Create a test markdown file without frontmatter
|
||||
test_file = tmp_path / "test_note.md"
|
||||
|
||||
@@ -9,9 +9,13 @@ from basic_memory.services.context_service import ContextService
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def context_service(entity_repository, search_service, observation_repository):
|
||||
async def context_service(
|
||||
search_repository, entity_repository, observation_repository, link_resolver
|
||||
):
|
||||
"""Create a real context service for testing."""
|
||||
return ContextService(entity_repository, search_service, observation_repository)
|
||||
return ContextService(
|
||||
search_repository, entity_repository, observation_repository, link_resolver=link_resolver
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -14,9 +14,13 @@ from basic_memory.models.project import Project
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def context_service(search_repository, entity_repository, observation_repository):
|
||||
async def context_service(
|
||||
search_repository, entity_repository, observation_repository, link_resolver
|
||||
):
|
||||
"""Create context service for testing."""
|
||||
return ContextService(search_repository, entity_repository, observation_repository)
|
||||
return ContextService(
|
||||
search_repository, entity_repository, observation_repository, link_resolver=link_resolver
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -333,3 +337,49 @@ async def test_project_isolation_in_find_related(session_maker, app_config):
|
||||
assert entity1_p1.project_id == project1.id
|
||||
assert entity2_p1.project_id == project1.id
|
||||
assert entity1_p2.project_id == project2.id
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_context_fallback_via_link_resolver(context_service, test_graph):
|
||||
"""Test that build_context falls back to LinkResolver when exact permalink fails.
|
||||
|
||||
The test_graph creates entities with permalinks like 'test-project/test/root'.
|
||||
Looking up by title ('Root') won't match the exact permalink, but LinkResolver
|
||||
can resolve it via title matching.
|
||||
"""
|
||||
# This identifier is the entity title, not a permalink — exact lookup will fail
|
||||
url = memory_url.validate_strings("memory://Root")
|
||||
context_result = await context_service.build_context(url)
|
||||
|
||||
# LinkResolver should resolve 'Root' → entity with permalink 'test-project/test/root'
|
||||
assert context_result.metadata.primary_count == 1
|
||||
assert len(context_result.results) == 1
|
||||
assert context_result.results[0].primary_result.id == test_graph["root"].id
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_context_fallback_not_found(context_service):
|
||||
"""Test that build_context returns empty when both exact lookup and fallback fail."""
|
||||
url = memory_url.validate_strings("memory://completely-nonexistent-note-xyz")
|
||||
context_result = await context_service.build_context(url)
|
||||
|
||||
assert context_result.metadata.primary_count == 0
|
||||
assert len(context_result.results) == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_context_without_link_resolver(
|
||||
search_repository, entity_repository, observation_repository, test_graph
|
||||
):
|
||||
"""Test that build_context still works without a link_resolver (no fallback)."""
|
||||
service = ContextService(search_repository, entity_repository, observation_repository)
|
||||
|
||||
# Exact permalink lookup should still work
|
||||
url = memory_url.validate_strings("memory://test-project/test/root")
|
||||
context_result = await service.build_context(url)
|
||||
assert context_result.metadata.primary_count == 1
|
||||
|
||||
# Title-based lookup should return empty (no fallback available)
|
||||
url = memory_url.validate_strings("memory://Root")
|
||||
context_result = await service.build_context(url)
|
||||
assert context_result.metadata.primary_count == 0
|
||||
|
||||
@@ -1101,8 +1101,11 @@ async def test_sync_permalink_not_created_if_no_frontmatter(
|
||||
sync_service: SyncService,
|
||||
project_config: ProjectConfig,
|
||||
file_service: FileService,
|
||||
app_config: BasicMemoryConfig,
|
||||
):
|
||||
"""Test that sync resolves permalink conflicts on update."""
|
||||
"""Test that sync does not add frontmatter when ensure_frontmatter_on_sync is disabled."""
|
||||
app_config.ensure_frontmatter_on_sync = False
|
||||
|
||||
project_dir = project_config.home
|
||||
|
||||
file = project_dir / "one.md"
|
||||
|
||||
@@ -342,15 +342,15 @@ class TestConfigManager:
|
||||
assert config.disable_permalinks is True
|
||||
|
||||
def test_ensure_frontmatter_on_sync_flag_default(self):
|
||||
"""Test that ensure_frontmatter_on_sync defaults to False."""
|
||||
"""Test that ensure_frontmatter_on_sync defaults to True."""
|
||||
config = BasicMemoryConfig()
|
||||
assert config.ensure_frontmatter_on_sync is False
|
||||
|
||||
def test_ensure_frontmatter_on_sync_flag_can_be_enabled(self):
|
||||
"""Test that ensure_frontmatter_on_sync can be set to True."""
|
||||
config = BasicMemoryConfig(ensure_frontmatter_on_sync=True)
|
||||
assert config.ensure_frontmatter_on_sync is True
|
||||
|
||||
def test_ensure_frontmatter_on_sync_flag_can_be_disabled(self):
|
||||
"""Test that ensure_frontmatter_on_sync can be set to False."""
|
||||
config = BasicMemoryConfig(ensure_frontmatter_on_sync=False)
|
||||
assert config.ensure_frontmatter_on_sync is False
|
||||
|
||||
def test_permalinks_include_project_flag_default(self):
|
||||
"""Test that permalinks_include_project defaults to True."""
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
Reference in New Issue
Block a user