fix: resolve based_on schema/serialization issues in reflect API (#348)

* fix: add default values to OpenAPI schema for default_factory fields

This commit fixes the OpenAPI schema to include default values for fields
using default_factory, which improves schema accuracy and client generation.

Changes:
1. Added FieldWithDefault() helper to inject default values into OpenAPI schema
2. Updated 14 fields using default_factory to include defaults in schema:
   - ReflectBasedOn.{memories, mental_models, directives}
   - ReflectTrace.{tool_calls, llm_calls}
   - All tags fields
   - All trigger fields
   - All include fields

3. Regenerated OpenAPI spec with proper defaults

4. Added tests to verify API returns correct format with empty banks

Note: This fixes the schema but doesn't change the v0.3.0 -> v0.4.0 breaking
change where based_on went from list to object. Clients should handle both
formats for backward compatibility.

* fix: remove client imports from API test

The test was failing in CI because it imported the client library
which isn't installed in the API test environment.

Changed to test only API JSON response format, not client parsing.
This is more appropriate for an API test anyway.

* test: add client tests for ReflectResponse parsing

Added comprehensive tests in hindsight-clients/python/tests to verify:
- v0.4.0+ format with empty based_on object
- v0.4.0+ format with null based_on
- v0.4.0+ format with populated facts
- v0.3.0 format (list) correctly fails validation
- Missing based_on field handling

These tests document the v0.3.0 -> v0.4.0 breaking change where
based_on changed from list to object.

hindsight-api/hindsight_api/api/http.py

@@ -32,9 +32,44 @@ def _parse_metadata(metadata: Any) -> dict[str, Any]:
     return {}
 
 
+from typing import Callable
+
 from pydantic import BaseModel, ConfigDict, Field, field_validator
 
 from hindsight_api import MemoryEngine
+
+
+def FieldWithDefault(default_factory: Callable, **kwargs) -> Any:
+    """
+    Field wrapper that ensures default_factory values appear in OpenAPI schema.
+
+    Pydantic doesn't include default_factory in OpenAPI schemas, causing OpenAPI
+    Generator to make fields Optional with default=None instead of non-optional
+    with the correct default value.
+
+    This wrapper adds json_schema_extra to include the default in the schema.
+    """
+    # Determine the default value for the schema based on the factory
+    if default_factory is list:
+        schema_default = []
+    elif default_factory is dict:
+        schema_default = {}
+    else:
+        # For custom factories (like IncludeOptions), use empty dict as placeholder
+        schema_default = {}
+
+    # Add or merge json_schema_extra
+    json_extra = kwargs.pop("json_schema_extra", {})
+    if isinstance(json_extra, dict):
+        json_extra["default"] = schema_default
+    else:
+        # If json_schema_extra was a function, we can't merge easily
+        # Fall back to just setting default
+        json_extra = {"default": schema_default}
+
+    return Field(default_factory=default_factory, json_schema_extra=json_extra, **kwargs)
+
+
 from hindsight_api.engine.db_utils import acquire_with_retry
 from hindsight_api.engine.memory_engine import Budget, _get_tiktoken_encoding, fq_table
 from hindsight_api.engine.reflect.observations import Observation
@@ -103,8 +138,8 @@ class RecallRequest(BaseModel):
     query_timestamp: str | None = Field(
         default=None, description="ISO format date string (e.g., '2023-05-30T23:40:00')"
     )
-    include: IncludeOptions = Field(
+    include: IncludeOptions = FieldWithDefault(
-        default_factory=IncludeOptions,
+        IncludeOptions,
         description="Options for including additional data (entities are included by default)",
     )
     tags: list[str] | None = Field(
@@ -570,18 +605,16 @@ class ReflectLLMCall(BaseModel):
 class ReflectBasedOn(BaseModel):
     """Evidence the response is based on: memories, mental models, and directives."""
 
-    memories: list[ReflectFact] = Field(default_factory=list, description="Memory facts used to generate the response")
+    memories: list[ReflectFact] = FieldWithDefault(list, description="Memory facts used to generate the response")
-    mental_models: list[ReflectMentalModel] = Field(
+    mental_models: list[ReflectMentalModel] = FieldWithDefault(list, description="Mental models used during reflection")
-        default_factory=list, description="Mental models used during reflection"
+    directives: list[ReflectDirective] = FieldWithDefault(list, description="Directives applied during reflection")
-    )
-    directives: list[ReflectDirective] = Field(default_factory=list, description="Directives applied during reflection")
 
 
 class ReflectTrace(BaseModel):
     """Execution trace of LLM and tool calls during reflection."""
 
-    tool_calls: list[ReflectToolCall] = Field(default_factory=list, description="Tool calls made during reflection")
+    tool_calls: list[ReflectToolCall] = FieldWithDefault(list, description="Tool calls made during reflection")
-    llm_calls: list[ReflectLLMCall] = Field(default_factory=list, description="LLM calls made during reflection")
+    llm_calls: list[ReflectLLMCall] = FieldWithDefault(list, description="LLM calls made during reflection")
 
 
 class ReflectResponse(BaseModel):
@@ -942,7 +975,7 @@ class DocumentResponse(BaseModel):
     created_at: str
     updated_at: str
     memory_unit_count: int
-    tags: list[str] = Field(default_factory=list, description="Tags associated with this document")
+    tags: list[str] = FieldWithDefault(list, description="Tags associated with this document")
 
 
 class DeleteDocumentResponse(BaseModel):
@@ -1066,7 +1099,7 @@ class DirectiveResponse(BaseModel):
     content: str
     priority: int = 0
     is_active: bool = True
-    tags: list[str] = Field(default_factory=list)
+    tags: list[str] = FieldWithDefault(list)
     created_at: str | None = None
     updated_at: str | None = None
 
@@ -1084,7 +1117,7 @@ class CreateDirectiveRequest(BaseModel):
     content: str = Field(description="The directive text to inject into prompts")
     priority: int = Field(default=0, description="Higher priority directives are injected first")
     is_active: bool = Field(default=True, description="Whether this directive is active")
-    tags: list[str] = Field(default_factory=list, description="Tags for filtering")
+    tags: list[str] = FieldWithDefault(list, description="Tags for filtering")
 
 
 class UpdateDirectiveRequest(BaseModel):
@@ -1121,9 +1154,9 @@ class MentalModelResponse(BaseModel):
     content: str = Field(
         description="The mental model content as well-formatted markdown (auto-generated from reflect endpoint)"
     )
-    tags: list[str] = Field(default_factory=list)
+    tags: list[str] = FieldWithDefault(list)
     max_tokens: int = Field(default=2048)
-    trigger: MentalModelTrigger = Field(default_factory=MentalModelTrigger)
+    trigger: MentalModelTrigger = FieldWithDefault(MentalModelTrigger)
     last_refreshed_at: str | None = None
     created_at: str | None = None
     reflect_response: dict | None = Field(
@@ -1159,9 +1192,9 @@ class CreateMentalModelRequest(BaseModel):
     )
     name: str = Field(description="Human-readable name for the mental model")
     source_query: str = Field(description="The query to run to generate content")
-    tags: list[str] = Field(default_factory=list, description="Tags for scoped visibility")
+    tags: list[str] = FieldWithDefault(list, description="Tags for scoped visibility")
     max_tokens: int = Field(default=2048, ge=256, le=8192, description="Maximum tokens for generated content")
-    trigger: MentalModelTrigger = Field(default_factory=MentalModelTrigger, description="Trigger settings")
+    trigger: MentalModelTrigger = FieldWithDefault(MentalModelTrigger, description="Trigger settings")
 
 
 class CreateMentalModelResponse(BaseModel):

hindsight-api/tests/test_reflect_empty_based_on.py

@@ -0,0 +1,103 @@
+"""
+Test reflect endpoint with empty based_on (no memories scenario).
+
+This test verifies that the API returns the correct based_on format:
+- v0.3.0 (old): returned based_on as list []
+- v0.4.0+ (current): returns based_on as object {"memories": [], "mental_models": [], "directives": []}
+"""
+
+import pytest
+import pytest_asyncio
+import httpx
+from hindsight_api.api import create_app
+
+
+@pytest_asyncio.fixture
+async def api_client(memory):
+    """Create an async test client for the FastAPI app."""
+    app = create_app(memory, initialize_memory=False)
+    transport = httpx.ASGITransport(app=app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        yield client
+
+
+@pytest.mark.asyncio
+async def test_reflect_with_no_memories_empty_bank(api_client):
+    """Test reflect on an empty bank (no memories) with include.facts enabled."""
+    bank_id = "test_empty_bank"
+
+    # Reflect on empty bank with facts requested
+    response = await api_client.post(
+        f"/v1/default/banks/{bank_id}/reflect",
+        json={
+            "query": "What do you know about machine learning?",
+            "budget": "low",
+            "include": {
+                "facts": {}  # Request facts but bank is empty
+            }
+        }
+    )
+
+    assert response.status_code == 200
+    data = response.json()
+
+    # DEBUG: Print what the API actually returned
+    import json
+    print("\n" + "="*80)
+    print("API Response:")
+    print(json.dumps(data, indent=2))
+    print("="*80 + "\n")
+
+    # Verify response structure
+    assert "text" in data
+    assert "based_on" in data
+
+    # The API should return based_on as either:
+    # 1. null/None (if include.facts not set)
+    # 2. {"memories": [], "mental_models": [], "directives": []} (if include.facts set but empty)
+    # It should NEVER return based_on: []
+
+    based_on = data.get("based_on")
+    if based_on is not None:
+        assert isinstance(based_on, dict), f"based_on should be dict or null, got {type(based_on)}: {based_on}"
+        assert not isinstance(based_on, list), f"based_on should NEVER be a list! Got: {based_on}"
+        assert "memories" in based_on
+        assert "mental_models" in based_on
+        assert "directives" in based_on
+        # All should be empty lists
+        assert based_on["memories"] == []
+        assert based_on["mental_models"] == []
+        assert based_on["directives"] == []
+
+    # Verify the structure is parseable as proper types
+    assert isinstance(data["text"], str)
+    if based_on is not None:
+        # Verify it's the v0.4.0+ format (object with arrays)
+        assert isinstance(based_on["memories"], list)
+        assert isinstance(based_on["mental_models"], list)
+        assert isinstance(based_on["directives"], list)
+
+
+@pytest.mark.asyncio
+async def test_reflect_without_include_facts(api_client):
+    """Test reflect without requesting facts (based_on should be None)."""
+    bank_id = "test_no_facts"
+
+    response = await api_client.post(
+        f"/v1/default/banks/{bank_id}/reflect",
+        json={
+            "query": "Hello world",
+            "budget": "low"
+            # No include.facts
+        }
+    )
+
+    assert response.status_code == 200
+    data = response.json()
+
+    # When include.facts is not set, based_on should not be in response (or be null)
+    based_on = data.get("based_on")
+    assert based_on is None, f"based_on should be None when not requested, got {type(based_on)}: {based_on}"
+
+    # Verify structure
+    assert isinstance(data["text"], str)

hindsight-clients/python/tests/test_reflect_response_parsing.py

@@ -0,0 +1,125 @@
+"""
+Test ReflectResponse parsing for different API versions.
+
+This tests the client's ability to parse reflect responses from:
+- v0.3.0 API (based_on as list)
+- v0.4.0+ API (based_on as object)
+"""
+
+import pytest
+from hindsight_client_api.models.reflect_response import ReflectResponse
+from hindsight_client_api.models.reflect_based_on import ReflectBasedOn
+
+
+def test_parse_v4_format_with_empty_based_on():
+    """Test parsing v0.4.0+ format with empty based_on object."""
+    response_data = {
+        "text": "I don't have any information about that.",
+        "based_on": {
+            "memories": [],
+            "mental_models": [],
+            "directives": []
+        }
+    }
+
+    response = ReflectResponse.from_dict(response_data)
+    assert response is not None
+    assert response.text == "I don't have any information about that."
+    assert response.based_on is not None
+    assert isinstance(response.based_on, ReflectBasedOn)
+    assert response.based_on.memories == []
+    assert response.based_on.mental_models == []
+    assert response.based_on.directives == []
+
+
+def test_parse_v4_format_with_null_based_on():
+    """Test parsing v0.4.0+ format with null based_on (include.facts not set)."""
+    response_data = {
+        "text": "Hello!",
+        "based_on": None
+    }
+
+    response = ReflectResponse.from_dict(response_data)
+    assert response is not None
+    assert response.text == "Hello!"
+    assert response.based_on is None
+
+
+def test_parse_v4_format_with_populated_based_on():
+    """Test parsing v0.4.0+ format with actual facts."""
+    response_data = {
+        "text": "Based on my knowledge, AI is transformative.",
+        "based_on": {
+            "memories": [
+                {
+                    "id": "mem-123",
+                    "text": "AI is used in healthcare",
+                    "type": "world",
+                    "context": None,
+                    "occurred_start": None,
+                    "occurred_end": None
+                }
+            ],
+            "mental_models": [
+                {
+                    "id": "mm-456",
+                    "text": "AI transforms industries",
+                    "context": "technology trends"
+                }
+            ],
+            "directives": [
+                {
+                    "id": "dir-789",
+                    "name": "Be concise",
+                    "content": "Keep responses brief"
+                }
+            ]
+        }
+    }
+
+    response = ReflectResponse.from_dict(response_data)
+    assert response is not None
+    assert response.text == "Based on my knowledge, AI is transformative."
+    assert response.based_on is not None
+    assert len(response.based_on.memories) == 1
+    assert response.based_on.memories[0].id == "mem-123"
+    assert len(response.based_on.mental_models) == 1
+    assert response.based_on.mental_models[0].id == "mm-456"
+    assert len(response.based_on.directives) == 1
+    assert response.based_on.directives[0].id == "dir-789"
+
+
+def test_parse_v3_format_with_empty_list_fails():
+    """
+    Test that v0.3.0 format (based_on as list) fails validation.
+
+    This is a BREAKING CHANGE from v0.3.0 to v0.4.0.
+    Clients using v0.4.x SDK cannot parse v0.3.0 API responses.
+
+    Users must either:
+    - Upgrade API to v0.4.0+
+    - Use v0.3.0 client with v0.3.0 API
+    """
+    response_data = {
+        "text": "No information available.",
+        "based_on": []  # v0.3.0 format - incompatible with v0.4.0+ client
+    }
+
+    with pytest.raises(Exception) as exc_info:
+        ReflectResponse.from_dict(response_data)
+
+    # Should fail with validation error
+    assert "ValidationError" in str(type(exc_info.value).__name__) or "validation" in str(exc_info.value).lower()
+
+
+def test_parse_missing_based_on_field():
+    """Test parsing response when based_on field is omitted entirely."""
+    response_data = {
+        "text": "Hello!"
+        # based_on field not present
+    }
+
+    response = ReflectResponse.from_dict(response_data)
+    assert response is not None
+    assert response.text == "Hello!"
+    assert response.based_on is None

hindsight-docs/static/openapi.json

@@ -3560,7 +3560,8 @@
             },
             "type": "array",
             "title": "Tags",
-            "description": "Tags for filtering"
+            "description": "Tags for filtering",
+            "default": []
           }
         },
         "type": "object",
@@ -3601,7 +3602,8 @@
             },
             "type": "array",
             "title": "Tags",
