diff --git a/src/powermem/intelligence/importance_evaluator.py b/src/powermem/intelligence/importance_evaluator.py index 359c77153..44fc424ca 100644 --- a/src/powermem/intelligence/importance_evaluator.py +++ b/src/powermem/intelligence/importance_evaluator.py @@ -5,9 +5,11 @@ """ import logging +import re from typing import Any, Dict, Optional -import json + from ..prompts.importance_evaluation import ImportanceEvaluationPrompts +from ..utils.utils import parse_json_from_text logger = logging.getLogger(__name__) @@ -192,6 +194,13 @@ def _llm_based_evaluation( # Parse the response to extract importance score importance_score = self._parse_importance_response(response) + if importance_score is None: + logger.warning( + "LLM response could not be parsed reliably, " + "falling back to rule-based evaluation" + ) + return self._rule_based_evaluation(content, metadata, context) + logger.debug(f"LLM evaluated importance: {importance_score}") return importance_score @@ -310,43 +319,105 @@ def _evaluate_factual(self, content: str) -> float: return min(score, 1.0) - def _parse_importance_response(self, response: str) -> float: + def _parse_importance_response(self, response: str) -> Optional[float]: """ Parse LLM response to extract importance score. - + + Uses a three-level fallback strategy where each level only accepts + verifiable signals. Returns None when no reliable score can be + extracted, allowing the caller to fall back to rule-based evaluation. + Args: response: LLM response string - + Returns: - Importance score between 0 and 1 + Importance score between 0 and 1, or None if parsing fails """ - try: - # Try to extract JSON from response - if "{" in response and "}" in response: - start = response.find("{") - end = response.rfind("}") + 1 - json_str = response[start:end] - - result = json.loads(json_str) - - if "importance_score" in result: - score = float(result["importance_score"]) - # Ensure score is within valid range - return max(0.0, min(1.0, score)) - - # Fallback: try to extract number from response - import re - numbers = re.findall(r'\d+\.?\d*', response) - if numbers: - score = float(numbers[0]) - return max(0.0, min(1.0, score)) - - logger.warning(f"Could not parse importance score from response: {response}") - return 0.5 # Default medium importance - - except Exception as e: - logger.error(f"Failed to parse importance response: {e}") - return 0.5 # Default medium importance + # L1: Structured JSON parsing via shared utility + score = self._parse_importance_from_json(response) + if score is not None: + return score + + # L2: Field-name-anchored regex (only accepts numbers next to known keys) + score = self._parse_importance_from_field_regex(response) + if score is not None: + return score + + # L3: Safe failure — return None so caller can fall back to rule-based + logger.warning( + "Could not parse importance score from LLM response, " + "will fall back to rule-based evaluation" + ) + return None + + def _parse_importance_from_json(self, response: str) -> Optional[float]: + """L1: Extract importance score from JSON in the response.""" + result = parse_json_from_text(response, expected_type=dict) + if result is None: + return None + + # Try primary field names: importance_score, overall_score + for field in ("importance_score", "overall_score"): + if field in result: + try: + score = float(result[field]) + if 0.0 <= score <= 1.0: + return score + logger.warning( + f"Parsed '{field}' = {score} is outside [0, 1], ignoring" + ) + except (TypeError, ValueError): + pass + + # Fallback: synthesize from criteria_scores using weights + criteria = result.get("criteria_scores") + if isinstance(criteria, dict) and criteria: + return self._synthesize_from_criteria(criteria) + + return None + + def _synthesize_from_criteria(self, criteria: Dict[str, Any]) -> Optional[float]: + """Compute weighted importance score from criteria_scores dict.""" + weighted_sum = 0.0 + total_weight = 0.0 + + for key, weight in self.criteria_weights.items(): + raw = criteria.get(key) + # criteria_scores may be either flat floats or nested {"score": float} + if isinstance(raw, dict): + raw = raw.get("score") + if raw is None: + continue + try: + val = float(raw) + if 0.0 <= val <= 1.0: + weighted_sum += val * weight + total_weight += weight + except (TypeError, ValueError): + continue + + if total_weight == 0.0: + return None + + score = weighted_sum / total_weight + return max(0.0, min(1.0, score)) + + def _parse_importance_from_field_regex(self, response: str) -> Optional[float]: + """L2: Extract score only when anchored to a recognized field name.""" + patterns = [ + r'(?:importance_score|overall_score)\s*[":]\s*(\d+\.?\d*)', + r'(?:importance|score)\s*[:=]\s*(\d+\.?\d*)', + ] + for pattern in patterns: + match = re.search(pattern, response, re.IGNORECASE) + if match: + try: + score = float(match.group(1)) + if 0.0 <= score <= 1.0: + return score + except (ValueError, IndexError): + continue + return None def _evaluate_personal(self, content: str, metadata: Optional[Dict[str, Any]]) -> float: """Evaluate if content is personal.""" diff --git a/tests/unit/intelligence/test_importance_evaluator.py b/tests/unit/intelligence/test_importance_evaluator.py new file mode 100644 index 000000000..5b2ad3f78 --- /dev/null +++ b/tests/unit/intelligence/test_importance_evaluator.py @@ -0,0 +1,165 @@ +"""Unit tests for ImportanceEvaluator._parse_importance_response and fallback logic.""" + +import json +from unittest.mock import MagicMock, patch + +import pytest + +from powermem.intelligence.importance_evaluator import ImportanceEvaluator + + +@pytest.fixture +def evaluator(): + config = {} + llm_config = {} + ev = ImportanceEvaluator(config, llm_config) + return ev + + +class TestParseImportanceResponse: + """Tests for _parse_importance_response three-level fallback.""" + + def test_valid_json_with_importance_score(self, evaluator): + response = json.dumps({ + "importance_score": 0.75, + "reasoning": "High relevance", + "criteria_scores": { + "relevance": 0.8, "novelty": 0.6, + "emotional_impact": 0.5, "actionable": 0.7, + "factual": 0.6, "personal": 0.4 + } + }) + assert evaluator._parse_importance_response(response) == pytest.approx(0.75) + + def test_valid_json_with_overall_score(self, evaluator): + """detailed_importance_breakdown template uses 'overall_score'.""" + response = json.dumps({ + "overall_score": 0.62, + "reasoning": "Moderate", + "criteria_scores": {} + }) + assert evaluator._parse_importance_response(response) == pytest.approx(0.62) + + def test_json_only_criteria_scores_synthesizes_weighted(self, evaluator): + """When no total score field, synthesize from criteria_scores.""" + response = json.dumps({ + "reasoning": "No top-level score", + "criteria_scores": { + "relevance": 0.8, + "novelty": 0.6, + "emotional_impact": 0.4, + "actionable": 0.5, + "factual": 0.7, + "personal": 0.3 + } + }) + # Expected: 0.8*0.3 + 0.6*0.2 + 0.4*0.15 + 0.5*0.15 + 0.7*0.1 + 0.3*0.1 + # = 0.24 + 0.12 + 0.06 + 0.075 + 0.07 + 0.03 = 0.595 + result = evaluator._parse_importance_response(response) + assert result == pytest.approx(0.595) + + def test_json_criteria_scores_nested_format(self, evaluator): + """criteria_scores with nested {"score": float} format.""" + response = json.dumps({ + "reasoning": "detailed breakdown", + "criteria_scores": { + "relevance": {"score": 0.9, "reasoning": "very relevant"}, + "novelty": {"score": 0.5, "reasoning": "somewhat new"}, + "emotional_impact": {"score": 0.3, "reasoning": "low"}, + "actionable": {"score": 0.6, "reasoning": "usable"}, + "factual": {"score": 0.8, "reasoning": "verified"}, + "personal": {"score": 0.2, "reasoning": "general"} + } + }) + # 0.9*0.3 + 0.5*0.2 + 0.3*0.15 + 0.6*0.15 + 0.8*0.1 + 0.2*0.1 + # = 0.27 + 0.10 + 0.045 + 0.09 + 0.08 + 0.02 = 0.605 + result = evaluator._parse_importance_response(response) + assert result == pytest.approx(0.605) + + def test_malformed_json_with_field_name_regex(self, evaluator): + """Malformed JSON but field name pattern present — L2 extracts it.""" + response = 'I think the importance_score: 0.72 based on analysis...' + assert evaluator._parse_importance_response(response) == pytest.approx(0.72) + + def test_text_with_score_equals(self, evaluator): + """Alternative pattern: score = 0.65""" + response = 'After evaluation, the importance score = 0.65' + assert evaluator._parse_importance_response(response) == pytest.approx(0.65) + + def test_rejects_out_of_range_number_in_field_regex(self, evaluator): + """Number > 1.0 next to field name should be rejected.""" + response = 'importance_score: 85 (out of 100)' + assert evaluator._parse_importance_response(response) is None + + def test_rejects_number_slightly_above_one(self, evaluator): + """1.5 is outside [0, 1], should not clamp.""" + response = json.dumps({"importance_score": 1.5}) + assert evaluator._parse_importance_response(response) is None + + def test_reasoning_with_dimension_count_not_grabbed(self, evaluator): + """'6 dimensions' should not be treated as importance score.""" + response = 'I evaluated across 6 dimensions and found moderate relevance.' + assert evaluator._parse_importance_response(response) is None + + def test_percentage_expression_not_grabbed(self, evaluator): + """'85%' in text should not be misinterpreted.""" + response = 'The content appears to be about 85% factual with high novelty.' + assert evaluator._parse_importance_response(response) is None + + def test_threshold_reference_not_grabbed(self, evaluator): + """References to config thresholds should not be used as score.""" + response = 'Since the short_term threshold is 0.6, this seems moderate.' + assert evaluator._parse_importance_response(response) is None + + def test_completely_unparseable_returns_none(self, evaluator): + """Gibberish text returns None for safe fallback.""" + response = 'This content is interesting and noteworthy.' + assert evaluator._parse_importance_response(response) is None + + def test_empty_response_returns_none(self, evaluator): + assert evaluator._parse_importance_response("") is None + + def test_json_wrapped_in_markdown_code_block(self, evaluator): + """JSON inside ```json ... ``` should be parsed via parse_json_from_text.""" + payload = json.dumps({"importance_score": 0.88, "reasoning": "critical"}) + response = f"```json\n{payload}\n```" + assert evaluator._parse_importance_response(response) == pytest.approx(0.88) + + +class TestLLMBasedEvaluationFallback: + """Tests that _llm_based_evaluation falls back correctly on parse failure.""" + + def test_unparseable_response_falls_back_to_rule_based(self, evaluator): + mock_llm = MagicMock() + mock_llm.is_noop = False + mock_llm.generate_response.return_value = "No numbers or JSON here at all." + evaluator.set_llm(mock_llm) + + with patch.object(evaluator, '_rule_based_evaluation', return_value=0.35) as mock_rule: + result = evaluator._llm_based_evaluation("test content", None, None) + assert result == 0.35 + mock_rule.assert_called_once_with("test content", None, None) + + def test_valid_response_does_not_fallback(self, evaluator): + mock_llm = MagicMock() + mock_llm.is_noop = False + mock_llm.generate_response.return_value = json.dumps({ + "importance_score": 0.9, "reasoning": "critical" + }) + evaluator.set_llm(mock_llm) + + with patch.object(evaluator, '_rule_based_evaluation') as mock_rule: + result = evaluator._llm_based_evaluation("test content", None, None) + assert result == pytest.approx(0.9) + mock_rule.assert_not_called() + + def test_llm_exception_falls_back_to_rule_based(self, evaluator): + mock_llm = MagicMock() + mock_llm.is_noop = False + mock_llm.generate_response.side_effect = RuntimeError("API timeout") + evaluator.set_llm(mock_llm) + + with patch.object(evaluator, '_rule_based_evaluation', return_value=0.4) as mock_rule: + result = evaluator._llm_based_evaluation("test content", None, None) + assert result == 0.4 + mock_rule.assert_called_once()