mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(mcp): split comma-separated tags in search_notes tags param (#932)
Closes #910 Signed-off-by: phernandez <paul@basicmemory.com>
This commit is contained in:
@@ -11,7 +11,7 @@ from fastmcp import Context
|
||||
from pydantic import AliasChoices, BeforeValidator, Field
|
||||
|
||||
from basic_memory.config import ConfigManager, has_cloud_credentials
|
||||
from basic_memory.utils import build_canonical_permalink, coerce_dict, coerce_list
|
||||
from basic_memory.utils import build_canonical_permalink, coerce_dict, coerce_list, parse_tags
|
||||
from basic_memory.mcp.async_client import (
|
||||
_explicit_routing,
|
||||
_force_local_mode,
|
||||
@@ -676,9 +676,13 @@ async def search_notes(
|
||||
Dict[str, Any] | None,
|
||||
BeforeValidator(coerce_dict),
|
||||
] = None,
|
||||
# parse_tags, not coerce_list: tags="a,b" must split into ["a", "b"] to match the
|
||||
# tag: query shorthand below and write_note's documented tags convention (#910).
|
||||
# coerce_list would wrap the comma string as the single literal tag ["a,b"],
|
||||
# which matches nothing.
|
||||
tags: Annotated[
|
||||
List[str] | None,
|
||||
BeforeValidator(coerce_list),
|
||||
BeforeValidator(parse_tags),
|
||||
] = None,
|
||||
status: Optional[str] = None,
|
||||
min_similarity: Annotated[
|
||||
@@ -795,7 +799,9 @@ async def search_notes(
|
||||
observations whose category matches exactly.
|
||||
after_date: Optional date filter for recent content (e.g., "1 week", "2d", "2024-01-01")
|
||||
metadata_filters: Optional structured frontmatter filters (e.g., {"status": "in-progress"})
|
||||
tags: Optional tag filter (frontmatter tags); shorthand for metadata_filters["tags"]
|
||||
tags: Optional tag filter (frontmatter tags); shorthand for metadata_filters["tags"].
|
||||
Accepts a list (["a", "b"]) or a comma-separated string ("a,b"), matching the
|
||||
write_note tags convention and the tag: query shorthand.
|
||||
status: Optional status filter (frontmatter status); shorthand for metadata_filters["status"]
|
||||
min_similarity: Optional float to override the global semantic_min_similarity threshold
|
||||
for this query. E.g., 0.0 to see all vector results, or 0.8 for high precision.
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
"""Tests for search MCP tools."""
|
||||
|
||||
import inspect
|
||||
|
||||
import pytest
|
||||
from contextlib import asynccontextmanager
|
||||
from datetime import datetime, timedelta
|
||||
from types import SimpleNamespace
|
||||
from typing import cast
|
||||
|
||||
from pydantic import TypeAdapter
|
||||
|
||||
from basic_memory.mcp.tools import write_note
|
||||
from basic_memory.mcp.tools.search import (
|
||||
search_notes,
|
||||
@@ -1622,6 +1626,74 @@ async def test_search_notes_tag_prefix_with_remaining_text(monkeypatch):
|
||||
assert captured_payload["text"] == "authentication"
|
||||
|
||||
|
||||
# --- Tests for comma-separated tags parameter (#910) ----------------------------
|
||||
|
||||
|
||||
def test_search_notes_tags_annotation_splits_comma_strings():
|
||||
"""The tags parameter annotation must parse every documented input form (#910).
|
||||
|
||||
Direct function calls bypass the BeforeValidator, so validate through the same
|
||||
Annotated metadata pydantic applies on the MCP path. coerce_list wrapped a bare
|
||||
comma string as the single literal tag ["a,b"]; parse_tags splits it like the
|
||||
tag: query shorthand and write_note's tags convention.
|
||||
"""
|
||||
annotation = inspect.signature(search_notes).parameters["tags"].annotation
|
||||
adapter = TypeAdapter(annotation)
|
||||
|
||||
real_list = adapter.validate_python(["a", "b"])
|
||||
comma_string = adapter.validate_python("a,b")
|
||||
json_string = adapter.validate_python('["a", "b"]')
|
||||
single_string = adapter.validate_python("a")
|
||||
|
||||
assert real_list == ["a", "b"]
|
||||
# The comma string and the real list must behave identically (the #910 bug).
|
||||
assert comma_string == real_list
|
||||
assert json_string == real_list
|
||||
assert single_string == ["a"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_notes_tags_comma_string_filters_via_mcp(mcp, client, test_project):
|
||||
"""tags="alpha,beta" through the real MCP layer must match like a real list (#910)."""
|
||||
from fastmcp import Client
|
||||
|
||||
async with Client(mcp) as mcp_client:
|
||||
await mcp_client.call_tool(
|
||||
"write_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"title": "Tag Split Note",
|
||||
"directory": "test",
|
||||
"content": "# Tag Split Note\nTagSplitToken body",
|
||||
"tags": ["alpha", "beta"],
|
||||
},
|
||||
)
|
||||
|
||||
async def found(tags_value: object) -> bool:
|
||||
result = await mcp_client.call_tool(
|
||||
"search_notes",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"query": "TagSplitToken",
|
||||
"search_type": "text",
|
||||
"tags": tags_value,
|
||||
},
|
||||
)
|
||||
return "Tag Split Note" in result.content[0].text
|
||||
|
||||
as_list = await found(["alpha", "beta"])
|
||||
as_comma_string = await found("alpha,beta")
|
||||
as_json_string = await found('["alpha", "beta"]')
|
||||
as_single_string = await found("alpha")
|
||||
|
||||
assert as_list, "real-list tags must match (sanity)"
|
||||
assert as_comma_string == as_list, "comma string must behave like the real list"
|
||||
assert as_json_string == as_list
|
||||
assert as_single_string == as_list
|
||||
# Negative control: the filter is actually applied, not silently dropped.
|
||||
assert not await found("gamma")
|
||||
|
||||
|
||||
# --- Tests for text output format (#641) -----------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user