-            "description": "Tags for scoped visibility"
+            "description": "Tags for scoped visibility",
+            "default": []
           },
           "max_tokens": {
             "type": "integer",
@@ -3613,7 +3615,8 @@
           },
           "trigger": {
             "$ref": "#/components/schemas/MentalModelTrigger",
-            "description": "Trigger settings"
+            "description": "Trigger settings",
+            "default": {}
           }
         },
         "type": "object",
@@ -3789,7 +3792,8 @@
               "type": "string"
             },
             "type": "array",
-            "title": "Tags"
+            "title": "Tags",
+            "default": []
           },
           "created_at": {
             "anyOf": [
@@ -3905,7 +3909,8 @@
             },
             "type": "array",
             "title": "Tags",
-            "description": "Tags associated with this document"
+            "description": "Tags associated with this document",
+            "default": []
           }
         },
         "type": "object",
@@ -4686,7 +4691,8 @@
               "type": "string"
             },
             "type": "array",
-            "title": "Tags"
+            "title": "Tags",
+            "default": []
           },
           "max_tokens": {
             "type": "integer",
@@ -4694,7 +4700,8 @@
             "default": 2048
           },
           "trigger": {
-            "$ref": "#/components/schemas/MentalModelTrigger"
+            "$ref": "#/components/schemas/MentalModelTrigger",
+            "default": {}
           },
           "last_refreshed_at": {
             "anyOf": [
@@ -5008,7 +5015,8 @@
           },
           "include": {
             "$ref": "#/components/schemas/IncludeOptions",
-            "description": "Options for including additional data (entities are included by default)"
+            "description": "Options for including additional data (entities are included by default)",
+            "default": {}
           },
           "tags": {
             "anyOf": [
@@ -5333,7 +5341,8 @@
             },
             "type": "array",
             "title": "Memories",
-            "description": "Memory facts used to generate the response"
+            "description": "Memory facts used to generate the response",
+            "default": []
           },
           "mental_models": {
             "items": {
@@ -5341,7 +5350,8 @@
             },
             "type": "array",
             "title": "Mental Models",
-            "description": "Mental models used during reflection"
+            "description": "Mental models used during reflection",
+            "default": []
           },
           "directives": {
             "items": {
@@ -5349,7 +5359,8 @@
             },
             "type": "array",
             "title": "Directives",
-            "description": "Directives applied during reflection"
+            "description": "Directives applied during reflection",
+            "default": []
           }
         },
         "type": "object",
@@ -5825,7 +5836,8 @@
             },
             "type": "array",
             "title": "Tool Calls",
-            "description": "Tool calls made during reflection"
+            "description": "Tool calls made during reflection",
+            "default": []
           },
           "llm_calls": {
             "items": {
@@ -5833,7 +5845,8 @@
             },
             "type": "array",
             "title": "Llm Calls",
-            "description": "LLM calls made during reflection"
+            "description": "LLM calls made during reflection",
+            "default": []
           }
         },
         "type": "object",