From 59134affa44d514cc92ffca8a7013cc60c780346 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Sun, 1 Mar 2026 09:41:22 -0600 Subject: [PATCH] fix: read schema definitions from file instead of stale database metadata (#635) Signed-off-by: Drew Cain Signed-off-by: phernandez Co-authored-by: Claude Opus 4.6 Co-authored-by: phernandez --- .../api/v2/routers/schema_router.py | 71 ++++- tests/api/v2/test_schema_router.py | 271 ++++++++++++++++++ 2 files changed, 334 insertions(+), 8 deletions(-) diff --git a/src/basic_memory/api/v2/routers/schema_router.py b/src/basic_memory/api/v2/routers/schema_router.py index 4e98b851..92a24183 100644 --- a/src/basic_memory/api/v2/routers/schema_router.py +++ b/src/basic_memory/api/v2/routers/schema_router.py @@ -10,9 +10,15 @@ Flow: Entity loaded with eager observations/relations -> convert to tuples -> co from pathlib import Path as FilePath +import frontmatter from fastapi import APIRouter, Path, Query +from loguru import logger -from basic_memory.deps import EntityRepositoryV2ExternalDep, LinkResolverV2ExternalDep +from basic_memory.deps import ( + EntityRepositoryV2ExternalDep, + FileServiceV2ExternalDep, + LinkResolverV2ExternalDep, +) from basic_memory.models.knowledge import Entity from basic_memory.schemas.schema import ( ValidationReport, @@ -67,11 +73,54 @@ def _entity_to_note_data(entity: Entity) -> NoteData: def _entity_frontmatter(entity: Entity) -> dict: - """Build a frontmatter dict from an entity for schema resolution.""" - frontmatter = dict(entity.entity_metadata) if entity.entity_metadata else {} + """Build a frontmatter dict from an entity's database metadata. + + Used for the notes being validated — their type and schema ref are + unlikely to change between syncs. + """ + fm = dict(entity.entity_metadata) if entity.entity_metadata else {} if entity.note_type: - frontmatter.setdefault("type", entity.note_type) - return frontmatter + fm.setdefault("type", entity.note_type) + return fm + + +async def _schema_frontmatter_from_file( + file_service: FileServiceV2ExternalDep, + entity: Entity, +) -> dict: + """Read a schema entity's frontmatter directly from its file. + + Schema definitions (field declarations, validation mode) are the source + of truth for validation. Reading from the file ensures schema-validate + always uses the latest settings, even when the file watcher hasn't + synced changes to entity_metadata in the database. + """ + try: + content = await file_service.read_file_content(entity.file_path) + post = frontmatter.loads(content) + metadata = dict(post.metadata) + + # Trigger: file is mid-edit and missing required schema fields + # Why: parse_schema_note() raises ValueError for missing entity/schema, + # which would turn validation into a 500 response + # Outcome: fall back to last-known-good database metadata + if not metadata.get("entity") or not isinstance(metadata.get("schema"), dict): + logger.warning( + "Schema file has incomplete frontmatter, falling back to database metadata", + file_path=entity.file_path, + ) + return _entity_frontmatter(entity) + + return metadata + except Exception: + # Trigger: file is missing, unreadable, or has malformed frontmatter + # Why: fall back to database metadata rather than failing validation entirely + # Outcome: behaves like before this change — uses potentially stale data + logger.warning( + "Failed to read schema file, falling back to database metadata", + file_path=entity.file_path, + ) + return _entity_frontmatter(entity) # --- Validation --- @@ -80,6 +129,7 @@ def _entity_frontmatter(entity: Entity) -> dict: @router.post("/schema/validate", response_model=ValidationReport) async def validate_schema( entity_repository: EntityRepositoryV2ExternalDep, + file_service: FileServiceV2ExternalDep, link_resolver: LinkResolverV2ExternalDep, project_id: str = Path(..., description="Project external UUID"), note_type: str | None = Query(None, description="Note type to validate"), @@ -89,6 +139,10 @@ async def validate_schema( Validates a specific note (by identifier) or all notes of a given type. Returns warnings/errors based on the schema's validation mode. + + Schema definitions are read directly from their files to ensure the + latest settings (validation mode, field declarations) are always used, + even when file changes haven't been synced to the database yet. """ results: list[NoteValidationResponse] = [] @@ -109,7 +163,7 @@ async def validate_schema( query, allow_reference_match=isinstance(schema_ref, str) and query == schema_ref, ) - return [_entity_frontmatter(e) for e in entities] + return [await _schema_frontmatter_from_file(file_service, e) for e in entities] schema_def = await resolve_schema(frontmatter, search_fn) if schema_def: @@ -145,7 +199,7 @@ async def validate_schema( query, allow_reference_match=isinstance(schema_ref, str) and query == schema_ref, ) - return [_entity_frontmatter(e) for e in entities] + return [await _schema_frontmatter_from_file(file_service, e) for e in entities] schema_def = await resolve_schema(frontmatter, search_fn) if schema_def: @@ -219,6 +273,7 @@ async def infer_schema_endpoint( @router.get("/schema/diff/{note_type}", response_model=DriftReport) async def diff_schema_endpoint( entity_repository: EntityRepositoryV2ExternalDep, + file_service: FileServiceV2ExternalDep, note_type: str = Path(..., description="Note type to check for drift"), project_id: str = Path(..., description="Project external UUID"), ): @@ -231,7 +286,7 @@ async def diff_schema_endpoint( async def search_fn(query: str) -> list[dict]: entities = await _find_schema_entities(entity_repository, query) - return [_entity_frontmatter(e) for e in entities] + return [await _schema_frontmatter_from_file(file_service, e) for e in entities] # Resolve schema by note type schema_frontmatter = {"type": note_type} diff --git a/tests/api/v2/test_schema_router.py b/tests/api/v2/test_schema_router.py index da4ca980..cf96b098 100644 --- a/tests/api/v2/test_schema_router.py +++ b/tests/api/v2/test_schema_router.py @@ -7,6 +7,7 @@ Note: EntityType uses BeforeValidator(to_snake_case) so "Person" becomes "person in the database. All query params must use the stored (snake_case) form. """ +from pathlib import Path from textwrap import dedent import pytest @@ -14,6 +15,7 @@ from httpx import AsyncClient from basic_memory.models import Project from basic_memory.schemas.base import Entity as EntitySchema +from basic_memory.services.file_service import FileService # --- Helpers --- @@ -624,3 +626,272 @@ async def test_diff_with_schema_note( assert isinstance(data["new_fields"], list) assert isinstance(data["dropped_fields"], list) assert isinstance(data["cardinality_changes"], list) + + +# --- File-based schema frontmatter tests --- + + +@pytest.mark.asyncio +async def test_validate_reads_schema_from_file_not_database( + client: AsyncClient, + test_project: Project, + v2_project_url: str, + entity_service, + search_service, + file_service: FileService, +): + """Validate uses schema frontmatter from the file, not stale database metadata. + + Simulates the core bug from #634: user edits a schema file to change + validation mode from 'warn' to 'strict', but the file watcher hasn't + synced. The database still has 'warn', but validation should use 'strict' + from the file. + """ + # Create schema entity — DB gets validation=warn + schema_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="Editable Schema", + directory="schemas", + note_type="schema", + entity_metadata={ + "entity": "editable_type", + "schema": {"name": "string", "role": "string"}, + "settings": {"validation": "warn"}, + }, + content=dedent("""\ + ## Observations + - [note] Schema that will be edited on disk + """), + ) + ) + await search_service.index_entity(schema_entity) + + # Overwrite the file on disk with validation=strict + file_path = Path(file_service.base_path) / schema_entity.file_path + file_path.write_text(dedent("""\ + --- + title: Editable Schema + permalink: schemas/editable-schema + type: schema + entity: editable_type + schema: + name: string + role: string + settings: + validation: strict + --- + + # Editable Schema + + ## Observations + - [note] Schema that will be edited on disk + """)) + + # Create a note missing "role" — strict mode should produce errors, not warnings + note_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="TestNote", + directory="notes", + note_type="editable_type", + content=dedent("""\ + ## Observations + - [name] Test Person + """), + ) + ) + await search_service.index_entity(note_entity) + + response = await client.post( + f"{v2_project_url}/schema/validate", + params={"identifier": note_entity.permalink}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["total_notes"] == 1 + result = data["results"][0] + # strict mode: missing required field is an error, not a warning + assert result["passed"] is False + assert any("role" in e for e in result["errors"]) + + +@pytest.mark.asyncio +async def test_validate_falls_back_to_db_on_incomplete_frontmatter( + client: AsyncClient, + test_project: Project, + v2_project_url: str, + entity_service, + search_service, + file_service: FileService, +): + """Validate falls back to database metadata when file has incomplete frontmatter. + + Simulates a mid-edit state where the user has removed the 'schema' key + from the file. The validator should use the last-known-good metadata + from the database rather than failing with a 500. + """ + schema_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="Incomplete Schema", + directory="schemas", + note_type="schema", + entity_metadata={ + "entity": "incomplete_type", + "schema": {"name": "string"}, + }, + content=dedent("""\ + ## Observations + - [note] Schema that will have incomplete frontmatter + """), + ) + ) + await search_service.index_entity(schema_entity) + + # Overwrite file with frontmatter missing the 'schema' key + file_path = Path(file_service.base_path) / schema_entity.file_path + file_path.write_text(dedent("""\ + --- + title: Incomplete Schema + permalink: schemas/incomplete-schema + type: schema + entity: incomplete_type + --- + + # Incomplete Schema + + ## Observations + - [note] Mid-edit state + """)) + + # Create a note to validate against this schema + note_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="FallbackNote", + directory="notes", + note_type="incomplete_type", + content=dedent("""\ + ## Observations + - [name] Test Fallback + """), + ) + ) + await search_service.index_entity(note_entity) + + response = await client.post( + f"{v2_project_url}/schema/validate", + params={"note_type": "incomplete_type"}, + ) + + # Should not 500 — falls back to DB metadata and validates successfully + assert response.status_code == 200 + data = response.json() + assert data["total_notes"] == 1 + result = data["results"][0] + assert result["passed"] is True + + +@pytest.mark.asyncio +async def test_validate_falls_back_to_db_on_missing_file( + client: AsyncClient, + test_project: Project, + v2_project_url: str, + entity_service, + search_service, + file_service: FileService, +): + """Validate falls back to database metadata when schema file is missing. + + Simulates a race condition where the file has been deleted but the + database still has the entity. The validator should use DB metadata + rather than failing entirely. + """ + schema_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="Missing File Schema", + directory="schemas", + note_type="schema", + entity_metadata={ + "entity": "missing_file_type", + "schema": {"name": "string"}, + }, + content=dedent("""\ + ## Observations + - [note] Schema whose file will be deleted + """), + ) + ) + await search_service.index_entity(schema_entity) + + # Delete the schema file from disk + file_path = Path(file_service.base_path) / schema_entity.file_path + file_path.unlink() + + # Create a note to validate + note_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="OrphanNote", + directory="notes", + note_type="missing_file_type", + content=dedent("""\ + ## Observations + - [name] Test Orphan + """), + ) + ) + await search_service.index_entity(note_entity) + + response = await client.post( + f"{v2_project_url}/schema/validate", + params={"note_type": "missing_file_type"}, + ) + + # Should not 500 — falls back to DB metadata and validates + assert response.status_code == 200 + data = response.json() + assert data["total_notes"] == 1 + result = data["results"][0] + assert result["passed"] is True + + +@pytest.mark.asyncio +async def test_diff_falls_back_to_db_on_missing_file( + client: AsyncClient, + test_project: Project, + v2_project_url: str, + entity_service, + search_service, + file_service: FileService, +): + """Diff endpoint falls back to DB metadata when schema file is missing.""" + schema_entity, _ = await entity_service.create_or_update_entity( + EntitySchema( + title="Diff Missing Schema", + directory="schemas", + note_type="schema", + entity_metadata={ + "entity": "diff_missing_type", + "schema": {"name": "string", "role": "string"}, + }, + content=dedent("""\ + ## Observations + - [note] Schema for diff fallback test + """), + ) + ) + await search_service.index_entity(schema_entity) + + # Delete the schema file + file_path = Path(file_service.base_path) / schema_entity.file_path + file_path.unlink() + + # Create person entities + await create_person_entities(entity_service, search_service) + + response = await client.get( + f"{v2_project_url}/schema/diff/diff_missing_type", + ) + + # Should not 500 — falls back to DB metadata for schema resolution + assert response.status_code == 200 + data = response.json() + assert data["note_type"] == "diff_missing_type"