fix(core): parse picoschema modifier descriptions (#796)

Signed-off-by: Drew Cain <groksrc@gmail.com>
Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: phernandez <paul@basicmachines.co>

Author: groksrc

src/basic_memory/schema/parser.py

@@ -8,7 +8,9 @@
   field: type, description          # required field
   field?: type, description         # optional field
   field(array): type                # array of values
+  field(array, description): type   # array with description
   field?(enum): [val1, val2]        # enumeration
+  field?(enum, description): [val1, val2]  # enum with description
   field?(object):                   # nested object
     sub_field: type
   EntityName as type (capitalized)  # entity reference
@@ -58,46 +60,88 @@ class SchemaDefinition:
 # with an uppercase letter is treated as an entity reference.
 
 SCALAR_TYPES = frozenset({"string", "integer", "number", "boolean", "any"})
+MODIFIER_TYPES = frozenset({"array", "enum", "object"})
 
 
 # --- Field Name Parsing ---
 
 
-def _parse_field_key(key: str) -> tuple[str, bool, bool, bool, bool]:
+def _parse_field_key_parts(key: str) -> tuple[str, bool, bool, bool, bool, str | None]:
     """Parse a Picoschema field key into its components.
 
-    Returns (name, required, is_array, is_enum, is_object).
-    The key format is: name[?][(array|enum|object)]
+    Returns (name, required, is_array, is_enum, is_object, description).
+    The key format is: name[?][(array|enum|object[, description])]
 
     Examples:
-        "name"           -> ("name", True, False, False)
-        "role?"          -> ("role", False, False, False)
-        "tags?(array)"   -> ("tags", False, True, False)
-        "status?(enum)"  -> ("status", False, False, True)
-        "metadata?(object)" -> ("metadata", False, False, False) + children
+        "name"           -> ("name", True, False, False, False, None)
+        "role?"          -> ("role", False, False, False, False, None)
+        "tags?(array)"   -> ("tags", False, True, False, False, None)
+        "tags?(array, labels)" -> ("tags", False, True, False, False, "labels")
+        "status?(enum)"  -> ("status", False, False, True, False, None)
+        "metadata?(object)" -> ("metadata", False, False, False, True, None)
     """
     required = True
     is_array = False
     is_enum = False
     is_object = False
+    description = None
+
+    key, modifier, description = _split_modifier_suffix(key)
 
-    # Check for modifier suffix: (array), (enum), (object)
-    if key.endswith("(array)"):
+    if modifier == "array":
         is_array = True
-        key = key[: -len("(array)")]
-    elif key.endswith("(enum)"):
+    elif modifier == "enum":
         is_enum = True
-        key = key[: -len("(enum)")]
-    elif key.endswith("(object)"):
+    elif modifier == "object":
         is_object = True
-        key = key[: -len("(object)")]
 
     # Check for optional marker
     if key.endswith("?"):
         required = False
         key = key[:-1]
 
-    return key, required, is_array, is_enum, is_object
+    return key.strip(), required, is_array, is_enum, is_object, description
+
+
+def _parse_field_key(key: str) -> tuple[str, bool, bool, bool, bool]:
+    """Parse a Picoschema field key, discarding any modifier description."""
+    name, required, is_array, is_enum, is_object, _description = _parse_field_key_parts(key)
+    return name, required, is_array, is_enum, is_object
+
+
+def _split_modifier_suffix(key: str) -> tuple[str, str | None, str | None]:
+    """Split a trailing picoschema modifier from a field key."""
+    stripped_key = key.rstrip()
+    if not stripped_key.endswith(")"):
+        return key, None, None
+
+    # Trigger: field names and modifier descriptions may both contain parentheses
+    # Why: only the parenthesis paired with the final suffix can introduce a modifier
+    # Outcome: preserves names like "risk(score)" and descriptions like "labels (freeform)"
+    open_paren_index = -1
+    depth = 0
+    for index in range(len(stripped_key) - 1, -1, -1):
+        char = stripped_key[index]
+        if char == ")":
+            depth += 1
+        elif char == "(":
+            depth -= 1
+            if depth == 0:
+                open_paren_index = index
+                break
+
+    if open_paren_index == -1:
+        return key, None, None
+
+    modifier_text = stripped_key[open_paren_index + 1 : -1].strip()
+    modifier, separator, description = modifier_text.partition(",")
+    modifier = modifier.strip()
+    if modifier not in MODIFIER_TYPES:
+        return key, None, None
+
+    key_without_modifier = stripped_key[:open_paren_index].rstrip()
+    parsed_description = description.strip() if separator else None
+    return key_without_modifier, modifier, parsed_description or None
 
 
 def _parse_type_and_description(value: str) -> tuple[str, str | None]:
@@ -170,7 +214,7 @@ def parse_picoschema(yaml_dict: dict) -> list[SchemaField]:
     fields: list[SchemaField] = []
 
     for key, value in yaml_dict.items():
-        name, required, is_array, is_enum, is_object = _parse_field_key(key)
+        name, required, is_array, is_enum, is_object, key_description = _parse_field_key_parts(key)
 
         # --- Enum fields ---
         # Trigger: value is a list or a string containing bracketed enum values
@@ -179,11 +223,12 @@ def parse_picoschema(yaml_dict: dict) -> list[SchemaField]:
         #   in YAML to avoid parse errors)
         # Outcome: SchemaField with is_enum=True and enum_values populated
         if is_enum:
-            description = None
+            description = key_description
             if isinstance(value, list):
                 enum_values = [str(v) for v in value]
             else:
-                enum_values, description = _parse_enum_string(str(value))
+                enum_values, value_description = _parse_enum_string(str(value))
+                description = description or value_description
             fields.append(
                 SchemaField(
                     name=name,
@@ -207,13 +252,15 @@ def parse_picoschema(yaml_dict: dict) -> list[SchemaField]:
                     name=name,
                     type="object",
                     required=required,
+                    description=key_description,
                     children=children,
                 )
             )
             continue
 
         # --- Scalar and entity ref fields ---
-        type_str, description = _parse_type_and_description(str(value))
+        type_str, value_description = _parse_type_and_description(str(value))
+        description = key_description or value_description
         is_entity_ref = _is_entity_ref_type(type_str)
 
         fields.append(

tests/mcp/test_tool_schema.py

@@ -116,6 +116,71 @@ async def test_schema_validate_json_output(app, test_project, sync_service):
     assert result["results"][0]["passed"] is True
 
 
+@pytest.mark.asyncio
+async def test_schema_validate_picoschema_modifier_descriptions(app, test_project, sync_service):
+    """Modifier descriptions should not become literal field names."""
+    project_path = Path(test_project.path)
+
+    _write_schema_file(
+        project_path,
+        "schemas/PicoTest.md",
+        """\
+---
+title: PicoTest
+type: schema
+entity: pico_test
+schema:
+  name: string
+  status(enum, current state): [active, inactive]
+  tags(array, list of tags): string
+settings:
+  validation: warn
+---
+
+# PicoTest
+""",
+    )
+    _write_schema_file(
+        project_path,
+        "pico/PicoTest1.md",
+        """\
+---
+title: PicoTest1
+type: pico_test
+permalink: pico/pico-test-1
+---
+
+# PicoTest1
+
+## Observations
+- [name] PicoTest1
+- [status] active
+- [tags] foo
+- [tags] bar
+""",
+    )
+
+    await sync_service.sync(project_path)
+
+    result = await schema_validate(
+        note_type="pico_test",
+        project=test_project.name,
+        output_format="json",
+    )
+
+    assert isinstance(result, dict)
+    note_result = result["results"][0]
+    field_statuses = {fr["field_name"]: fr["status"] for fr in note_result["field_results"]}
+    assert result["valid_count"] == 1
+    assert note_result["warnings"] == []
+    assert note_result["unmatched_observations"] == {}
+    assert field_statuses == {
+        "name": "present",
+        "status": "present",
+        "tags": "present",
+    }
+
+
 @pytest.mark.asyncio
 async def test_schema_validate_by_identifier(app, test_project, sync_service):
     """Validate a specific note by identifier."""

tests/mcp/test_tool_search.py

@@ -28,7 +28,10 @@ async def test_search_text(client, test_project):
 
     # Search for it (use json format to inspect structured results)
     response = await search_notes(
-        project=test_project.name, query="searchable", output_format="json"
+        project=test_project.name,
+        query="searchable",
+        search_type="text",
+        output_format="json",
     )
 
     # Verify results - handle both success and error cases
@@ -183,7 +186,12 @@ async def test_search_pagination(client, test_project):
 
     # Search for it (use json format to inspect structured results)
     response = await search_notes(
-        project=test_project.name, query="searchable", page=1, page_size=1, output_format="json"
+        project=test_project.name,
+        query="searchable",
+        search_type="text",
+        page=1,
+        page_size=1,
+        output_format="json",
     )
 
     # Verify results - handle both success and error cases
@@ -212,7 +220,11 @@ async def test_search_with_type_filter(client, test_project):
 
     # Search with note type filter (use json format to inspect structured results)
     response = await search_notes(
-        project=test_project.name, query="type", note_types=["note"], output_format="json"
+        project=test_project.name,
+        query="type",
+        search_type="text",
+        note_types=["note"],
+        output_format="json",
     )
 
     # Verify results - handle both success and error cases
@@ -237,7 +249,11 @@ async def test_search_with_entity_type_filter(client, test_project):
 
     # Search with entity_types (SearchItemType) filter (use json format)
     response = await search_notes(
-        project=test_project.name, query="type", entity_types=["entity"], output_format="json"
+        project=test_project.name,
+        query="type",
+        search_type="text",
+        entity_types=["entity"],
+        output_format="json",
     )
 
     # Verify results - handle both success and error cases
@@ -265,6 +281,7 @@ async def test_search_with_date_filter(client, test_project):
     response = await search_notes(
         project=test_project.name,
         query="recent",
+        search_type="text",
         after_date=one_hour_ago.isoformat(),
         output_format="json",
     )