mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
Compare commits
2 Commits
main
...
codex/pr-861-ci
| Author | SHA1 | Date | |
|---|---|---|---|
| 3480e917e2 | |||
| 6ac28aeee9 |
@@ -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
|
||||
|
||||
@@ -1447,7 +1447,16 @@ 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.
|
||||
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:
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user