From 49e233cdb7ccc22749a47020ccc431cfab79dd6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Boschi?= Date: Thu, 8 Jan 2026 14:43:48 +0100 Subject: [PATCH] fix: duplicated causal relationships and token optimization (#126) * fix: duplicated causal relationships and token optimization * doc * doc --- hindsight-api/hindsight_api/config.py | 4 + .../engine/retain/fact_extraction.py | 240 +++++++----------- hindsight-api/hindsight_api/main.py | 1 + hindsight-api/tests/test_causal_relations.py | 223 ++++++++++++++++ .../tests/test_causal_relationships.py | 74 ++---- .../docs/developer/configuration.md | 1 + 6 files changed, 346 insertions(+), 197 deletions(-) create mode 100644 hindsight-api/tests/test_causal_relations.py diff --git a/hindsight-api/hindsight_api/config.py b/hindsight-api/hindsight_api/config.py index 87b59ceb..06902f0b 100644 --- a/hindsight-api/hindsight_api/config.py +++ b/hindsight-api/hindsight_api/config.py @@ -64,6 +64,7 @@ ENV_OBSERVATION_TOP_ENTITIES = "HINDSIGHT_API_OBSERVATION_TOP_ENTITIES" # Retain settings ENV_RETAIN_MAX_COMPLETION_TOKENS = "HINDSIGHT_API_RETAIN_MAX_COMPLETION_TOKENS" +ENV_RETAIN_CHUNK_SIZE = "HINDSIGHT_API_RETAIN_CHUNK_SIZE" # Optimization flags ENV_SKIP_LLM_VERIFICATION = "HINDSIGHT_API_SKIP_LLM_VERIFICATION" @@ -103,6 +104,7 @@ DEFAULT_OBSERVATION_TOP_ENTITIES = 5 # Max entities to process per retain batch # Retain settings DEFAULT_RETAIN_MAX_COMPLETION_TOKENS = 64000 # Max tokens for fact extraction LLM call +DEFAULT_RETAIN_CHUNK_SIZE = 3000 # Max chars per chunk for fact extraction # Database migrations DEFAULT_RUN_MIGRATIONS_ON_STARTUP = True @@ -182,6 +184,7 @@ class HindsightConfig: # Retain settings retain_max_completion_tokens: int + retain_chunk_size: int # Optimization flags skip_llm_verification: bool @@ -239,6 +242,7 @@ class HindsightConfig: retain_max_completion_tokens=int( os.getenv(ENV_RETAIN_MAX_COMPLETION_TOKENS, str(DEFAULT_RETAIN_MAX_COMPLETION_TOKENS)) ), + retain_chunk_size=int(os.getenv(ENV_RETAIN_CHUNK_SIZE, str(DEFAULT_RETAIN_CHUNK_SIZE))), # Database migrations run_migrations_on_startup=os.getenv(ENV_RUN_MIGRATIONS_ON_STARTUP, "true").lower() == "true", ) diff --git a/hindsight-api/hindsight_api/engine/retain/fact_extraction.py b/hindsight-api/hindsight_api/engine/retain/fact_extraction.py index af72f264..5580c47b 100644 --- a/hindsight-api/hindsight_api/engine/retain/fact_extraction.py +++ b/hindsight-api/hindsight_api/engine/retain/fact_extraction.py @@ -111,52 +111,44 @@ class Fact(BaseModel): class CausalRelation(BaseModel): - """Causal relationship between facts (legacy - embedded in each fact).""" + """Causal relationship from this fact to a previous fact (stored format).""" - target_fact_index: int = Field( - description="Index of the related fact in the facts array (0-based). " - "This creates a directed causal link to another fact in the extraction." - ) - relation_type: Literal["causes", "caused_by", "enables", "prevents"] = Field( - description="Type of causal relationship: " - "'causes' = this fact directly causes the target fact, " - "'caused_by' = this fact was caused by the target fact, " - "'enables' = this fact enables/allows the target fact, " - "'prevents' = this fact prevents/blocks the target fact" + target_fact_index: int = Field(description="Index of the related fact in the facts array (0-based).") + relation_type: Literal["caused_by", "enabled_by", "prevented_by"] = Field( + description="How this fact relates to the target: " + "'caused_by' = this fact was caused by the target, " + "'enabled_by' = this fact was enabled by the target, " + "'prevented_by' = this fact was prevented by the target" ) strength: float = Field( - description="Strength of causal relationship (0.0 to 1.0). " - "1.0 = direct/strong causation, 0.5 = moderate, 0.3 = weak/indirect", + description="Strength of relationship (0.0 to 1.0)", ge=0.0, le=1.0, default=1.0, ) -class TopLevelCausalRelation(BaseModel): +class FactCausalRelation(BaseModel): """ - Causal relationship between two facts (top-level schema). + Causal relationship from this fact to a PREVIOUS fact (embedded in each fact). - This is the preferred format - defined AFTER all facts are extracted, - allowing the LLM to see the full list of facts before specifying relationships. + Uses index-based references but ONLY allows referencing facts that appear + BEFORE this fact in the list. This prevents hallucination of invalid indices. """ - from_fact_index: int = Field( - description="Index of the source fact (0-based). The fact that causes/enables/prevents." + target_index: int = Field( + description="Index of the PREVIOUS fact this relates to (0-based). " + "MUST be less than this fact's position in the list. " + "Example: if this is fact #5, target_index can only be 0, 1, 2, 3, or 4." ) - to_fact_index: int = Field( - description="Index of the target fact (0-based). The fact that is caused/enabled/prevented." - ) - relation_type: Literal["causes", "caused_by", "enables", "prevents"] = Field( - description="Type of causal relationship: " - "'causes' = source fact directly causes the target fact, " - "'caused_by' = source fact was caused by the target fact, " - "'enables' = source fact enables/allows the target fact, " - "'prevents' = source fact prevents/blocks the target fact" + relation_type: Literal["caused_by", "enabled_by", "prevented_by"] = Field( + description="How this fact relates to the target fact: " + "'caused_by' = this fact was caused by the target fact, " + "'enabled_by' = this fact was enabled by the target fact, " + "'prevented_by' = this fact was blocked/prevented by the target fact" ) strength: float = Field( - description="Strength of causal relationship (0.0 to 1.0). " - "1.0 = direct/strong causation, 0.5 = moderate, 0.3 = weak/indirect", + description="Strength of relationship (0.0 to 1.0). 1.0 = strong, 0.5 = moderate", ge=0.0, le=1.0, default=1.0, @@ -246,8 +238,12 @@ class ExtractedFact(BaseModel): default=None, description="Named entities, objects, AND abstract concepts from the fact. Include: people names, organizations, places, significant objects (e.g., 'coffee maker', 'car'), AND abstract concepts/themes (e.g., 'friendship', 'career growth', 'loss', 'celebration'). Extract anything that could help link related facts together.", ) - causal_relations: list[CausalRelation] | None = Field( - default=None, description="Causal links to other facts. Can be null." + + # Causal relations to PREVIOUS facts only (prevents hallucination of invalid indices) + causal_relations: list[FactCausalRelation] | None = Field( + default=None, + description="Causal links to PREVIOUS facts only. target_index MUST be less than this fact's position. " + "Example: fact #3 can only reference facts 0, 1, or 2. Max 2 relations per fact.", ) @field_validator("entities", mode="before") @@ -258,14 +254,6 @@ class ExtractedFact(BaseModel): return [] return v - @field_validator("causal_relations", mode="before") - @classmethod - def ensure_causal_relations_list(cls, v): - """Ensure causal_relations is always a list (convert None to empty list).""" - if v is None: - return [] - return v - def build_fact_text(self) -> str: """Combine all dimensions into a single comprehensive fact string.""" parts = [self.what] @@ -285,15 +273,9 @@ class ExtractedFact(BaseModel): class FactExtractionResponse(BaseModel): - """Response containing all extracted facts and their causal relationships.""" + """Response containing all extracted facts (causal relations are embedded in each fact).""" facts: list[ExtractedFact] = Field(description="List of extracted factual statements") - causal_relationships: list[TopLevelCausalRelation] | None = Field( - default=None, - description="Causal relationships between facts. Define these AFTER listing all facts. " - "Each relationship specifies from_fact_index -> to_fact_index with a relation type. " - "Indices must be valid (0 to N-1 where N is the number of facts).", - ) def chunk_text(text: str, max_chars: int) -> list[str]: @@ -616,50 +598,49 @@ WHAT TO EXTRACT vs SKIP ❌ SKIP: Greetings, filler ("thanks", "cool"), purely structural statements ══════════════════════════════════════════════════════════════════════════ -CAUSAL RELATIONSHIPS (CRITICAL - DEFINE AFTER ALL FACTS) +CAUSAL RELATIONSHIPS (EMBEDDED IN EACH FACT - REFERENCE PREVIOUS FACTS ONLY) ══════════════════════════════════════════════════════════════════════════ -⚠️ IMPORTANT: Causal relationships are defined at the TOP LEVEL, AFTER listing all facts! +Each fact can have a `causal_relations` array that links to PREVIOUS facts only. +⚠️ CRITICAL: target_index MUST be less than this fact's position in the list! -The `causal_relationships` array goes at the root of your response (NOT inside each fact). -This allows you to see all facts first before defining how they relate. +If you're writing fact #5, you can only reference facts 0, 1, 2, 3, or 4. +This ensures all references are valid. -Format: -```json -{{ - "facts": [...all your extracted facts...], - "causal_relationships": [ - {{"from_fact_index": 0, "to_fact_index": 1, "relation_type": "causes", "strength": 0.9}}, - {{"from_fact_index": 1, "to_fact_index": 2, "relation_type": "enables", "strength": 0.7}} - ] -}} -``` +Relationship types (all describe how THIS fact relates to the target): +- "caused_by": This fact was caused by the target fact +- "enabled_by": This fact was enabled/allowed by the target fact +- "prevented_by": This fact was blocked/prevented by the target fact -Relationship types: -- "causes": Fact A directly causes Fact B (A → B) -- "caused_by": Fact A was caused by Fact B (A ← B) -- "enables": Fact A enables/allows Fact B to happen -- "prevents": Fact A prevents/blocks Fact B from happening - -⚠️ INDEX VALIDATION: If you extract N facts (indices 0 to N-1), both from_fact_index and to_fact_index MUST be in range [0, N-1]. +Max 2 causal relations per fact. Only add if there's a clear causal link. Example (Event Date: March 15, 2024): Input: "I lost my job in January. Because of that, I couldn't pay rent. So I had to move to a cheaper apartment." -Facts extracted: -- Fact 0: "User lost their job in January due to layoffs" -- Fact 1: "User couldn't pay rent because of job loss" -- Fact 2: "User moved to a cheaper apartment" - -Causal relationships (at root level): +Output facts: ```json -"causal_relationships": [ - {{"from_fact_index": 0, "to_fact_index": 1, "relation_type": "causes", "strength": 1.0}}, - {{"from_fact_index": 1, "to_fact_index": 2, "relation_type": "causes", "strength": 0.9}} -] +{{ + "facts": [ + {{ + "what": "User lost their job in January due to company layoffs", + ...other fields... + "causal_relations": null // First fact - nothing to reference + }}, + {{ + "what": "User couldn't pay rent because of job loss", + ...other fields... + "causal_relations": [{{"target_index": 0, "relation_type": "caused_by", "strength": 1.0}}] + }}, + {{ + "what": "User moved to a cheaper apartment", + ...other fields... + "causal_relations": [{{"target_index": 1, "relation_type": "caused_by", "strength": 0.9}}] + }} + ] +}} ``` -This creates a chain: Job loss (0) → Can't pay rent (1) → Moved to cheaper apartment (2)""" +This creates: Job loss (0) ← Can't pay rent (1) ← Moved apartment (2)""" import logging @@ -722,8 +703,6 @@ Text: return [], usage raw_facts = extraction_response_json.get("facts", []) - # Get top-level causal relationships (new schema) - top_level_causal_relations = extraction_response_json.get("causal_relationships", []) if not raw_facts: logger.debug( @@ -735,47 +714,6 @@ Text: f"text: {chunk}" ) - # Build a map from fact index to causal relations (from top-level field) - # This converts from_fact_index -> [{target_fact_index, relation_type, strength}] - causal_relations_by_fact: dict[int, list[dict]] = {} - if top_level_causal_relations: - num_facts = len(raw_facts) - for rel in top_level_causal_relations: - if not isinstance(rel, dict): - continue - from_idx = rel.get("from_fact_index") - to_idx = rel.get("to_fact_index") - relation_type = rel.get("relation_type") - strength = rel.get("strength", 1.0) - - # Validate indices - if from_idx is None or to_idx is None or relation_type is None: - logger.warning(f"Skipping malformed top-level causal relation: {rel}") - continue - if from_idx < 0 or from_idx >= num_facts: - logger.warning( - f"Invalid from_fact_index {from_idx} in top-level causal relation " - f"(valid range: 0-{num_facts - 1}). Skipping." - ) - continue - if to_idx < 0 or to_idx >= num_facts: - logger.warning( - f"Invalid to_fact_index {to_idx} in top-level causal relation " - f"(valid range: 0-{num_facts - 1}). Skipping." - ) - continue - - # Add to the map for the from_fact_index - if from_idx not in causal_relations_by_fact: - causal_relations_by_fact[from_idx] = [] - causal_relations_by_fact[from_idx].append( - { - "target_fact_index": to_idx, - "relation_type": relation_type, - "strength": strength, - } - ) - for i, llm_fact in enumerate(raw_facts): # Skip non-dict entries but track them for retry if not isinstance(llm_fact, dict): @@ -880,37 +818,38 @@ Text: if validated_entities: fact_data["entities"] = validated_entities - # Add causal relations from both sources: - # 1. Top-level causal_relationships (preferred, new schema) - # 2. Per-fact causal_relations (legacy, for backward compatibility) + # Add per-fact causal relations (new schema: target_index must be < current fact index) validated_relations = [] + causal_relations_raw = get_value("causal_relations") + if causal_relations_raw: + for rel in causal_relations_raw: + if not isinstance(rel, dict): + continue + # New schema uses target_index + target_idx = rel.get("target_index") + relation_type = rel.get("relation_type") + strength = rel.get("strength", 1.0) + + if target_idx is None or relation_type is None: + continue + + # Validate: target_index must be < current fact index + if target_idx < 0 or target_idx >= i: + logger.debug( + f"Invalid target_index {target_idx} for fact {i} (must be 0 to {i - 1}). Skipping." + ) + continue - # First, add relations from top-level (already validated above) - if i in causal_relations_by_fact: - for rel in causal_relations_by_fact[i]: try: - validated_relations.append(CausalRelation.model_validate(rel)) - except Exception as e: - logger.warning(f"Invalid top-level causal relation for fact {i}: {rel}: {e}") - - # Then, add any legacy per-fact relations (with index validation) - legacy_causal_relations = get_value("causal_relations") - if legacy_causal_relations: - num_facts = len(raw_facts) - for rel in legacy_causal_relations: - if isinstance(rel, dict) and "target_fact_index" in rel and "relation_type" in rel: - target_idx = rel.get("target_fact_index") - # Validate target index for legacy format too - if target_idx is not None and 0 <= target_idx < num_facts: - try: - validated_relations.append(CausalRelation.model_validate(rel)) - except Exception as e: - logger.warning(f"Invalid causal relation {rel}: {e}") - else: - logger.warning( - f"Invalid target_fact_index {target_idx} in per-fact causal relation " - f"from fact {i} (valid range: 0-{num_facts - 1}). Skipping." + validated_relations.append( + CausalRelation( + target_fact_index=target_idx, + relation_type=relation_type, + strength=strength, ) + ) + except Exception as e: + logger.debug(f"Invalid causal relation {rel}: {e}") if validated_relations: fact_data["causal_relations"] = validated_relations @@ -1099,7 +1038,8 @@ async def extract_facts_from_text( - chunks: List of tuples (chunk_text, fact_count) for each chunk - usage: Aggregated token usage across all LLM calls """ - chunks = chunk_text(text, max_chars=3000) + config = get_config() + chunks = chunk_text(text, max_chars=config.retain_chunk_size) tasks = [ _extract_facts_with_auto_split( chunk=chunk, diff --git a/hindsight-api/hindsight_api/main.py b/hindsight-api/hindsight_api/main.py index b7026c1a..4c74170e 100644 --- a/hindsight-api/hindsight_api/main.py +++ b/hindsight-api/hindsight_api/main.py @@ -193,6 +193,7 @@ def main(): observation_min_facts=config.observation_min_facts, observation_top_entities=config.observation_top_entities, retain_max_completion_tokens=config.retain_max_completion_tokens, + retain_chunk_size=config.retain_chunk_size, skip_llm_verification=config.skip_llm_verification, lazy_reranker=config.lazy_reranker, run_migrations_on_startup=config.run_migrations_on_startup, diff --git a/hindsight-api/tests/test_causal_relations.py b/hindsight-api/tests/test_causal_relations.py new file mode 100644 index 00000000..2b096ddb --- /dev/null +++ b/hindsight-api/tests/test_causal_relations.py @@ -0,0 +1,223 @@ +""" +Test suite for causal relations extraction and validation. + +Tests that: +1. Causal relations only reference previous facts (target_index < current fact index) +2. Invalid causal relation indices are rejected +3. The new per-fact causal relations schema works correctly +""" + +from datetime import datetime + +import pytest + +from hindsight_api import LLMConfig +from hindsight_api.engine.retain.fact_extraction import extract_facts_from_text + + +class TestCausalRelationsValidation: + """Tests for causal relations index validation.""" + + @pytest.mark.asyncio + async def test_causal_relations_only_reference_previous_facts(self): + """ + Test that causal relations can only reference facts that appear before them. + + This test verifies the new schema that prevents hallucination of invalid + fact indices by constraining target_index to be less than the current fact's index. + """ + # Text with clear causal chain + text = """ + I lost my job in January due to company layoffs. + Because I lost my job, I couldn't pay my rent. + Since I couldn't afford rent, I had to move to a cheaper apartment. + After moving, I started looking for a new job. + """ + + context = "Personal life update" + llm_config = LLMConfig.for_memory() + event_date = datetime(2024, 3, 15) + + facts, _, usage = await extract_facts_from_text( + text=text, + event_date=event_date, + context=context, + llm_config=llm_config, + agent_name="TestUser", + ) + + assert len(facts) > 0, "Should extract at least one fact" + + # Verify all causal relations reference valid previous facts + for i, fact in enumerate(facts): + if fact.causal_relations: + for rel in fact.causal_relations: + assert rel.target_fact_index < i, ( + f"Fact {i} has causal relation to fact {rel.target_fact_index}, " + f"but target_index must be < current index ({i})" + ) + assert rel.target_fact_index >= 0, ( + f"Fact {i} has negative causal relation index: {rel.target_fact_index}" + ) + assert rel.relation_type in ["caused_by", "enabled_by", "prevented_by"], ( + f"Invalid relation_type: {rel.relation_type}" + ) + + @pytest.mark.asyncio + async def test_first_fact_has_no_causal_relations(self): + """ + Test that the first fact (index 0) cannot have causal relations. + + Since causal relations can only reference previous facts, + and there are no facts before index 0, the first fact should + have no causal relations. + """ + text = """ + The user started a new machine learning project. + The project requires learning TensorFlow. + Learning TensorFlow is challenging but rewarding. + """ + + context = "Project update" + llm_config = LLMConfig.for_memory() + event_date = datetime(2024, 6, 1) + + facts, _, _ = await extract_facts_from_text( + text=text, + event_date=event_date, + context=context, + llm_config=llm_config, + agent_name="TestUser", + ) + + assert len(facts) > 0, "Should extract at least one fact" + + # First fact should have no causal relations (nothing to reference) + if facts[0].causal_relations: + # If there are causal relations on the first fact, they should be empty + # or the validation should have filtered them out + for rel in facts[0].causal_relations: + # This should never happen due to validation + assert False, ( + f"First fact should not have causal relations, " + f"but found: target_index={rel.target_fact_index}" + ) + + @pytest.mark.asyncio + async def test_causal_chain_extraction(self): + """ + Test that a clear causal chain is extracted with valid relations. + """ + text = """ + Emily got promoted to senior engineer last month. + Because of her promotion, she received a significant salary increase. + With the extra money, she decided to buy a new car. + """ + + context = "Personal achievement story" + llm_config = LLMConfig.for_memory() + event_date = datetime(2024, 7, 15) + + facts, _, _ = await extract_facts_from_text( + text=text, + event_date=event_date, + context=context, + llm_config=llm_config, + agent_name="TestUser", + ) + + assert len(facts) > 0, "Should extract facts about the causal chain" + + # Collect all causal relations + all_relations = [] + for i, fact in enumerate(facts): + if fact.causal_relations: + for rel in fact.causal_relations: + all_relations.append({ + "from_fact": i, + "to_fact": rel.target_fact_index, + "type": rel.relation_type, + }) + + # If causal relations were extracted, verify they form a valid chain + if all_relations: + for rel in all_relations: + assert rel["to_fact"] < rel["from_fact"], ( + f"Causal relation from fact {rel['from_fact']} to fact {rel['to_fact']} " + f"is invalid (target must be < source)" + ) + + @pytest.mark.asyncio + async def test_token_efficiency_with_causal_relations(self): + """ + Test that causal relations don't cause excessive output tokens. + + This test verifies that the new schema (per-fact causal relations + with index constraints) doesn't waste tokens on invalid relations. + """ + text = """ + The company announced budget cuts in Q1. + Due to the budget cuts, the marketing team was reduced. + The reduced team meant fewer campaigns could be run. + With fewer campaigns, lead generation dropped. + Lower leads resulted in decreased sales. + """ + + context = "Business impact analysis" + llm_config = LLMConfig.for_memory() + event_date = datetime(2024, 4, 1) + + facts, _, usage = await extract_facts_from_text( + text=text, + event_date=event_date, + context=context, + llm_config=llm_config, + agent_name="TestUser", + ) + + assert len(facts) > 0, "Should extract facts" + + # Calculate output/input ratio + if usage.input_tokens > 0: + ratio = usage.output_tokens / usage.input_tokens + # The ratio should be reasonable (< 5x) with the new schema + # Previously it could be 7-10x due to hallucinated indices + assert ratio < 6, ( + f"Output/input token ratio {ratio:.2f}x is too high. " + f"Input: {usage.input_tokens}, Output: {usage.output_tokens}" + ) + + @pytest.mark.asyncio + async def test_relation_types_are_backward_looking(self): + """ + Test that all relation types describe how the current fact + relates to a previous fact (caused_by, enabled_by, prevented_by). + """ + text = """ + Alice learned Python programming. + Because she knew Python, she got a job as a data scientist. + Her data science skills enabled her to lead the analytics team. + """ + + context = "Career progression" + llm_config = LLMConfig.for_memory() + event_date = datetime(2024, 5, 1) + + facts, _, _ = await extract_facts_from_text( + text=text, + event_date=event_date, + context=context, + llm_config=llm_config, + agent_name="TestUser", + ) + + # Verify relation types are all backward-looking + valid_types = {"caused_by", "enabled_by", "prevented_by"} + + for i, fact in enumerate(facts): + if fact.causal_relations: + for rel in fact.causal_relations: + assert rel.relation_type in valid_types, ( + f"Invalid relation_type '{rel.relation_type}'. " + f"Must be one of: {valid_types}" + ) diff --git a/hindsight-api/tests/test_causal_relationships.py b/hindsight-api/tests/test_causal_relationships.py index 6a8f61d3..88720764 100644 --- a/hindsight-api/tests/test_causal_relationships.py +++ b/hindsight-api/tests/test_causal_relationships.py @@ -37,11 +37,7 @@ After searching for weeks, I finally found a cheaper apartment in Brooklyn. llm_config = LLMConfig.for_memory() facts, _, _ = await extract_facts_from_text( - text=text, - event_date=datetime(2024, 3, 15), - context=context, - llm_config=llm_config, - agent_name="TestUser" + text=text, event_date=datetime(2024, 3, 15), context=context, llm_config=llm_config, agent_name="TestUser" ) assert len(facts) >= 3, f"Should extract at least 3 facts from the causal chain. Got {len(facts)}" @@ -51,24 +47,28 @@ After searching for weeks, I finally found a cheaper apartment in Brooklyn. for i, fact in enumerate(facts): if fact.causal_relations: for rel in fact.causal_relations: - all_causal_relations.append({ - "from_fact_index": i, - "to_fact_index": rel.target_fact_index, - "relation_type": rel.relation_type, - "strength": rel.strength, - "from_fact_text": fact.fact[:50], - }) + all_causal_relations.append( + { + "from_fact_index": i, + "to_fact_index": rel.target_fact_index, + "relation_type": rel.relation_type, + "strength": rel.strength, + "from_fact_text": fact.fact[:50], + } + ) # Verify that ALL causal relation indices are valid + # New constraint: target_index must be < from_fact_index (can only reference PREVIOUS facts) num_facts = len(facts) invalid_relations = [] for rel in all_causal_relations: - if rel["to_fact_index"] < 0 or rel["to_fact_index"] >= num_facts: + # Must be non-negative and less than the current fact's index + if rel["to_fact_index"] < 0 or rel["to_fact_index"] >= rel["from_fact_index"]: invalid_relations.append(rel) assert len(invalid_relations) == 0, ( f"Found {len(invalid_relations)} causal relations with invalid indices! " - f"Valid range is 0-{num_facts - 1}. " + f"Each target_fact_index must be < from_fact_index (can only reference previous facts). " f"Invalid relations: {invalid_relations}" ) @@ -78,8 +78,8 @@ After searching for weeks, I finally found a cheaper apartment in Brooklyn. f"Got {len(all_causal_relations)}: {all_causal_relations}" ) - # Verify relation types are valid - valid_types = {"causes", "caused_by", "enables", "prevents"} + # Verify relation types are valid (passive only - facts reference PREVIOUS facts) + valid_types = {"caused_by", "enabled_by", "prevented_by"} for rel in all_causal_relations: assert rel["relation_type"] in valid_types, ( f"Invalid relation_type '{rel['relation_type']}'. Must be one of {valid_types}" @@ -106,23 +106,18 @@ The renovation took three months and cost $15,000. llm_config = LLMConfig.for_memory() facts, _, _ = await extract_facts_from_text( - text=text, - event_date=datetime(2024, 6, 1), - context=context, - llm_config=llm_config, - agent_name="TestUser" + text=text, event_date=datetime(2024, 6, 1), context=context, llm_config=llm_config, agent_name="TestUser" ) assert len(facts) >= 4, f"Should extract at least 4 facts. Got {len(facts)}" - # Validate all causal relation indices - num_facts = len(facts) + # Validate all causal relation indices (must reference PREVIOUS facts only) for i, fact in enumerate(facts): if fact.causal_relations: for rel in fact.causal_relations: - assert 0 <= rel.target_fact_index < num_facts, ( + assert 0 <= rel.target_fact_index < i, ( f"Fact {i} has causal relation to invalid index {rel.target_fact_index}. " - f"Valid range is 0-{num_facts - 1}. " + f"Must reference previous facts only (valid range: 0 to {i - 1}). " f"Fact text: {fact.fact[:80]}..." ) @@ -141,11 +136,7 @@ Machine learning fascinated me so much that I changed my career to data science. llm_config = LLMConfig.for_memory() facts, _, _ = await extract_facts_from_text( - text=text, - event_date=datetime(2024, 1, 1), - context=context, - llm_config=llm_config, - agent_name="TestUser" + text=text, event_date=datetime(2024, 1, 1), context=context, llm_config=llm_config, agent_name="TestUser" ) # Check no fact references itself @@ -153,8 +144,7 @@ Machine learning fascinated me so much that I changed my career to data science. if fact.causal_relations: for rel in fact.causal_relations: assert rel.target_fact_index != i, ( - f"Fact {i} has a self-referencing causal relation! " - f"Fact text: {fact.fact}" + f"Fact {i} has a self-referencing causal relation! Fact text: {fact.fact}" ) @pytest.mark.asyncio @@ -173,22 +163,16 @@ The new role enabled me to lead a team of engineers. llm_config = LLMConfig.for_memory() facts, _, _ = await extract_facts_from_text( - text=text, - event_date=datetime(2024, 2, 15), - context=context, - llm_config=llm_config, - agent_name="TestUser" + text=text, event_date=datetime(2024, 2, 15), context=context, llm_config=llm_config, agent_name="TestUser" ) - num_facts = len(facts) - - # Validate all indices + # Validate all indices (must reference PREVIOUS facts only) for i, fact in enumerate(facts): if fact.causal_relations: for rel in fact.causal_relations: - assert 0 <= rel.target_fact_index < num_facts, ( + assert 0 <= rel.target_fact_index < i, ( f"Invalid target_fact_index {rel.target_fact_index} in fact {i}. " - f"Valid range: 0-{num_facts - 1}" + f"Must reference previous facts only (valid range: 0 to {i - 1})" ) @pytest.mark.asyncio @@ -206,11 +190,7 @@ Reduced spending somewhat affected local businesses. llm_config = LLMConfig.for_memory() facts, _, _ = await extract_facts_from_text( - text=text, - event_date=datetime(2024, 4, 1), - context=context, - llm_config=llm_config, - agent_name="TestUser" + text=text, event_date=datetime(2024, 4, 1), context=context, llm_config=llm_config, agent_name="TestUser" ) for i, fact in enumerate(facts): diff --git a/hindsight-docs/docs/developer/configuration.md b/hindsight-docs/docs/developer/configuration.md index 33441626..1ed70e61 100644 --- a/hindsight-docs/docs/developer/configuration.md +++ b/hindsight-docs/docs/developer/configuration.md @@ -250,6 +250,7 @@ Controls the retain (memory ingestion) pipeline. | Variable | Description | Default | |----------|-------------|---------| | `HINDSIGHT_API_RETAIN_MAX_COMPLETION_TOKENS` | Max completion tokens for fact extraction LLM calls | `64000` | +| `HINDSIGHT_API_RETAIN_CHUNK_SIZE` | Max characters per chunk for fact extraction. Larger chunks extract fewer LLM calls but may lose context. | `3000` | ### Local MCP Server