diff --git a/src/basic_memory/schemas/response.py b/src/basic_memory/schemas/response.py index 166d4ea7..80a13f36 100644 --- a/src/basic_memory/schemas/response.py +++ b/src/basic_memory/schemas/response.py @@ -14,7 +14,7 @@ from datetime import datetime from typing import List, Optional, Dict -from pydantic import BaseModel, ConfigDict, Field, AliasPath, AliasChoices +from pydantic import BaseModel, ConfigDict, Field, model_validator from basic_memory.schemas.base import Relation, Permalink, EntityType, ContentType, Observation @@ -64,32 +64,89 @@ class RelationResponse(Relation, SQLAlchemyModel): permalink: Permalink - from_id: Permalink = Field( - # use the permalink from the associated Entity - # or the from_id value - validation_alias=AliasChoices( - AliasPath("from_entity", "permalink"), - "from_id", - ) - ) - to_id: Optional[Permalink] = Field( # pyright: ignore - # use the permalink from the associated Entity - # or the to_id value - validation_alias=AliasChoices( - AliasPath("to_entity", "permalink"), - "to_id", - ), - default=None, - ) - to_name: Optional[Permalink] = Field( - # use the permalink from the associated Entity - # or the to_id value - validation_alias=AliasChoices( - AliasPath("to_entity", "title"), - "to_name", - ), - default=None, - ) + # Override base Relation fields to allow Optional values + from_id: Optional[Permalink] = Field(default=None) # pyright: ignore[reportIncompatibleVariableOverride] + to_id: Optional[Permalink] = Field(default=None) # pyright: ignore[reportIncompatibleVariableOverride] + to_name: Optional[str] = Field(default=None) + + @model_validator(mode="before") + @classmethod + def resolve_entity_references(cls, data): + """Resolve from_id and to_id from joined entities, falling back to file_path. + + When loading from SQLAlchemy models, the from_entity and to_entity relationships + are joined. We extract the permalink from these entities, falling back to + file_path when permalink is None. + + We use file_path directly (not converted to permalink format) because if the + entity doesn't have a permalink, the system won't be able to find it by a + generated one anyway. Using the actual file_path preserves the real identifier. + """ + # Handle dict input (e.g., from API or tests) + if isinstance(data, dict): + from_entity = data.get("from_entity") + to_entity = data.get("to_entity") + + # Resolve from_id: prefer permalink, fall back to file_path + if from_entity and isinstance(from_entity, dict): + permalink = from_entity.get("permalink") + if permalink: + data["from_id"] = permalink + elif from_entity.get("file_path"): + data["from_id"] = from_entity["file_path"] + + # Resolve to_id: prefer permalink, fall back to file_path + if to_entity and isinstance(to_entity, dict): + permalink = to_entity.get("permalink") + if permalink: + data["to_id"] = permalink + elif to_entity.get("file_path"): + data["to_id"] = to_entity["file_path"] + + # Also resolve to_name from entity title + if to_entity.get("title") and not data.get("to_name"): + data["to_name"] = to_entity["title"] + + return data + + # Handle SQLAlchemy model input (from_attributes=True) + # Access attributes directly from the ORM model + from_entity = getattr(data, "from_entity", None) + to_entity = getattr(data, "to_entity", None) + + # Build a dict from the model's attributes + result = {} + + # Copy base fields + for field in ["permalink", "relation_type", "context", "to_name"]: + if hasattr(data, field): + result[field] = getattr(data, field) + + # Resolve from_id: prefer permalink, fall back to file_path + if from_entity: + permalink = getattr(from_entity, "permalink", None) + file_path = getattr(from_entity, "file_path", None) + if permalink: + result["from_id"] = permalink + elif file_path: + result["from_id"] = file_path + + # Resolve to_id: prefer permalink, fall back to file_path + if to_entity: + permalink = getattr(to_entity, "permalink", None) + file_path = getattr(to_entity, "file_path", None) + if permalink: + result["to_id"] = permalink + elif file_path: + result["to_id"] = file_path + + # Also resolve to_name from entity title if not set + if not result.get("to_name"): + title = getattr(to_entity, "title", None) + if title: + result["to_name"] = title + + return result class EntityResponse(SQLAlchemyModel): diff --git a/tests/schemas/test_schemas.py b/tests/schemas/test_schemas.py index 4e981abf..baf43fc6 100644 --- a/tests/schemas/test_schemas.py +++ b/tests/schemas/test_schemas.py @@ -91,6 +91,45 @@ def test_relation_response(): assert relation.context is None +def test_relation_response_with_null_permalink(): + """Test RelationResponse handles null permalinks by falling back to file_path (fixes issue #483). + + When entities are imported from environments without permalinks enabled, + the from_entity.permalink and to_entity.permalink can be None. + In this case, we fall back to file_path to ensure the API always returns + a usable identifier for the related entities. + + We use file_path directly (not converted to permalink format) because if the + entity doesn't have a permalink, the system won't find it by a generated one. + """ + data = { + "permalink": "test/relation/123", + "relation_type": "relates_to", + "from_entity": {"permalink": None, "file_path": "notes/source-note.md"}, + "to_entity": {"permalink": None, "file_path": "notes/target-note.md", "title": "Target Note"}, + } + relation = RelationResponse.model_validate(data) + # Falls back to file_path directly (not converted to permalink) + assert relation.from_id == "notes/source-note.md" + assert relation.to_id == "notes/target-note.md" + assert relation.to_name == "Target Note" + assert relation.relation_type == "relates_to" + + +def test_relation_response_with_permalink_preferred_over_file_path(): + """Test that permalink is preferred over file_path when both are available.""" + data = { + "permalink": "test/relation/123", + "relation_type": "links_to", + "from_entity": {"permalink": "from-permalink", "file_path": "notes/from-file.md"}, + "to_entity": {"permalink": "to-permalink", "file_path": "notes/to-file.md"}, + } + relation = RelationResponse.model_validate(data) + # Prefers permalink over file_path + assert relation.from_id == "from-permalink" + assert relation.to_id == "to-permalink" + + def test_entity_out_from_attributes(): """Test EntityOut creation from database model attributes.""" # Simulate database model attributes