Skip to content

Commit 26381ae

Browse files
authored
fix(mcp): use lightweight graph hydration lookup (#806)
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 7918e5c commit 26381ae

4 files changed

Lines changed: 122 additions & 4 deletions

File tree

src/basic_memory/api/v2/utils.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919

2020
class EntityBatchLookup(Protocol):
21-
async def find_by_ids(self, ids: List[int]) -> Sequence[Any]: ...
21+
async def find_by_ids_for_hydration(self, ids: List[int]) -> Sequence[Any]: ...
2222

2323

2424
class EntityServiceBatchLookup(Protocol):
@@ -76,7 +76,7 @@ async def to_graph_context(
7676
if item.to_id:
7777
entity_ids_needed.add(item.to_id)
7878

79-
# Batch fetch all entities at once - get both title and external_id
79+
# Batch fetch just the entity fields needed to shape the response.
8080
entity_title_lookup: dict[int, str] = {}
8181
entity_external_id_lookup: dict[int, str] = {}
8282
if entity_ids_needed:
@@ -87,7 +87,9 @@ async def to_graph_context(
8787
phase="lookup_entities",
8888
result_count=len(entity_ids_needed),
8989
):
90-
entities = await entity_repository.find_by_ids(list(entity_ids_needed))
90+
entities = await entity_repository.find_by_ids_for_hydration(
91+
list(entity_ids_needed)
92+
)
9193
for e in entities:
9294
entity_title_lookup[e.id] = e.title
9395
entity_external_id_lookup[e.id] = e.external_id

src/basic_memory/repository/entity_repository.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from sqlalchemy import select, func
99
from sqlalchemy.exc import IntegrityError
1010
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
11-
from sqlalchemy.orm import selectinload
11+
from sqlalchemy.orm import load_only, selectinload
1212
from sqlalchemy.orm.interfaces import LoaderOption
1313
from sqlalchemy.engine import Row
1414

@@ -178,6 +178,24 @@ async def get_all_permalinks(self) -> List[str]:
178178
result = await self.execute_query(query, use_query_options=False)
179179
return list(result.scalars().all())
180180

181+
async def find_by_ids_for_hydration(self, ids: List[int]) -> Sequence[Entity]:
182+
"""Fetch minimal entity fields needed for context hydration.
183+
184+
Context hydration only needs an entity's primary key, title, and external
185+
UUID. Keeping this separate from find_by_ids avoids the relationship eager
186+
loads that are useful for full entity reads but expensive for response shaping.
187+
"""
188+
if not ids:
189+
return []
190+
191+
query = (
192+
self.select()
193+
.where(Entity.id.in_(ids))
194+
.options(load_only(Entity.id, Entity.title, Entity.external_id))
195+
)
196+
result = await self.execute_query(query, use_query_options=False)
197+
return list(result.scalars().all())
198+
181199
async def get_permalink_to_file_path_map(self) -> dict[str, str]:
182200
"""Get a mapping of permalink -> file_path for all entities.
183201

tests/api/v2/test_memory_hydration.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,25 @@ async def find_by_ids(self, ids: list[int]):
5555
self.calls.append(ids)
5656
return [self.entities_by_id[i] for i in ids if i in self.entities_by_id]
5757

58+
async def find_by_ids_for_hydration(self, ids: list[int]):
59+
self.calls.append(ids)
60+
return [self.entities_by_id[i] for i in ids if i in self.entities_by_id]
61+
62+
63+
class LightweightOnlyEntityRepository:
64+
"""Raises if graph hydration uses the eager-loading repository method."""
65+
66+
def __init__(self, entities_by_id: dict[int, SimpleNamespace]):
67+
self.entities_by_id = entities_by_id
68+
self.hydration_calls: list[list[int]] = []
69+
70+
async def find_by_ids(self, ids: list[int]):
71+
raise AssertionError("graph hydration must use the lightweight hydration lookup")
72+
73+
async def find_by_ids_for_hydration(self, ids: list[int]):
74+
self.hydration_calls.append(ids)
75+
return [self.entities_by_id[i] for i in ids if i in self.entities_by_id]
76+
5877

5978
# --- Single batch fetch (N+1 elimination) ---
6079

@@ -198,3 +217,63 @@ async def test_to_graph_context_empty_results_skip_entity_lookup():
198217

199218
assert repo.calls == []
200219
assert list(graph.results) == []
220+
221+
222+
@pytest.mark.asyncio
223+
async def test_to_graph_context_uses_lightweight_hydration_lookup():
224+
"""Hydration should not load observations/relations when only entity fields are needed."""
225+
repo = LightweightOnlyEntityRepository(
226+
{
227+
1: _make_entity(1, "Root", "ext-root"),
228+
2: _make_entity(2, "Child", "ext-child"),
229+
}
230+
)
231+
now = datetime.now(timezone.utc)
232+
233+
context = ServiceContextResult(
234+
results=[
235+
ContextResultItem(
236+
primary_result=_make_row(
237+
type="entity",
238+
id=1,
239+
root_id=1,
240+
title="Root",
241+
permalink="notes/root",
242+
file_path="notes/root.md",
243+
created_at=now,
244+
),
245+
observations=[],
246+
related_results=[
247+
_make_row(
248+
type="relation",
249+
id=20,
250+
root_id=1,
251+
title="links_to: Child",
252+
permalink="notes/root",
253+
file_path="notes/root.md",
254+
relation_type="links_to",
255+
from_id=1,
256+
to_id=2,
257+
depth=1,
258+
created_at=now,
259+
)
260+
],
261+
)
262+
],
263+
metadata=ContextMetadata(
264+
types=[SearchItemType.ENTITY, SearchItemType.RELATION],
265+
depth=1,
266+
primary_count=1,
267+
related_count=1,
268+
total_relations=1,
269+
),
270+
)
271+
272+
graph = await to_graph_context(context, entity_repository=repo)
273+
274+
assert len(repo.hydration_calls) == 1
275+
assert set(repo.hydration_calls[0]) == {1, 2}
276+
relation = graph.results[0].related_results[0]
277+
assert isinstance(relation, RelationSummary)
278+
assert relation.from_entity_external_id == "ext-root"
279+
assert relation.to_entity_external_id == "ext-child"

tests/repository/test_entity_repository.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,25 @@ async def test_get_all_permalinks(entity_repository: EntityRepository, session_m
10501050
assert isinstance(permalink, str)
10511051

10521052

1053+
@pytest.mark.asyncio
1054+
async def test_find_by_ids_for_hydration_skips_eager_load_options(
1055+
entity_repository: EntityRepository, sample_entity: Entity, monkeypatch: pytest.MonkeyPatch
1056+
):
1057+
"""Context hydration should bypass relationship loader options."""
1058+
1059+
def fail_get_load_options():
1060+
raise AssertionError("hydration lookup must not eager load entity relationships")
1061+
1062+
monkeypatch.setattr(entity_repository, "get_load_options", fail_get_load_options)
1063+
1064+
found = await entity_repository.find_by_ids_for_hydration([sample_entity.id])
1065+
1066+
assert len(found) == 1
1067+
assert found[0].id == sample_entity.id
1068+
assert found[0].title == sample_entity.title
1069+
assert found[0].external_id == sample_entity.external_id
1070+
1071+
10531072
@pytest.mark.asyncio
10541073
async def test_get_permalink_to_file_path_map(entity_repository: EntityRepository, session_maker):
10551074
"""Test getting permalink -> file_path mapping for bulk operations."""

0 commit comments

Comments
 (0)