diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py index 08f87bc1..c6e8778d 100644 --- a/src/basic_memory/mcp/tools/search.py +++ b/src/basic_memory/mcp/tools/search.py @@ -14,7 +14,7 @@ from basic_memory.config import ConfigManager, has_cloud_credentials from basic_memory.utils import ( build_canonical_permalink, coerce_dict, - coerce_list, + parse_str_list, parse_tags, strict_search_tags, ) @@ -649,24 +649,30 @@ async def search_notes( # Plural-vs-singular trips models constantly. Accept the singular too. note_types: Annotated[ List[str] | None, - BeforeValidator(coerce_list), + # parse_str_list, not coerce_list: "note,task" must split into ["note", "task"] + # consistent with how tags are handled (#910/#930). coerce_list wraps the whole + # comma string as the single literal type ["note,task"], which matches nothing. + BeforeValidator(parse_str_list), Field(default=None, validation_alias=AliasChoices("note_types", "note_type", "types")), "Filter by the 'type' field in note frontmatter (e.g. 'note', 'chapter', 'person'). " + "Accepts a list, a comma-separated string (e.g. 'note,task'), or a JSON-array string. " "Case-insensitive.", ] = None, entity_types: Annotated[ List[str] | None, - BeforeValidator(coerce_list), + BeforeValidator(parse_str_list), Field(default=None, validation_alias=AliasChoices("entity_types", "entity_type")), "Filter by knowledge graph item type: 'entity' (whole notes), 'observation', or " "'relation'. Defaults to 'entity'. Do NOT pass schema/frontmatter types like " - "'Chapter' here — use note_types instead.", + "'Chapter' here — use note_types instead. " + "Accepts a list, a comma-separated string (e.g. 'entity,observation'), or a JSON-array string.", ] = None, categories: Annotated[ List[str] | None, - BeforeValidator(coerce_list), + BeforeValidator(parse_str_list), Field(default=None, validation_alias=AliasChoices("categories", "category")), "Filter observation results to these exact categories (e.g. ['requirement']). " + "Accepts a list, a comma-separated string (e.g. 'requirement,decision'), or a JSON-array string. " "Pair with entity_types=['observation'] to return only observations whose " "category matches exactly — not every row mentioning the word.", ] = None, diff --git a/src/basic_memory/utils.py b/src/basic_memory/utils.py index 05249498..43ed9490 100644 --- a/src/basic_memory/utils.py +++ b/src/basic_memory/utils.py @@ -600,6 +600,55 @@ def strict_search_tags(v: Any) -> Any: return v +def parse_str_list(v: Any) -> List[str]: + """Parse a list of plain strings from various input formats. + + Like parse_tags but without stripping '#' — correct for type/category params + where the value is a literal identifier, not a hashtag. + + Handles the four input shapes that MCP clients commonly produce: + - None → [] + - "note,task" → ["note", "task"] (comma-split string) + - '["note","task"]' → ["note", "task"] (JSON array string) + - ["note,task"] → ["note", "task"] (list with comma-string element) + + Non-str/list/None values are returned unchanged so Pydantic can reject them + with a clear validation error instead of silently coercing. + """ + if v is None: + return [] + + if isinstance(v, list): + # Trigger: a list element may itself be a comma-separated string (e.g. some MCP clients + # serialise `["note,task"]` when the caller passed `note_types="note,task"`). + # Outcome: flatten each element by splitting on commas and stripping whitespace. + return [ + item.strip() + for raw in v + if raw is not None + for item in str(raw).split(",") + if item and item.strip() + ] + + if isinstance(v, str): + # Trigger: MCP clients sometimes send a JSON array string like '["note","task"]'. + # Outcome: parse it as JSON first, then recurse to handle the resulting list. + stripped = v.strip() + if stripped.startswith("[") and stripped.endswith("]"): + try: + parsed_json = json.loads(stripped) + if isinstance(parsed_json, list): + return parse_str_list(parsed_json) + except json.JSONDecodeError: + pass + + # Plain comma-separated string: "note,task" → ["note", "task"] + return [item.strip() for item in v.split(",") if item and item.strip()] + + # Non-str/list/None — return unchanged so Pydantic rejects with a clear error. + return v # type: ignore[return-value] + + def coerce_list(v: Any) -> Any: """Coerce string input to list for MCP clients that serialize lists as strings.""" if v is None: diff --git a/tests/test_coerce.py b/tests/test_coerce.py index a3aebc82..1c826058 100644 --- a/tests/test_coerce.py +++ b/tests/test_coerce.py @@ -1,9 +1,9 @@ -"""Tests for coerce_list, coerce_dict, and strict_search_tags utility functions. +"""Tests for coerce_list, coerce_dict, strict_search_tags, and parse_str_list utility functions. These must fail until the helpers are implemented in utils.py. """ -from basic_memory.utils import coerce_list, coerce_dict, strict_search_tags +from basic_memory.utils import coerce_dict, coerce_list, parse_str_list, strict_search_tags class TestCoerceList: @@ -113,3 +113,66 @@ class TestCoerceDict: def test_int_passthrough(self): assert coerce_dict(42) == 42 + + +class TestParseStrList: + """Tests for parse_str_list — the comma-split coercer for note_types/entity_types/categories.""" + + # --- None input --- + + def test_none_returns_empty_list(self): + assert parse_str_list(None) == [] + + # --- Single string inputs --- + + def test_single_string_wraps_as_one_element(self): + assert parse_str_list("note") == ["note"] + + def test_comma_string_splits(self): + """The primary motivation for this function: "note,task" → ["note", "task"].""" + assert parse_str_list("note,task") == ["note", "task"] + + def test_comma_string_with_spaces_strips(self): + assert parse_str_list("note, task, person") == ["note", "task", "person"] + + # --- JSON array string inputs (MCP clients sometimes serialize lists as strings) --- + + def test_json_array_string(self): + assert parse_str_list('["note", "task"]') == ["note", "task"] + + def test_json_array_string_single_element(self): + assert parse_str_list('["note"]') == ["note"] + + def test_json_array_string_with_comma_elements(self): + """JSON array where an element is itself a comma-string — flatten it.""" + assert parse_str_list('["note,task"]') == ["note", "task"] + + # --- List inputs --- + + def test_plain_list_passthrough(self): + assert parse_str_list(["note", "task"]) == ["note", "task"] + + def test_list_with_comma_element_splits(self): + """A list containing a comma-string is flattened.""" + assert parse_str_list(["note,task"]) == ["note", "task"] + + def test_list_with_multiple_comma_elements(self): + assert parse_str_list(["note,task", "person"]) == ["note", "task", "person"] + + # --- No '#' stripping (unlike parse_tags) --- + + def test_hash_prefix_preserved(self): + """parse_str_list must NOT strip '#' — these are type identifiers, not hashtags.""" + assert parse_str_list("#type") == ["#type"] + + def test_hash_prefix_in_comma_string_preserved(self): + assert parse_str_list("#type,#other") == ["#type", "#other"] + + # --- Non-str/list/None pass through for Pydantic rejection --- + + def test_int_passthrough_for_pydantic_rejection(self): + assert parse_str_list(42) == 42 # type: ignore[arg-type] + + def test_dict_passthrough_for_pydantic_rejection(self): + value = {"a": 1} + assert parse_str_list(value) is value # type: ignore[arg-type]