From 58c4d65778dd93ad06ccc0bcd94579bb4be5d824 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=B2=20Boschi?= <boschi1997@gmail.com>
Date: Thu, 19 Feb 2026 11:38:37 +0100
Subject: [PATCH] fix: reranker crashes on provider error (#403)

* fix: reranker crashes on provider error

* fix: reranker crashes on provider error
---
 .../hindsight_api/engine/memory_engine.py     |  3 +-
 .../tests/test_reranker_error_handling.py     | 71 +++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 hindsight-api/tests/test_reranker_error_handling.py

diff --git a/hindsight-api/hindsight_api/engine/memory_engine.py b/hindsight-api/hindsight_api/engine/memory_engine.py
index 6b865e50..6e592413 100644
--- a/hindsight-api/hindsight_api/engine/memory_engine.py
+++ b/hindsight-api/hindsight_api/engine/memory_engine.py
@@ -2628,6 +2628,8 @@ class MemoryEngine(MemoryEngineInterface):
             rerank_span.set_attribute("hindsight.bank_id", bank_id)
             rerank_span.set_attribute("hindsight.candidates_count", len(merged_candidates))
 
+            scored_results: list = []
+            pre_filtered_count = 0
             try:
                 # Ensure reranker is initialized (for lazy initialization mode)
                 await reranker_instance.ensure_initialized()
@@ -2635,7 +2637,6 @@ class MemoryEngine(MemoryEngineInterface):
                 # Pre-filter candidates to reduce reranking cost (RRF already provides good ranking)
                 # This is especially important for remote rerankers with network latency
                 reranker_max_candidates = get_config().reranker_max_candidates
-                pre_filtered_count = 0
                 if len(merged_candidates) > reranker_max_candidates:
                     # Sort by RRF score and take top candidates
                     merged_candidates.sort(key=lambda mc: mc.rrf_score, reverse=True)
diff --git a/hindsight-api/tests/test_reranker_error_handling.py b/hindsight-api/tests/test_reranker_error_handling.py
new file mode 100644
index 00000000..b7b4cbdc
--- /dev/null
+++ b/hindsight-api/tests/test_reranker_error_handling.py
@@ -0,0 +1,71 @@
+"""
+Regression test for UnboundLocalError in recall when the reranker raises.
+
+Before the fix, `scored_results` and `pre_filtered_count` were only assigned
+inside the `try` block, but referenced in the `finally` block.  If
+`reranker_instance.rerank()` (or `ensure_initialized()`) raised, the `finally`
+block crashed with `UnboundLocalError` instead of propagating the original
+exception.
+
+Fix: initialise both variables to safe defaults before the try/finally block.
+"""
+
+from datetime import datetime, timezone
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+
+@pytest.mark.asyncio
+async def test_recall_reranker_error_does_not_raise_unbound_local(memory, request_context):
+    """Recall must propagate the reranker's exception, not an UnboundLocalError."""
+    bank_id = f"test_reranker_err_{datetime.now(timezone.utc).timestamp()}"
+
+    try:
+        await memory.retain_async(
+            bank_id=bank_id,
+            content="Paris is the capital of France",
+            request_context=request_context,
+        )
+
+        # Simulate a reranker failure (e.g. Cohere API error on empty/small candidate set)
+        rerank_mock = AsyncMock(side_effect=RuntimeError("reranker API error"))
+        memory._cross_encoder_reranker._initialized = True  # skip ensure_initialized
+
+        with patch.object(memory._cross_encoder_reranker, "rerank", rerank_mock):
+            with pytest.raises(Exception, match="reranker API error"):
+                await memory.recall_async(
+                    bank_id=bank_id,
+                    query="capital of France",
+                    request_context=request_context,
+                )
+
+    finally:
+        await memory.delete_bank(bank_id, request_context=request_context)
+
+
+@pytest.mark.asyncio
+async def test_recall_reranker_init_error_does_not_raise_unbound_local(memory, request_context):
+    """Same regression when ensure_initialized() raises (before pre_filtered_count is set)."""
+    bank_id = f"test_reranker_init_err_{datetime.now(timezone.utc).timestamp()}"
+
+    try:
+        await memory.retain_async(
+            bank_id=bank_id,
+            content="Paris is the capital of France",
+            request_context=request_context,
+        )
+
+        init_mock = AsyncMock(side_effect=RuntimeError("reranker init failed"))
+        memory._cross_encoder_reranker._initialized = False
+
+        with patch.object(memory._cross_encoder_reranker, "ensure_initialized", init_mock):
+            with pytest.raises(Exception, match="reranker init failed"):
+                await memory.recall_async(
+                    bank_id=bank_id,
+                    query="capital of France",
+                    request_context=request_context,
+                )
+
+    finally:
+        await memory.delete_bank(bank_id, request_context=request_context)