From 6ac28aeee9f752e428848cb9ff7fb3e63f794384 Mon Sep 17 00:00:00 2001 From: Dennis Hempel Date: Thu, 28 May 2026 09:50:30 +0200 Subject: [PATCH] fix(sync): use strict link resolution for deferred forward references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deferred relation resolution in `sync_service.resolve_forward_references` and `BatchIndexer._resolve_batch_relations` called `LinkResolver.resolve_link(to_name)` with the default `strict=False`. When the link text didn't match any entity's permalink, title, or file_path, the resolver fell through to BM25/ts_rank fuzzy search and silently picked `results[0]` — even for ambiguous links that share only incidental tokens with the wrong entity. Producers commonly emit disambiguator-style wikilinks like `[[overview (state-management/session-execution)]]`. basic-memory does not parse `(qualifier)` as a disambiguator, so no entity matches exactly. The previous behavior resolved such links to whichever unrelated note shared the most tokens, polluting the graph with confidently-wrong edges that `basic-memory status` / `doctor` do not catch. Entity-creation already passes `strict=True` (see `entity_service.update_entity_relations` at line 1030). The deferred sync and batch-indexer paths must use the same contract so unresolved relations stay unresolved (`to_id=NULL`) and surface to the producer instead of getting silently filled with a wrong target. Adds two regression tests that spy on `resolve_link` and assert `strict=True` is passed by both deferred paths. Verified failing on the previous code and passing after. Reported in basic-memory-bug-report Issue 2. Signed-off-by: Dennis Hempel --- src/basic_memory/indexing/batch_indexer.py | 8 ++- src/basic_memory/sync/sync_service.py | 18 +++++- tests/indexing/test_batch_indexer.py | 72 +++++++++++++++++++++ tests/sync/test_sync_service.py | 73 ++++++++++++++++++++++ 4 files changed, 169 insertions(+), 2 deletions(-) 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,