feat(mcp): typed-entity tools (person/place/list)

Nine tools — list/create/update for each of person, place, list. Get and delete reuse fable_get_note / fable_delete_note (typed entities share the Note model). Lists: the wrappers accept an `items: list[str]` for ergonomics and translate to the {text, checked} dict shape that services/knowledge.py and KnowledgeView.vue expect. items=[] clears; items=None leaves unchanged. Updates do an explicit get → merge → update round trip so updating one typed field doesn't clobber the others stored alongside it in entity_meta (which is a single JSONB column). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-26 20:53:36 -04:00
parent 6961144c3a
commit d4f3516552
3 changed files with 438 additions and 1 deletions
@@ -0,0 +1,191 @@
+"""Tests for typed-entity tools (person/place/list).
+
+Focuses on the metadata-shape translations and the entity_meta merge logic,
+since that's where bugs would hide."""
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from fabledassistant.mcp._context import _user_id_ctx
+from fabledassistant.mcp.tools.entities import (
+    fable_list_persons, fable_create_person, fable_update_person,
+    fable_create_place, fable_update_place,
+    fable_create_list, fable_update_list,
+)
+
+
+@pytest.fixture(autouse=True)
+def _bind_user():
+    token = _user_id_ctx.set(7)
+    yield
+    _user_id_ctx.reset(token)
+
+
+def _fake_note(*, note_type="note", entity_meta=None, **overrides) -> MagicMock:
+    n = MagicMock()
+    n.note_type = note_type
+    n.entity_meta = entity_meta
+    base = {"id": 1, "title": "t", "note_type": note_type,
+            "metadata": entity_meta or {}}
+    base.update(overrides)
+    n.to_dict.return_value = base
+    return n
+
+
+# ─── list ────────────────────────────────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_list_persons_calls_knowledge_with_person_type():
+    mock = AsyncMock(return_value=([{"id": 1, "title": "alice"}], 1))
+    with patch("fabledassistant.mcp.tools.entities.knowledge_svc.query_knowledge", mock):
+        out = await fable_list_persons(tag="work")
+    kwargs = mock.call_args.kwargs
+    assert kwargs["note_type"] == "person"
+    assert kwargs["tags"] == ["work"]
+    assert out["total"] == 1
+    assert "persons" in out  # type-specific plural key
+
+
+# ─── create: metadata building ───────────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_create_person_only_includes_provided_fields_in_meta():
+    """Empty-string fields must NOT pollute entity_meta."""
+    fake = _fake_note(note_type="person")
+    mock = AsyncMock(return_value=fake)
+    with patch("fabledassistant.mcp.tools.entities.notes_svc.create_note", mock):
+        await fable_create_person(name="Alice", email="a@x.com")
+    kwargs = mock.call_args.kwargs
+    assert kwargs["title"] == "Alice"
+    assert kwargs["note_type"] == "person"
+    assert kwargs["entity_meta"] == {"email": "a@x.com"}
+
+
+@pytest.mark.asyncio
+async def test_create_person_all_empty_meta_is_none():
+    """Service is called with entity_meta=None when no typed fields were given."""
+    fake = _fake_note(note_type="person")
+    mock = AsyncMock(return_value=fake)
+    with patch("fabledassistant.mcp.tools.entities.notes_svc.create_note", mock):
+        await fable_create_person(name="Bob")
+    assert mock.call_args.kwargs["entity_meta"] is None
+
+
+@pytest.mark.asyncio
+async def test_create_place_categories_into_meta():
+    fake = _fake_note(note_type="place")
+    mock = AsyncMock(return_value=fake)
+    with patch("fabledassistant.mcp.tools.entities.notes_svc.create_note", mock):
+        await fable_create_place(name="Cafe X", address="123 Main", category="coffee")
+    meta = mock.call_args.kwargs["entity_meta"]
+    assert meta == {"address": "123 Main", "category": "coffee"}
+
+
+@pytest.mark.asyncio
+async def test_create_list_translates_strings_to_unchecked_items():
+    fake = _fake_note(note_type="list")
+    mock = AsyncMock(return_value=fake)
+    with patch("fabledassistant.mcp.tools.entities.notes_svc.create_note", mock):
+        await fable_create_list(name="shopping", items=["milk", "bread"])
+    meta = mock.call_args.kwargs["entity_meta"]
+    assert meta["list_items"] == [
+        {"text": "milk", "checked": False},
+        {"text": "bread", "checked": False},
+    ]
+
+
+# ─── update: meta merge ──────────────────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_update_person_merges_meta_preserving_other_fields():
+    """Updating one field must NOT clobber the others stored in entity_meta."""
+    existing = _fake_note(
+        id=5, note_type="person",
+        entity_meta={"email": "old@x.com", "phone": "555-1234"},
+    )
+    get_mock = AsyncMock(return_value=existing)
+    updated = _fake_note(note_type="person")
+    update_mock = AsyncMock(return_value=updated)
+    with patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.get_note", get_mock,
+    ), patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.update_note", update_mock,
+    ):
+        await fable_update_person(person_id=5, email="new@x.com")
+    new_meta = update_mock.call_args.kwargs["entity_meta"]
+    assert new_meta == {"email": "new@x.com", "phone": "555-1234"}
+
+
+@pytest.mark.asyncio
+async def test_update_person_no_typed_fields_keeps_meta_unchanged():
+    existing = _fake_note(
+        id=5, note_type="person",
+        entity_meta={"email": "a@x.com"},
+    )
+    updated = _fake_note(note_type="person")
+    with patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.get_note",
+        AsyncMock(return_value=existing),
+    ), patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.update_note",
+        AsyncMock(return_value=updated),
+    ) as update_mock:
+        await fable_update_person(person_id=5, name="New Name")
+    # entity_meta unchanged, but still passed (service gets the full new dict)
+    assert update_mock.call_args.kwargs["entity_meta"] == {"email": "a@x.com"}
+    assert update_mock.call_args.kwargs["title"] == "New Name"
+
+
+@pytest.mark.asyncio
+async def test_update_person_rejects_wrong_type():
+    """Trying to update a 'place' as a 'person' must fail."""
+    wrong_type = _fake_note(id=5, note_type="place")
+    with patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.get_note",
+        AsyncMock(return_value=wrong_type),
+    ):
+        with pytest.raises(ValueError, match="person 5 not found"):
+            await fable_update_person(person_id=5, email="x@x.com")
+
+
+@pytest.mark.asyncio
+async def test_update_list_items_empty_list_clears_items():
+    """items=[] clears all items; items=None leaves unchanged."""
+    existing = _fake_note(
+        id=5, note_type="list",
+        entity_meta={"list_items": [{"text": "old", "checked": True}]},
+    )
+    updated = _fake_note(note_type="list")
+    with patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.get_note",
+        AsyncMock(return_value=existing),
+    ), patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.update_note",
+        AsyncMock(return_value=updated),
+    ) as update_mock:
+        await fable_update_list(list_id=5, items=[])
+    assert update_mock.call_args.kwargs["entity_meta"]["list_items"] == []
+
+
+@pytest.mark.asyncio
+async def test_update_list_items_none_leaves_items_unchanged():
+    existing = _fake_note(
+        id=5, note_type="list",
+        entity_meta={"list_items": [{"text": "keep", "checked": False}]},
+    )
+    updated = _fake_note(note_type="list")
+    with patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.get_note",
+        AsyncMock(return_value=existing),
+    ), patch(
+        "fabledassistant.mcp.tools.entities.notes_svc.update_note",
+        AsyncMock(return_value=updated),
+    ) as update_mock:
+        await fable_update_list(list_id=5, name="renamed")
+    # Existing items intact in the merged meta
+    assert update_mock.call_args.kwargs["entity_meta"]["list_items"] == [
+        {"text": "keep", "checked": False},
+    ]