diff --git a/src/basic_memory/indexing/batch_indexer.py b/src/basic_memory/indexing/batch_indexer.py index a670d708..9a6d3b47 100644 --- a/src/basic_memory/indexing/batch_indexer.py +++ b/src/basic_memory/indexing/batch_indexer.py @@ -493,8 +493,14 @@ class BatchIndexer: async def resolve_relation(relation: Relation) -> int: async with semaphore: try: + # strict=True for deferred resolution: only fill in to_id on an + # exact permalink/title/file_path match. Fuzzy fallback would silently + # resolve ambiguous links to whichever entity shares tokens with the + # link text, mismatching this with the sync_service forward-reference + # path and producing confidently-wrong graph edges. See + # sync_service.resolve_forward_references for the same change. resolved_entity = await self.entity_service.link_resolver.resolve_link( - relation.to_name + relation.to_name, strict=True ) if resolved_entity is None or resolved_entity.id == relation.from_id: return 0 diff --git a/src/basic_memory/sync/sync_service.py b/src/basic_memory/sync/sync_service.py index ab27da76..0e623952 100644 --- a/src/basic_memory/sync/sync_service.py +++ b/src/basic_memory/sync/sync_service.py @@ -1447,7 +1447,23 @@ class SyncService: f"to_name={relation.to_name}" ) - resolved_entity = await self.entity_service.link_resolver.resolve_link(relation.to_name) + # Use strict=True: deferred resolution should only fill in to_id when an + # exact permalink/title/file_path match exists. The fuzzy fallback (search-based + # token match) would silently resolve ambiguous links like + # `[[overview (state-management/session-execution)]]` to whichever entity shares + # the most tokens, polluting the graph with confidently-wrong edges that no + # audit catches. Leaving such relations unresolved keeps to_id=NULL so they + # surface as forward references and can be fixed by the producer. + # Use strict=True: deferred resolution should only fill in to_id when an + # exact permalink/title/file_path match exists. The fuzzy fallback (search-based + # token match) would silently resolve ambiguous links like + # `[[overview (state-management/session-execution)]]` to whichever entity shares + # the most tokens, polluting the graph with confidently-wrong edges that no + # audit catches. Leaving such relations unresolved keeps to_id=NULL so they + # surface as forward references and can be fixed by the producer. + resolved_entity = await self.entity_service.link_resolver.resolve_link( + relation.to_name, strict=True + ) # ignore reference to self if resolved_entity and resolved_entity.id != relation.from_id: diff --git a/tests/indexing/test_batch_indexer.py b/tests/indexing/test_batch_indexer.py index 90bccf09..f60cf5ea 100644 --- a/tests/indexing/test_batch_indexer.py +++ b/tests/indexing/test_batch_indexer.py @@ -688,6 +688,78 @@ async def test_batch_indexer_index_markdown_file_can_defer_relation_resolution( assert source.outgoing_relations[0].to_name == "Deferred Target" +@pytest.mark.asyncio +async def test_batch_indexer_uses_strict_link_resolution_for_deferred_relations( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + project_config, + monkeypatch, +): + """Regression: batch indexer's deferred relation resolution must call + resolve_link with strict=True. + + Mirror of sync_service.resolve_forward_references. Fuzzy fallback in the + deferred path silently fills in to_id from BM25/ts_rank results, polluting + the graph with confidently-wrong edges. Entity-creation already uses + strict=True; this is the other deferred path. + """ + path = "notes/source.md" + await _create_file( + project_config.home / path, + dedent( + """ + --- + title: Source + type: note + --- + + # Source + + - links_to [[never-resolves-target]] + """ + ), + ) + + batch_indexer = _make_batch_indexer( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + ) + + original_resolve_link = entity_service.link_resolver.resolve_link + seen_strict: list[object] = [] + + async def spy_resolve_link(*args, **kwargs): + seen_strict.append(kwargs.get("strict", False)) + return await original_resolve_link(*args, **kwargs) + + monkeypatch.setattr(entity_service.link_resolver, "resolve_link", spy_resolve_link) + + await batch_indexer.index_files( + {path: await _load_input(file_service, path)}, + max_concurrent=1, + ) + + assert seen_strict, "batch indexer did not invoke link_resolver.resolve_link" + assert all(strict is True for strict in seen_strict), ( + f"Deferred resolution must call resolve_link(strict=True). Observed: {seen_strict!r}" + ) + + # The unresolvable relation stayed unresolved. + source = await entity_repository.get_by_file_path(path) + assert source is not None + assert len(source.outgoing_relations) == 1 + assert source.outgoing_relations[0].to_id is None + assert source.outgoing_relations[0].to_name == "never-resolves-target" + + @pytest.mark.asyncio async def test_batch_indexer_strips_frontmatter_from_search_content_when_body_is_empty( app_config, diff --git a/tests/sync/test_sync_service.py b/tests/sync/test_sync_service.py index db76f996..d43c113e 100644 --- a/tests/sync/test_sync_service.py +++ b/tests/sync/test_sync_service.py @@ -108,6 +108,79 @@ Target content assert source.relations[0].to_name == target.title +@pytest.mark.asyncio +async def test_resolve_relations_uses_strict_link_resolution( + sync_service: SyncService, + project_config: ProjectConfig, + entity_service: EntityService, + monkeypatch, +): + """Regression: deferred forward-reference resolution must call resolve_link + with strict=True. + + Producers sometimes emit disambiguator-style links like + `[[overview (state-management/session-execution)]]` whose exact text does + not match any entity's permalink, title, or file_path. The previous + behavior fell through to BM25/ts_rank fuzzy search in + LinkResolver._resolve_in_project and silently picked whichever entity + shared the most tokens — polluting the graph with confidently-wrong edges + that no audit catches. + + Entity-creation already resolves relations with strict=True (see + entity_service.update_entity_relations). The deferred sync path must use + the same contract; otherwise unresolved relations get silently filled + later by fuzzy search. + """ + # Create a source file with a forward reference. The target doesn't exist, + # so resolution will fail — which is exactly when fuzzy fallback would + # previously silently pick a wrong target. + source_content = dedent(""" + --- + type: knowledge + --- + # Source + + ## Relations + - part_of [[never-resolves-target]] + """) + await create_test_file(project_config.home / "source.md", source_content) + await sync_service.sync(project_config.home) + + project_prefix = generate_permalink(project_config.name) + source = await entity_service.get_by_permalink(f"{project_prefix}/source") + assert len(source.relations) == 1 + assert source.relations[0].to_id is None # initial creation already strict + + # Spy on resolve_link to capture the strict flag the deferred resolver uses. + original_resolve_link = sync_service.entity_service.link_resolver.resolve_link + seen_strict: list[Any] = [] + + async def spy_resolve_link(*args, **kwargs): + seen_strict.append(kwargs.get("strict", False)) + return await original_resolve_link(*args, **kwargs) + + monkeypatch.setattr( + sync_service.entity_service.link_resolver, + "resolve_link", + spy_resolve_link, + ) + + await sync_service.resolve_relations() + + # Deferred resolution invoked resolve_link, and every call passed strict=True. + assert seen_strict, "resolve_relations did not invoke link_resolver.resolve_link" + assert all(strict is True for strict in seen_strict), ( + f"Deferred resolution must call resolve_link(strict=True) to avoid silent " + f"fuzzy matching. Observed strict values: {seen_strict!r}" + ) + + # Sanity check: the unresolvable relation stayed unresolved — no silent + # fuzzy match polluted it. + source = await entity_service.get_by_permalink(f"{project_prefix}/source") + assert source.relations[0].to_id is None + assert source.relations[0].to_name == "never-resolves-target" + + @pytest.mark.asyncio async def test_resolve_relations_deletes_duplicate_unresolved_relation( sync_service: SyncService,