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
This commit is contained in:
Nicolò Boschi
GitHub
parent
8d1bfbbd2b
commit
9cfdd464a9
17 changed files with 174 additions and 79 deletions
|
|
@ -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'"
|
||||
)
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"])
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"""
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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!")
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
)
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue