From 9cfdd464a9ad425c7f4fba4ea6688a82e90e6afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Boschi?= Date: Thu, 2 Apr 2026 12:20:37 +0200 Subject: [PATCH] fix(retain): preserve normalized experience fact types (#848) * fix(retain): preserve normalized experience fact types and remove deprecated opinion type The ExtractedFactType conversion was re-checking for raw "assistant" fact_type after the parsing layer had already normalized it to "experience". Since fact_from_llm.fact_type was always "experience" (never "assistant"), the ternary always fell through to "world", silently losing experience classification. Also removes the deprecated "opinion" fact type from internal extraction models, database constraints/indexes (via migration), and dead code paths. The public API surface (descriptions, response models, backwards-compat filter) is unchanged. * refactor(retain): drop unused confidence_score column The confidence_score column was only ever non-null for opinion facts (which are now removed). It was always written as NULL and never read back from the database. Remove it from: - DB model and migration (DROP COLUMN) - INSERT queries in fact_storage.py - retain_async/retain_batch_async parameters - RetainContext/RetainResult extension models - RetainBatch dataclass --- .../g2h3i4j5k6l7_remove_opinion_fact_type.py | 83 +++++++++++++++++++ hindsight-api-slim/hindsight_api/api/http.py | 6 +- .../hindsight_api/engine/memory_engine.py | 24 ++---- .../hindsight_api/engine/response_models.py | 1 - .../engine/retain/fact_extraction.py | 6 +- .../engine/retain/fact_storage.py | 20 ++--- .../hindsight_api/engine/retain/types.py | 3 +- .../engine/search/graph_retrieval.py | 2 +- .../engine/search/think_utils.py | 6 +- .../hindsight_api/engine/search/trace.py | 6 +- .../hindsight_api/engine/search/tracer.py | 2 +- .../extensions/operation_validator.py | 3 - hindsight-api-slim/hindsight_api/models.py | 24 +----- .../tests/test_batch_api_integration.py | 2 +- .../test_experience_fact_type_passthrough.py | 59 +++++++++++++ hindsight-api-slim/tests/test_extensions.py | 4 - hindsight-api-slim/tests/test_llm_provider.py | 2 +- 17 files changed, 174 insertions(+), 79 deletions(-) create mode 100644 hindsight-api-slim/hindsight_api/alembic/versions/g2h3i4j5k6l7_remove_opinion_fact_type.py create mode 100644 hindsight-api-slim/tests/test_experience_fact_type_passthrough.py diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/g2h3i4j5k6l7_remove_opinion_fact_type.py b/hindsight-api-slim/hindsight_api/alembic/versions/g2h3i4j5k6l7_remove_opinion_fact_type.py new file mode 100644 index 00000000..a1f39689 --- /dev/null +++ b/hindsight-api-slim/hindsight_api/alembic/versions/g2h3i4j5k6l7_remove_opinion_fact_type.py @@ -0,0 +1,83 @@ +"""remove_opinion_fact_type + +Revision ID: g2h3i4j5k6l7 +Revises: f1a2b3c4d5e6 +Create Date: 2026-04-02 + +Remove the deprecated 'opinion' fact type: drop opinion-specific indexes, +update CHECK constraints, delete any remaining opinion rows, and drop the +confidence_score column (was only used for opinions, always NULL otherwise). +""" + +from collections.abc import Sequence + +from alembic import context, op + +# revision identifiers, used by Alembic. +revision: str = "g2h3i4j5k6l7" +down_revision: str | Sequence[str] | None = "f1a2b3c4d5e6" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def _get_schema_prefix() -> str: + """Get schema prefix for table names (required for multi-tenant support).""" + schema = context.config.get_main_option("target_schema") + return f'"{schema}".' if schema else "" + + +def upgrade() -> None: + schema = _get_schema_prefix() + + # 1. Delete any remaining opinion rows + op.execute(f"DELETE FROM {schema}memory_units WHERE fact_type = 'opinion'") + + # 2. Drop opinion-specific indexes + op.execute(f"DROP INDEX IF EXISTS {schema}idx_memory_units_opinion_confidence") + op.execute(f"DROP INDEX IF EXISTS {schema}idx_memory_units_opinion_date") + + # 3. Drop confidence_score constraints and column (only used for opinions, always NULL otherwise) + op.execute(f"ALTER TABLE {schema}memory_units DROP CONSTRAINT IF EXISTS confidence_score_fact_type_check") + op.execute(f"ALTER TABLE {schema}memory_units DROP CONSTRAINT IF EXISTS memory_units_confidence_score_check") + op.execute(f"ALTER TABLE {schema}memory_units DROP COLUMN IF EXISTS confidence_score") + + # 4. Replace fact_type CHECK constraint + op.execute(f"ALTER TABLE {schema}memory_units DROP CONSTRAINT IF EXISTS memory_units_fact_type_check") + op.execute( + f"ALTER TABLE {schema}memory_units ADD CONSTRAINT memory_units_fact_type_check " + f"CHECK (fact_type IN ('world', 'experience', 'observation'))" + ) + + +def downgrade() -> None: + schema = _get_schema_prefix() + + # Restore confidence_score column + op.execute(f"ALTER TABLE {schema}memory_units ADD COLUMN IF NOT EXISTS confidence_score float") + op.execute( + f"ALTER TABLE {schema}memory_units ADD CONSTRAINT memory_units_confidence_score_check " + f"CHECK (confidence_score IS NULL OR (confidence_score >= 0.0 AND confidence_score <= 1.0))" + ) + op.execute( + f"ALTER TABLE {schema}memory_units ADD CONSTRAINT confidence_score_fact_type_check " + f"CHECK ((fact_type = 'opinion' AND confidence_score IS NOT NULL) OR " + f"(fact_type = 'observation') OR " + f"(fact_type NOT IN ('opinion', 'observation') AND confidence_score IS NULL))" + ) + + # Restore original fact_type CHECK constraint (with opinion) + op.execute(f"ALTER TABLE {schema}memory_units DROP CONSTRAINT IF EXISTS memory_units_fact_type_check") + op.execute( + f"ALTER TABLE {schema}memory_units ADD CONSTRAINT memory_units_fact_type_check " + f"CHECK (fact_type IN ('world', 'experience', 'opinion', 'observation'))" + ) + + # Recreate opinion indexes + op.execute( + f"CREATE INDEX idx_memory_units_opinion_confidence ON {schema}memory_units " + f"(bank_id, confidence_score DESC) WHERE fact_type = 'opinion'" + ) + op.execute( + f"CREATE INDEX idx_memory_units_opinion_date ON {schema}memory_units " + f"(bank_id, event_date DESC) WHERE fact_type = 'opinion'" + ) diff --git a/hindsight-api-slim/hindsight_api/api/http.py b/hindsight-api-slim/hindsight_api/api/http.py index e9463535..2c1739f3 100644 --- a/hindsight-api-slim/hindsight_api/api/http.py +++ b/hindsight-api-slim/hindsight_api/api/http.py @@ -748,7 +748,7 @@ class ReflectFact(BaseModel): text: str = Field( description="Fact text. When type='observation', this contains markdown-formatted consolidated knowledge" ) - type: str | None = None # fact type: world, experience, observation + type: str | None = None # fact type: world, experience, opinion, observation context: str | None = None occurred_start: str | None = None occurred_end: str | None = None @@ -5109,7 +5109,9 @@ def _register_routes(app: FastAPI): ): """Clear memories for a memory bank, optionally filtered by type.""" try: - await app.state.memory.delete_bank(bank_id, fact_type=type, delete_bank_profile=False, request_context=request_context) + await app.state.memory.delete_bank( + bank_id, fact_type=type, delete_bank_profile=False, request_context=request_context + ) return DeleteResponse(success=True) except OperationValidationError as e: diff --git a/hindsight-api-slim/hindsight_api/engine/memory_engine.py b/hindsight-api-slim/hindsight_api/engine/memory_engine.py index 56d6180f..146c61bf 100644 --- a/hindsight-api-slim/hindsight_api/engine/memory_engine.py +++ b/hindsight-api-slim/hindsight_api/engine/memory_engine.py @@ -270,7 +270,7 @@ class MemoryEngine(MemoryEngineInterface): This class provides: - Embedding generation for semantic search - Entity, temporal, and semantic link creation - - Think operations for formulating answers with opinions + - Think operations for formulating answers with observations - bank profile and disposition management """ @@ -2003,7 +2003,6 @@ class MemoryEngine(MemoryEngineInterface): event_date: datetime | None = None, document_id: str | None = None, fact_type_override: str | None = None, - confidence_score: float | None = None, *, request_context: "RequestContext", ) -> list[str]: @@ -2019,7 +2018,6 @@ class MemoryEngine(MemoryEngineInterface): event_date: When the event occurred (defaults to now) document_id: Optional document ID for tracking (always upserts if document already exists) fact_type_override: Override fact type ('world', 'experience') - confidence_score: Confidence score (0.0 to 1.0) request_context: Request context for authentication. Returns: @@ -2038,7 +2036,6 @@ class MemoryEngine(MemoryEngineInterface): contents=[content_dict], request_context=request_context, fact_type_override=fact_type_override, - confidence_score=confidence_score, ) # Return the first (and only) list of unit IDs @@ -2052,7 +2049,6 @@ class MemoryEngine(MemoryEngineInterface): request_context: "RequestContext", document_id: str | None = None, fact_type_override: str | None = None, - confidence_score: float | None = None, document_tags: list[str] | None = None, return_usage: bool = False, operation_id: str | None = None, @@ -2078,7 +2074,6 @@ class MemoryEngine(MemoryEngineInterface): document_id: **DEPRECATED** - Use "document_id" key in each content dict instead. Applies the same document_id to ALL content items that don't specify their own. fact_type_override: Override fact type for all facts ('world', 'experience') - confidence_score: Confidence score (0.0 to 1.0) return_usage: If True, returns tuple of (unit_ids, TokenUsage). Default False for backward compatibility. Returns: @@ -2128,7 +2123,6 @@ class MemoryEngine(MemoryEngineInterface): request_context=request_context, document_id=document_id, fact_type_override=fact_type_override, - confidence_score=confidence_score, ) result = await self._validate_operation(self._operation_validator.validate_retain(ctx)) if result and result.contents is not None: @@ -2254,7 +2248,6 @@ class MemoryEngine(MemoryEngineInterface): request_context=request_context, document_id=document_id, fact_type_override=fact_type_override, - confidence_score=confidence_score, unit_ids=result, success=True, error=None, @@ -2373,7 +2366,7 @@ class MemoryEngine(MemoryEngineInterface): Args: bank_id: bank ID to recall for query: Recall query - fact_type: Required filter for fact type ('world', 'experience', or 'opinion') + fact_type: Required filter for fact type ('world' or 'experience') budget: Budget level for graph traversal (low=100, mid=300, high=600 units) max_tokens: Maximum tokens to return (counts only 'text' field, default 4096) enable_trace: If True, returns detailed trace object @@ -2467,8 +2460,10 @@ class MemoryEngine(MemoryEngineInterface): if fact_type is None: fact_type = list(VALID_RECALL_FACT_TYPES) - # Filter out 'opinion' early (deprecated, silently ignore) + # Filter out 'opinion' (removed fact type, silently ignore for backwards compat) fact_type = [ft for ft in fact_type if ft != "opinion"] + if not fact_type: + return RecallResultModel(results=[], entities={}, chunks={}) # Validate fact types invalid_types = set(fact_type) - VALID_RECALL_FACT_TYPES @@ -2477,9 +2472,6 @@ class MemoryEngine(MemoryEngineInterface): f"Invalid fact type(s): {', '.join(sorted(invalid_types))}. " f"Must be one of: {', '.join(sorted(VALID_RECALL_FACT_TYPES))}" ) - if not fact_type: - # All requested types were opinions - return empty result - return RecallResultModel(results=[], entities={}, chunks={}) # Validate operation if validator is configured if self._operation_validator: @@ -3854,7 +3846,7 @@ class MemoryEngine(MemoryEngineInterface): Args: bank_id: bank ID to delete - fact_type: Optional fact type filter (world, experience, opinion). If provided, only deletes memories of that type. + fact_type: Optional fact type filter (world, experience). If provided, only deletes memories of that type. request_context: Request context for authentication. Returns: @@ -4173,7 +4165,7 @@ class MemoryEngine(MemoryEngineInterface): Args: bank_id: Filter by bank ID - fact_type: Filter by fact type (world, experience, opinion) + fact_type: Filter by fact type (world, experience) limit: Maximum number of items to return (default: 1000) q: Full-text search query (searches text and context fields) tags: Filter by tags @@ -4552,7 +4544,7 @@ class MemoryEngine(MemoryEngineInterface): Args: bank_id: Filter by bank ID - fact_type: Filter by fact type (world, experience, opinion) + fact_type: Filter by fact type (world, experience) search_query: Full-text search query (searches text and context fields) limit: Maximum number of results to return offset: Offset for pagination diff --git a/hindsight-api-slim/hindsight_api/engine/response_models.py b/hindsight-api-slim/hindsight_api/engine/response_models.py index 065bb4ff..bfd7a69c 100644 --- a/hindsight-api-slim/hindsight_api/engine/response_models.py +++ b/hindsight-api-slim/hindsight_api/engine/response_models.py @@ -10,7 +10,6 @@ from typing import Any from pydantic import BaseModel, ConfigDict, Field, field_validator -# Valid fact types for recall operations (excludes 'opinion' which is deprecated) VALID_RECALL_FACT_TYPES = frozenset(["world", "experience", "observation"]) diff --git a/hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py b/hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py index f4f7363e..e9e6ebaa 100644 --- a/hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py +++ b/hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py @@ -87,7 +87,7 @@ class Fact(BaseModel): # Required fields fact: str = Field(description="Combined fact text: what | when | where | who | why") - fact_type: Literal["world", "experience", "opinion"] = Field(description="Perspective: world/experience/opinion") + fact_type: Literal["world", "experience"] = Field(description="Perspective: world/experience") # Optional temporal fields occurred_start: str | None = None @@ -1967,7 +1967,7 @@ async def extract_facts_from_contents_batch_api( for fact_from_llm in chunk_facts: extracted_fact = ExtractedFactType( fact_text=fact_from_llm.fact, - fact_type="experience" if fact_from_llm.fact_type == "assistant" else "world", + fact_type=fact_from_llm.fact_type, entities=[e.text for e in (fact_from_llm.entities or [])], occurred_start=_parse_datetime(fact_from_llm.occurred_start) if fact_from_llm.occurred_start else None, occurred_end=_parse_datetime(fact_from_llm.occurred_end) if fact_from_llm.occurred_end else None, @@ -2152,7 +2152,7 @@ async def extract_facts_from_contents( # mentioned_at is always the event_date (when the conversation/document occurred) extracted_fact = ExtractedFactType( fact_text=fact_from_llm.fact, - fact_type="experience" if fact_from_llm.fact_type == "assistant" else "world", + fact_type=fact_from_llm.fact_type, entities=[e.text for e in (fact_from_llm.entities or [])], # occurred_start/end: from LLM only, leave None if not provided occurred_start=_parse_datetime(fact_from_llm.occurred_start) diff --git a/hindsight-api-slim/hindsight_api/engine/retain/fact_storage.py b/hindsight-api-slim/hindsight_api/engine/retain/fact_storage.py index beb127f0..4e9d78b5 100644 --- a/hindsight-api-slim/hindsight_api/engine/retain/fact_storage.py +++ b/hindsight-api-slim/hindsight_api/engine/retain/fact_storage.py @@ -44,7 +44,6 @@ async def insert_facts_batch( mentioned_ats = [] contexts = [] fact_types = [] - confidence_scores = [] metadata_jsons = [] chunk_ids = [] document_ids = [] @@ -64,8 +63,6 @@ async def insert_facts_batch( mentioned_ats.append(fact.mentioned_at) contexts.append(_sanitize_text(fact.context)) fact_types.append(fact.fact_type) - # confidence_score is only for opinion facts - confidence_scores.append(1.0 if fact.fact_type == "opinion" else None) metadata_jsons.append(json.dumps(fact.metadata)) chunk_ids.append(fact.chunk_id) # Use per-fact document_id if available, otherwise fallback to batch-level document_id @@ -103,18 +100,18 @@ async def insert_facts_batch( WITH input_data AS ( SELECT * FROM unnest( $2::text[], $3::vector[], $4::timestamptz[], $5::timestamptz[], $6::timestamptz[], $7::timestamptz[], - $8::text[], $9::text[], $10::float[], $11::jsonb[], $12::text[], $13::text[], $14::jsonb[], $15::jsonb[], $16::text[] + $8::text[], $9::text[], $10::jsonb[], $11::text[], $12::text[], $13::jsonb[], $14::jsonb[], $15::text[] ) AS t(text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, tags_json, + context, fact_type, metadata, chunk_id, document_id, tags_json, observation_scopes_json, text_signals) ) INSERT INTO {fq_table("memory_units")} (bank_id, text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, tags, + context, fact_type, metadata, chunk_id, document_id, tags, observation_scopes, text_signals, search_vector) SELECT $1, text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, + context, fact_type, metadata, chunk_id, document_id, COALESCE( (SELECT array_agg(elem) FROM jsonb_array_elements_text(tags_json) AS elem), '{{}}'::varchar[] @@ -135,18 +132,18 @@ async def insert_facts_batch( WITH input_data AS ( SELECT * FROM unnest( $2::text[], $3::vector[], $4::timestamptz[], $5::timestamptz[], $6::timestamptz[], $7::timestamptz[], - $8::text[], $9::text[], $10::float[], $11::jsonb[], $12::text[], $13::text[], $14::jsonb[], $15::jsonb[], $16::text[] + $8::text[], $9::text[], $10::jsonb[], $11::text[], $12::text[], $13::jsonb[], $14::jsonb[], $15::text[] ) AS t(text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, tags_json, + context, fact_type, metadata, chunk_id, document_id, tags_json, observation_scopes_json, text_signals) ) INSERT INTO {fq_table("memory_units")} (bank_id, text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, tags, + context, fact_type, metadata, chunk_id, document_id, tags, observation_scopes, text_signals) SELECT $1, text, embedding, event_date, occurred_start, occurred_end, mentioned_at, - context, fact_type, confidence_score, metadata, chunk_id, document_id, + context, fact_type, metadata, chunk_id, document_id, COALESCE( (SELECT array_agg(elem) FROM jsonb_array_elements_text(tags_json) AS elem), '{{}}'::varchar[] @@ -168,7 +165,6 @@ async def insert_facts_batch( mentioned_ats, contexts, fact_types, - confidence_scores, metadata_jsons, chunk_ids, document_ids, diff --git a/hindsight-api-slim/hindsight_api/engine/retain/types.py b/hindsight-api-slim/hindsight_api/engine/retain/types.py index 6cf70608..b27e0896 100644 --- a/hindsight-api-slim/hindsight_api/engine/retain/types.py +++ b/hindsight-api-slim/hindsight_api/engine/retain/types.py @@ -107,7 +107,7 @@ class ExtractedFact: """ fact_text: str - fact_type: str # "world", "experience", "opinion", "observation" + fact_type: str # "world", "experience", "observation" entities: list[str] = field(default_factory=list) occurred_start: datetime | None = None occurred_end: datetime | None = None @@ -287,7 +287,6 @@ class RetainBatch: contents: list[RetainContent] document_id: str | None = None fact_type_override: str | None = None - confidence_score: float | None = None document_tags: list[str] = field(default_factory=list) # Tags applied to all items # Extracted data (populated during processing) diff --git a/hindsight-api-slim/hindsight_api/engine/search/graph_retrieval.py b/hindsight-api-slim/hindsight_api/engine/search/graph_retrieval.py index 377b83cb..25955205 100644 --- a/hindsight-api-slim/hindsight_api/engine/search/graph_retrieval.py +++ b/hindsight-api-slim/hindsight_api/engine/search/graph_retrieval.py @@ -53,7 +53,7 @@ class GraphRetriever(ABC): pool: Database connection pool query_embedding_str: Query embedding as string (for finding entry points) bank_id: Memory bank identifier - fact_type: Fact type to filter ('world', 'experience', 'opinion', 'observation') + fact_type: Fact type to filter ('world', 'experience', 'observation') budget: Maximum number of nodes to explore/return query_text: Original query text (optional, for some strategies) semantic_seeds: Pre-computed semantic entry points (from semantic retrieval) diff --git a/hindsight-api-slim/hindsight_api/engine/search/think_utils.py b/hindsight-api-slim/hindsight_api/engine/search/think_utils.py index 2b1c31bb..31648298 100644 --- a/hindsight-api-slim/hindsight_api/engine/search/think_utils.py +++ b/hindsight-api-slim/hindsight_api/engine/search/think_utils.py @@ -110,11 +110,7 @@ def build_think_prompt( context: str | None = None, entity_summaries_text: str | None = None, ) -> str: - """Build the think prompt for the LLM. - - Note: opinion_facts_text parameter removed - opinions are now stored as mental models - and included via entity_summaries_text. - """ + """Build the think prompt for the LLM.""" disposition_desc = build_disposition_description(disposition) name_section = f""" diff --git a/hindsight-api-slim/hindsight_api/engine/search/trace.py b/hindsight-api-slim/hindsight_api/engine/search/trace.py index b61557bd..7f6c4465 100644 --- a/hindsight-api-slim/hindsight_api/engine/search/trace.py +++ b/hindsight-api-slim/hindsight_api/engine/search/trace.py @@ -131,7 +131,7 @@ class RetrievalResult(BaseModel): text: str = Field(description="Memory unit text content") context: str = Field(default="", description="Memory unit context") event_date: datetime | None = Field(default=None, description="When the memory occurred") - fact_type: str | None = Field(default=None, description="Fact type (world, experience, opinion)") + fact_type: str | None = Field(default=None, description="Fact type (world, experience)") score: float = Field(description="Score from this retrieval method") score_name: str = Field(description="Name of the score (e.g., 'similarity', 'bm25_score', 'activation')") @@ -140,9 +140,7 @@ class RetrievalMethodResults(BaseModel): """Results from a single retrieval method.""" method_name: Literal["semantic", "bm25", "graph", "temporal"] = Field(description="Name of retrieval method") - fact_type: str | None = Field( - default=None, description="Fact type this retrieval was for (world, experience, opinion)" - ) + fact_type: str | None = Field(default=None, description="Fact type this retrieval was for (world, experience)") results: list[RetrievalResult] = Field(description="Retrieved results with ranks") duration_seconds: float = Field(description="Time taken for this retrieval") metadata: dict[str, Any] = Field(default_factory=dict, description="Method-specific metadata") diff --git a/hindsight-api-slim/hindsight_api/engine/search/tracer.py b/hindsight-api-slim/hindsight_api/engine/search/tracer.py index 19247998..6298e34b 100644 --- a/hindsight-api-slim/hindsight_api/engine/search/tracer.py +++ b/hindsight-api-slim/hindsight_api/engine/search/tracer.py @@ -319,7 +319,7 @@ class SearchTracer: duration_seconds: Time taken for this retrieval score_field: Field name containing the score in data dict metadata: Optional metadata about this retrieval method - fact_type: Fact type this retrieval was for (world, experience, opinion) + fact_type: Fact type this retrieval was for (world, experience) """ retrieval_results = [] for rank, (doc_id, data) in enumerate(results, start=1): diff --git a/hindsight-api-slim/hindsight_api/extensions/operation_validator.py b/hindsight-api-slim/hindsight_api/extensions/operation_validator.py index b5f48afc..acbd4c59 100644 --- a/hindsight-api-slim/hindsight_api/extensions/operation_validator.py +++ b/hindsight-api-slim/hindsight_api/extensions/operation_validator.py @@ -96,7 +96,6 @@ class RetainContext: request_context: "RequestContext" document_id: str | None = None fact_type_override: str | None = None - confidence_score: float | None = None @dataclass @@ -169,7 +168,6 @@ class RetainResult: request_context: "RequestContext" document_id: str | None fact_type_override: str | None - confidence_score: float | None # Result unit_ids: list[list[str]] # List of unit IDs per content item success: bool = True @@ -402,7 +400,6 @@ class OperationValidatorExtension(Extension, ABC): - request_context: Request context with auth info - document_id: Optional document ID - fact_type_override: Optional fact type override - - confidence_score: Optional confidence score Returns: ValidationResult indicating whether the operation is allowed. diff --git a/hindsight-api-slim/hindsight_api/models.py b/hindsight-api-slim/hindsight_api/models.py index 15792903..8de106ba 100644 --- a/hindsight-api-slim/hindsight_api/models.py +++ b/hindsight-api-slim/hindsight_api/models.py @@ -97,7 +97,6 @@ class MemoryUnit(Base): occurred_end: Mapped[datetime | None] = mapped_column(TIMESTAMP(timezone=True)) # When fact occurred (range end) mentioned_at: Mapped[datetime | None] = mapped_column(TIMESTAMP(timezone=True)) # When fact was mentioned fact_type: Mapped[str] = mapped_column(Text, nullable=False, server_default="world") - confidence_score: Mapped[float | None] = mapped_column(Float) unit_metadata: Mapped[dict] = mapped_column( "metadata", JSONB, server_default=sql_text("'{}'::jsonb") ) # User-defined metadata (str->str) @@ -121,14 +120,7 @@ class MemoryUnit(Base): name="memory_units_document_fkey", ondelete="CASCADE", ), - CheckConstraint("fact_type IN ('world', 'experience', 'opinion', 'observation')"), - CheckConstraint("confidence_score IS NULL OR (confidence_score >= 0.0 AND confidence_score <= 1.0)"), - CheckConstraint( - "(fact_type = 'opinion' AND confidence_score IS NOT NULL) OR " - "(fact_type = 'observation') OR " - "(fact_type NOT IN ('opinion', 'observation') AND confidence_score IS NULL)", - name="confidence_score_fact_type_check", - ), + CheckConstraint("fact_type IN ('world', 'experience', 'observation')"), Index("idx_memory_units_bank_id", "bank_id"), Index("idx_memory_units_document_id", "document_id"), Index("idx_memory_units_event_date", "event_date", postgresql_ops={"event_date": "DESC"}), @@ -142,20 +134,6 @@ class MemoryUnit(Base): "event_date", postgresql_ops={"event_date": "DESC"}, ), - Index( - "idx_memory_units_opinion_confidence", - "bank_id", - "confidence_score", - postgresql_where=sql_text("fact_type = 'opinion'"), - postgresql_ops={"confidence_score": "DESC"}, - ), - Index( - "idx_memory_units_opinion_date", - "bank_id", - "event_date", - postgresql_where=sql_text("fact_type = 'opinion'"), - postgresql_ops={"event_date": "DESC"}, - ), Index( "idx_memory_units_observation_date", "bank_id", diff --git a/hindsight-api-slim/tests/test_batch_api_integration.py b/hindsight-api-slim/tests/test_batch_api_integration.py index 99d8d1b1..52a77f17 100644 --- a/hindsight-api-slim/tests/test_batch_api_integration.py +++ b/hindsight-api-slim/tests/test_batch_api_integration.py @@ -198,7 +198,7 @@ async def test_real_openai_batch_api(real_llm_config, test_contents_real, integr for fact in facts: assert hasattr(fact, "fact_text"), "Fact should have fact_text" assert hasattr(fact, "fact_type"), "Fact should have fact_type" - assert fact.fact_type in ["world", "experience", "opinion"], f"Invalid fact_type: {fact.fact_type}" + assert fact.fact_type in ["world", "experience"], f"Invalid fact_type: {fact.fact_type}" logger.info("\nāœ… All assertions passed!") diff --git a/hindsight-api-slim/tests/test_experience_fact_type_passthrough.py b/hindsight-api-slim/tests/test_experience_fact_type_passthrough.py new file mode 100644 index 00000000..6e25d3d9 --- /dev/null +++ b/hindsight-api-slim/tests/test_experience_fact_type_passthrough.py @@ -0,0 +1,59 @@ +""" +Regression test for experience fact_type preservation. + +The LLM extraction layer normalizes raw "assistant" ā†’ "experience" early in parsing. +The subsequent conversion to ExtractedFactType must pass through the already-normalized +fact_type rather than re-checking for "assistant" (which would remap experience ā†’ world). + +See: https://github.com/vectorize-io/hindsight/pull/839 +""" + +from datetime import datetime, timezone +from unittest.mock import AsyncMock, patch + +import pytest + +from hindsight_api.config import _get_raw_config +from hindsight_api.engine.response_models import TokenUsage +from hindsight_api.engine.retain.fact_extraction import ( + Fact, + RetainContent, + extract_facts_from_contents, + extract_facts_from_contents_batch_api, +) + + +@pytest.mark.asyncio +async def test_extract_facts_preserves_experience_type(): + """ + When extract_facts_from_text returns a Fact with fact_type="experience", + extract_facts_from_contents must preserve it (not remap to "world"). + """ + contents = [ + RetainContent( + content="I fixed the failing tests after discovering they mocked the wrong interface.", + event_date=datetime(2026, 4, 1, tzinfo=timezone.utc), + context="assistant work log", + ) + ] + extracted_fact = Fact( + fact="Fixed the failing tests after discovering they mocked the wrong interface.", + fact_type="experience", + ) + + with patch( + "hindsight_api.engine.retain.fact_extraction.extract_facts_from_text", + new=AsyncMock(return_value=([extracted_fact], [(contents[0].content, 1)], TokenUsage())), + ): + facts, _chunks, _usage = await extract_facts_from_contents( + contents=contents, + llm_config=None, + agent_name="TestAgent", + config=_get_raw_config(), + ) + + assert len(facts) == 1 + assert facts[0].fact_type == "experience", ( + f"Expected 'experience' but got '{facts[0].fact_type}' ā€” " + f"the conversion layer is remapping the already-normalized fact_type" + ) diff --git a/hindsight-api-slim/tests/test_extensions.py b/hindsight-api-slim/tests/test_extensions.py index 01645ece..292da62b 100644 --- a/hindsight-api-slim/tests/test_extensions.py +++ b/hindsight-api-slim/tests/test_extensions.py @@ -303,7 +303,6 @@ class TestOperationHooksParameters: contents=contents, document_id=document_id, fact_type_override="world", - confidence_score=0.9, request_context=ctx, ) @@ -317,7 +316,6 @@ class TestOperationHooksParameters: assert pre_ctx.contents[0]["content"] == contents[0]["content"] assert pre_ctx.document_id == document_id assert pre_ctx.fact_type_override == "world" - assert pre_ctx.confidence_score == 0.9 assert pre_ctx.request_context == ctx @pytest.mark.asyncio @@ -334,7 +332,6 @@ class TestOperationHooksParameters: contents=contents, document_id=document_id, fact_type_override="experience", - confidence_score=0.8, request_context=ctx, ) @@ -345,7 +342,6 @@ class TestOperationHooksParameters: assert post_result.bank_id == bank_id assert post_result.document_id == document_id assert post_result.fact_type_override == "experience" - assert post_result.confidence_score == 0.8 assert post_result.request_context == ctx # Verify result data diff --git a/hindsight-api-slim/tests/test_llm_provider.py b/hindsight-api-slim/tests/test_llm_provider.py index 616a2f66..dd28b0d5 100644 --- a/hindsight-api-slim/tests/test_llm_provider.py +++ b/hindsight-api-slim/tests/test_llm_provider.py @@ -284,7 +284,7 @@ async def test_llm_provider_memory_operations(provider: str, model: str): # Verify facts have required fields for fact in facts: assert fact.fact, f"{provider}/{model} fact missing text" - assert fact.fact_type in ["world", "experience", "opinion"], f"{provider}/{model} invalid fact_type: {fact.fact_type}" + assert fact.fact_type in ["world", "experience"], f"{provider}/{model} invalid fact_type: {fact.fact_type}" # Test 2: Reflect (actual reflect function) response = await reflect(