From bee3ca41da0d866154196decb0b5d7d7e872fc0b Mon Sep 17 00:00:00 2001 From: phernandez Date: Tue, 9 Jun 2026 17:40:18 -0500 Subject: [PATCH] fix(mcp): split comma-separated tags in search_notes tags param search_notes(tags="alpha,beta") returned zero results because BeforeValidator(coerce_list) wrapped the bare string as the single literal tag ["alpha,beta"]. This was inconsistent with the tag: query shorthand in the same tool (which splits commas) and with write_note's documented tags convention. Use parse_tags as the validator instead: it handles real lists, JSON-array strings, comma-separated strings, and strips leading '#', keeping all tag entry points on one source of truth. coerce_list is left unchanged because canvas.py uses it for nodes/edges (lists of dicts) that must never comma-split. Regression tests cover the annotation wiring directly and the full MCP validation path via fastmcp Client, asserting comma-string and real-list inputs behave identically, plus a negative control proving the filter still applies. Fixes #910 Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: phernandez --- src/basic_memory/mcp/tools/search.py | 12 +++-- tests/mcp/test_tool_search.py | 72 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py index 5ec3bcc37..562ed4111 100644 --- a/src/basic_memory/mcp/tools/search.py +++ b/src/basic_memory/mcp/tools/search.py @@ -11,7 +11,7 @@ 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. diff --git a/tests/mcp/test_tool_search.py b/tests/mcp/test_tool_search.py index 1660af40a..9acecafa6 100644 --- a/tests/mcp/test_tool_search.py +++ b/tests/mcp/test_tool_search.py @@ -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 fake_resolve(client, query, project, context): 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) -----------------------------------