diff --git a/hindsight-api/hindsight_api/api/http.py b/hindsight-api/hindsight_api/api/http.py index 6631d2ce..ec028fe2 100644 --- a/hindsight-api/hindsight_api/api/http.py +++ b/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( - default_factory=IncludeOptions, + include: IncludeOptions = FieldWithDefault( + 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") - mental_models: list[ReflectMentalModel] = Field( - default_factory=list, description="Mental models used during reflection" - ) - directives: list[ReflectDirective] = Field(default_factory=list, description="Directives applied during reflection") + memories: list[ReflectFact] = FieldWithDefault(list, description="Memory facts used to generate the response") + mental_models: list[ReflectMentalModel] = FieldWithDefault(list, description="Mental models used during reflection") + directives: list[ReflectDirective] = FieldWithDefault(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") - llm_calls: list[ReflectLLMCall] = Field(default_factory=list, description="LLM calls made during reflection") + tool_calls: list[ReflectToolCall] = FieldWithDefault(list, description="Tool 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): diff --git a/hindsight-api/tests/test_reflect_empty_based_on.py b/hindsight-api/tests/test_reflect_empty_based_on.py new file mode 100644 index 00000000..a7339f00 --- /dev/null +++ b/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) diff --git a/hindsight-clients/python/tests/test_reflect_response_parsing.py b/hindsight-clients/python/tests/test_reflect_response_parsing.py new file mode 100644 index 00000000..c4ce6045 --- /dev/null +++ b/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 diff --git a/hindsight-docs/static/openapi.json b/hindsight-docs/static/openapi.json index 9d4b2a44..513304a6 100644 --- a/hindsight-docs/static/openapi.json +++ b/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",