Merge branch 'main' of github.com:basicmachines-co/basic-memory

Author: phernandez

src/basic_memory/api/v2/routers/project_router.py

@@ -11,7 +11,7 @@
 """
 
 import os
-from typing import Optional
+from typing import Literal, Optional
 
 from fastapi import APIRouter, HTTPException, Body, Query, Path
 from loguru import logger
@@ -25,6 +25,8 @@
     ProjectExternalIdPathDep,
 )
 from basic_memory.schemas import SyncReportResponse
+from basic_memory.models import Project
+from basic_memory.repository.project_repository import ProjectRepository
 from basic_memory.schemas.project_info import (
     ProjectItem,
     ProjectList,
@@ -36,6 +38,71 @@
 from basic_memory.utils import normalize_project_path, generate_permalink
 
 router = APIRouter(prefix="/projects", tags=["project_management-v2"])
+ProjectResolveMethod = Literal["external_id", "name", "permalink"]
+
+
+def _split_qualified_project_identifier(identifier: str) -> tuple[str | None, str]:
+    """Split ``<workspace>/<project>`` identifiers while preserving plain project names."""
+    cleaned = identifier.strip()
+    if "/" not in cleaned:
+        return None, cleaned
+
+    workspace_identifier, project_identifier = cleaned.split("/", 1)
+    if not workspace_identifier or not project_identifier:
+        return None, cleaned
+    return workspace_identifier, project_identifier
+
+
+async def _resolve_project_identifier_candidate(
+    project_repository: ProjectRepository,
+    identifier: str,
+) -> tuple[Project | None, ProjectResolveMethod]:
+    """Resolve one project identifier candidate and report the matching method."""
+    identifier_permalink = generate_permalink(identifier)
+
+    project = await project_repository.get_by_external_id(identifier)
+    if project:
+        return project, "external_id"
+
+    project = await project_repository.get_by_permalink(identifier_permalink)
+    if project:
+        return project, "permalink"
+
+    project = await project_repository.get_by_name_case_insensitive(identifier)
+    if project:
+        return project, "name"  # pragma: no cover
+
+    return None, "name"
+
+
+async def _resolve_project_identifier(
+    project_repository: ProjectRepository,
+    identifier: str,
+) -> tuple[Project | None, ProjectResolveMethod]:
+    """Resolve exact identifiers first, then accepted workspace-qualified forms."""
+    project, resolution_method = await _resolve_project_identifier_candidate(
+        project_repository,
+        identifier,
+    )
+    if project:
+        return project, resolution_method
+
+    workspace_identifier, project_identifier = _split_qualified_project_identifier(identifier)
+    if workspace_identifier is None:
+        return None, resolution_method
+
+    # Trigger: an MCP disambiguation error suggested ``workspace/project``.
+    # Why: request routing already selected the workspace/tenant; this endpoint
+    #   only needs the project segment to validate the active project.
+    # Outcome: models can follow the hint verbatim instead of looping on a 404.
+    project, resolution_method = await _resolve_project_identifier_candidate(
+        project_repository,
+        project_identifier,
+    )
+    if project:
+        return project, resolution_method
+
+    return None, resolution_method
 
 
 @router.get("/", response_model=ProjectList)
@@ -247,28 +314,10 @@ async def resolve_project_identifier(
     """
     logger.info(f"API v2 request: resolve_project_identifier for '{data.identifier}'")
 
-    # Generate permalink for comparison
-    identifier_permalink = generate_permalink(data.identifier)
-
-    resolution_method = "name"
-    project = None
-
-    # Try external_id first (UUID format)
-    project = await project_repository.get_by_external_id(data.identifier)
-    if project:
-        resolution_method = "external_id"
-
-    # If not found by external_id, try by permalink (exact match)
-    if not project:
-        project = await project_repository.get_by_permalink(identifier_permalink)
-        if project:
-            resolution_method = "permalink"
-
-    # If not found by permalink, try case-insensitive name search
-    if not project:
-        project = await project_repository.get_by_name_case_insensitive(data.identifier)
-        if project:
-            resolution_method = "name"  # pragma: no cover
+    project, resolution_method = await _resolve_project_identifier(
+        project_repository,
+        data.identifier,
+    )
 
     if not project:
         raise HTTPException(status_code=404, detail=f"Project not found: '{data.identifier}'")

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/api/v2/test_project_router.py

@@ -321,6 +321,21 @@ async def test_resolve_project_by_permalink(
     assert resolved.resolution_method in ["name", "permalink"]
 
 
+@pytest.mark.asyncio
+async def test_resolve_project_by_workspace_qualified_permalink(
+    client: AsyncClient, test_project: Project, v2_projects_url
+):
+    """Resolve the workspace/project form shown by MCP disambiguation errors."""
+    resolve_data = {"identifier": f"personal/{test_project.name}"}
+    response = await client.post(f"{v2_projects_url}/resolve", json=resolve_data)
+
+    assert response.status_code == 200
+    resolved = ProjectResolveResponse.model_validate(response.json())
+    assert resolved.external_id == test_project.external_id
+    assert resolved.name == test_project.name
+    assert resolved.resolution_method == "permalink"
+
+
 @pytest.mark.asyncio
 async def test_resolve_project_by_id(client: AsyncClient, test_project: Project, v2_projects_url):
     """Test resolving a project by external_id string returns correct project external_id."""

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."""