Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ HINDSIGHT_API_LOG_LEVEL=info
# Reranker Configuration (Optional - uses local by default)
# Provider: "local" (default) or "tei" (HuggingFace Text Embeddings Inference)
# HINDSIGHT_API_RERANKER_PROVIDER=local
# Fail recall when reranker initialization or scoring fails (default). Set to
# false to return RRF-ranked results and mark the trace as degraded instead.
# HINDSIGHT_API_RERANKER_REQUIRED=true
# Trusted gateway attribution (disabled by default). When enabled, remote
# reranker requests include X-Hindsight-Bank-Id with the current bank ID.
# HINDSIGHT_API_RERANKER_SEND_BANK_AS_HEADER=false
Expand Down
4 changes: 4 additions & 0 deletions hindsight-api-slim/hindsight_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ def _parse_boolean_env(env_name: str, default: bool) -> bool:
ENV_LITELLM_API_KEY = "HINDSIGHT_API_LITELLM_API_KEY"

ENV_RERANKER_PROVIDER = "HINDSIGHT_API_RERANKER_PROVIDER"
ENV_RERANKER_REQUIRED = "HINDSIGHT_API_RERANKER_REQUIRED"
ENV_RERANKER_SEND_BANK_AS_HEADER = "HINDSIGHT_API_RERANKER_SEND_BANK_AS_HEADER"
ENV_RERANKER_LOCAL_MODEL = "HINDSIGHT_API_RERANKER_LOCAL_MODEL"
ENV_RERANKER_LOCAL_FORCE_CPU = "HINDSIGHT_API_RERANKER_LOCAL_FORCE_CPU"
Expand Down Expand Up @@ -900,6 +901,7 @@ def _parse_worker_slot_reservations() -> dict[str, int]:
DEFAULT_EMBEDDING_DIMENSION = 384

DEFAULT_RERANKER_PROVIDER = "local"
DEFAULT_RERANKER_REQUIRED = True
DEFAULT_RERANKER_SEND_BANK_AS_HEADER = False
DEFAULT_RERANKER_LOCAL_MODEL = "cross-encoder/ms-marco-MiniLM-L-6-v2"
DEFAULT_RERANKER_LOCAL_FORCE_CPU = False # Force CPU mode for local reranker
Expand Down Expand Up @@ -1948,6 +1950,7 @@ class HindsightConfig:

# Reranker
reranker_provider: str
reranker_required: bool = field(default=DEFAULT_RERANKER_REQUIRED, kw_only=True)
reranker_send_bank_as_header: bool
reranker_local_model: str
reranker_local_force_cpu: bool
Expand Down Expand Up @@ -2905,6 +2908,7 @@ def from_env(cls) -> "HindsightConfig":
or os.getenv(ENV_LLM_VERTEXAI_SERVICE_ACCOUNT_KEY),
# Reranker
reranker_provider=os.getenv(ENV_RERANKER_PROVIDER, DEFAULT_RERANKER_PROVIDER),
reranker_required=_parse_boolean_env(ENV_RERANKER_REQUIRED, DEFAULT_RERANKER_REQUIRED),
reranker_send_bank_as_header=os.getenv(
ENV_RERANKER_SEND_BANK_AS_HEADER,
str(DEFAULT_RERANKER_SEND_BANK_AS_HEADER),
Expand Down
75 changes: 64 additions & 11 deletions hindsight-api-slim/hindsight_api/engine/memory_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
DEFAULT_REFLECT_SOURCE_FACTS_MAX_TOKENS,
DEFAULT_STORE_DOCUMENT_TEXT,
ENV_MODEL_INIT_TIMEOUT,
ENV_RERANKER_REQUIRED,
HindsightConfig,
LLMMemberConfig,
LLMStrategyConfig,
Expand Down Expand Up @@ -5286,6 +5287,8 @@ def to_tuple_format(results):
scored_results: list = []
pre_filtered_count = 0
rerank_kind = "cross-encoder"
reranker_degraded = False
reranker_error_type: str | None = None
try:
# Pre-filter candidates by RRF before the (optional) cross-encoder.
# RRF already provides good ranking; this caps cross-encoder cost.
Expand All @@ -5311,9 +5314,37 @@ def to_tuple_format(results):
if request_context is not None:
request_context.raise_if_cancelled()

# Ensure reranker is initialized (for lazy initialization mode)
await reranker_instance.ensure_initialized()
scored_results = await reranker_instance.rerank(query, merged_candidates)
try:
# Ensure reranker is initialized (for lazy initialization mode)
await reranker_instance.ensure_initialized()
scored_results = await reranker_instance.rerank(query, merged_candidates)
except OperationCancelledError:
raise
except Exception as e:
if get_config().reranker_required:
raise

# The retrieval candidates are already available, so an optional
# refinement failure should reduce ranking quality instead of
# discarding the recall. Do not log the exception message because
# remote providers may include query or document content in it.
reranker_degraded = True
reranker_error_type = type(e).__name__
rerank_kind = "rrf-fallback"
logger.warning(
"Reranker failed with %s; falling back to RRF ordering because %s=false",
reranker_error_type,
ENV_RERANKER_REQUIRED,
)
scored_results = [
ScoredResult(
candidate=mc,
cross_encoder_score=0.0,
cross_encoder_score_normalized=0.0,
weight=0.0,
)
for mc in sorted(merged_candidates, key=lambda mc: mc.rrf_score, reverse=True)
]
else:
# "rrf" / "interleave": skip the cross-encoder and keep the fusion order
# (rrf_score is descending by fusion position for both). The cross-encoder
Expand All @@ -5339,6 +5370,10 @@ def to_tuple_format(results):
)
finally:
rerank_span.set_attribute("hindsight.scored_count", len(scored_results))
rerank_span.set_attribute("hindsight.reranker_type", rerank_kind)
rerank_span.set_attribute("hindsight.reranker_degraded", reranker_degraded)
if reranker_error_type is not None:
rerank_span.set_attribute("hindsight.reranker_error_type", reranker_error_type)
if pre_filtered_count > 0:
rerank_span.set_attribute("hindsight.pre_filtered_count", pre_filtered_count)
rerank_span.end()
Expand All @@ -5365,7 +5400,9 @@ def to_tuple_format(results):
elif scored_results:
ce = reranker_instance.cross_encoder
# "rrf" mode is passthrough by construction; so is a configured "rrf" CE.
is_passthrough = (reranking == "rrf") or (ce is not None and ce.provider_name == "rrf")
is_passthrough = (
reranker_degraded or (reranking == "rrf") or (ce is not None and ce.provider_name == "rrf")
)
scoring_config = get_config()
apply_combined_scoring(
scored_results,
Expand Down Expand Up @@ -5397,8 +5434,16 @@ def to_tuple_format(results):
# threshold is a no-op. There is deliberately no default — the
# cross-encoder's absolute scores are not calibrated for a fixed cutoff
# (a clearly-relevant match can score ~0.001 while its *ranking* is right).
min_reranker = min_scores.reranker if min_scores else None
requested_min_reranker = min_scores.reranker if min_scores else None
# A degraded recall has no real reranker score to compare. Applying
# the requested floor to the RRF-derived placeholder would turn the
# fail-open path back into an empty response.
min_reranker = None if reranker_degraded else requested_min_reranker
min_final = min_scores.final if min_scores else None
if reranker_degraded and requested_min_reranker is not None:
log_buffer.append(
f" [4.9] min_scores.reranker={requested_min_reranker} skipped because reranking degraded"
)
if (min_reranker is not None or min_final is not None) and scored_results:
before_min_score = len(scored_results)
scored_results = [
Expand All @@ -5423,7 +5468,13 @@ def to_tuple_format(results):
tracer.add_phase_metric(
"reranking",
step_duration,
{"reranker_type": rerank_kind, "candidates_reranked": len(scored_results)},
{
"reranker_type": rerank_kind,
"candidates_reranked": len(scored_results),
"degraded": reranker_degraded,
"error_type": reranker_error_type,
"reranker_min_score_skipped": reranker_degraded and requested_min_reranker is not None,
},
)
# Combined scoring + additive boosts + final sort, plus the trace
# serialization of reranked entries done just above.
Expand Down Expand Up @@ -5969,12 +6020,14 @@ def _make_source_fact(sid: str, r: Any) -> MemoryFact:
# Convert results to MemoryFact objects
# Build per-result scores (final/reranker/semantic/text) keyed by id.
# reranker is None when the configured reranker is a passthrough (rrf /
# interleave modes, or the RRFPassthroughCrossEncoder), since its
# cross_encoder_score_normalized is then a rank-derived placeholder, not a
# true relevance score.
# interleave modes, or the RRFPassthroughCrossEncoder) or the optional
# reranker degraded to RRF, since cross_encoder_score_normalized is then a
# rank-derived placeholder rather than a true relevance score.
ce_model = self._cross_encoder_reranker.cross_encoder
reranker_passthrough = (reranking != "cross_encoder") or (
ce_model is not None and getattr(ce_model, "provider_name", None) == "rrf"
reranker_passthrough = (
reranker_degraded
or (reranking != "cross_encoder")
or (ce_model is not None and getattr(ce_model, "provider_name", None) == "rrf")
)
scores_by_id: dict[str, RecallScores] = {
sr.id: RecallScores(
Expand Down
173 changes: 161 additions & 12 deletions hindsight-api-slim/tests/test_reranker_error_handling.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,62 @@
"""
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.
"""
"""Regression coverage for reranker failures during recall."""

import logging
from datetime import datetime, timezone
from unittest.mock import AsyncMock, patch
from unittest.mock import AsyncMock, MagicMock, patch

import pytest

from hindsight_api.cancellation import OperationCancelledError
from hindsight_api.config import (
DEFAULT_RERANKER_REQUIRED,
ENV_RERANKER_REQUIRED,
HindsightConfig,
clear_config_cache,
)
from hindsight_api.engine.response_models import MinScores


@pytest.fixture(autouse=True)
def _strict_reranker_by_default(monkeypatch):
"""Keep module tests isolated from local env and the process-wide config cache."""
monkeypatch.setenv(ENV_RERANKER_REQUIRED, "true")
clear_config_cache()
yield
clear_config_cache()


def test_reranker_required_defaults_on(monkeypatch):
monkeypatch.delenv(ENV_RERANKER_REQUIRED, raising=False)

assert HindsightConfig.from_env().reranker_required is DEFAULT_RERANKER_REQUIRED is True


def test_reranker_required_can_be_disabled(monkeypatch):
monkeypatch.setenv(ENV_RERANKER_REQUIRED, "false")

assert HindsightConfig.from_env().reranker_required is False


def test_reranker_required_defaults_for_legacy_constructor_input(monkeypatch):
monkeypatch.delenv(ENV_RERANKER_REQUIRED, raising=False)
legacy_values = vars(HindsightConfig.from_env()).copy()
legacy_values.pop("reranker_required")

assert HindsightConfig(**legacy_values).reranker_required is True


@pytest.mark.parametrize("value", ["", "yes", "tru", "disabled"])
def test_reranker_required_rejects_ambiguous_values(monkeypatch, value):
monkeypatch.setenv(ENV_RERANKER_REQUIRED, value)

with pytest.raises(ValueError, match=ENV_RERANKER_REQUIRED):
HindsightConfig.from_env()


def test_reranker_required_is_static_server_config():
assert "reranker_required" in HindsightConfig.get_static_fields()
assert "reranker_required" not in HindsightConfig.get_configurable_fields()


@pytest.mark.asyncio
async def test_recall_reranker_error_does_not_raise_unbound_local(memory, request_context):
Expand Down Expand Up @@ -44,6 +86,113 @@ async def test_recall_reranker_error_does_not_raise_unbound_local(memory, reques
await memory.delete_bank(bank_id, request_context=request_context)


@pytest.mark.asyncio
@pytest.mark.parametrize("failure_point", ["ensure_initialized", "rerank"])
async def test_optional_reranker_failure_falls_back_to_rrf(
memory,
request_context,
monkeypatch,
caplog,
failure_point,
):
bank_id = f"test_reranker_fallback_{failure_point}_{datetime.now(timezone.utc).timestamp()}"

try:
for content in (
"Paris is the capital of France",
"Lyon is a large city in France",
"Berlin is the capital of Germany",
):
await memory.retain_async(bank_id=bank_id, content=content, request_context=request_context)

question_date = datetime.now(timezone.utc)
rrf_result = await memory.recall_async(
bank_id=bank_id,
query="cities in France",
question_date=question_date,
reranking="rrf",
request_context=request_context,
)
assert rrf_result.results

monkeypatch.setenv(ENV_RERANKER_REQUIRED, "false")
clear_config_cache()
caplog.clear()
caplog.set_level(logging.WARNING, logger="hindsight_api.engine.memory_engine")

reranker = memory._cross_encoder_reranker
reranker._initialized = failure_point == "rerank"
failure = AsyncMock(side_effect=RuntimeError("private reranker payload"))
otel_span = MagicMock()
otel_tracer = MagicMock()
otel_tracer.start_span.return_value = otel_span

with (
patch.object(reranker, failure_point, failure),
patch("hindsight_api.tracing.get_tracer", return_value=otel_tracer),
):
degraded_result = await memory.recall_async(
bank_id=bank_id,
query="cities in France",
question_date=question_date,
enable_trace=True,
min_scores=MinScores(reranker=1.0),
request_context=request_context,
)

assert [result.id for result in degraded_result.results] == [result.id for result in rrf_result.results]
assert all(result.scores is not None and result.scores.reranker is None for result in degraded_result.results)

assert degraded_result.trace is not None
reranking_phase = next(
phase for phase in degraded_result.trace["summary"]["phase_metrics"] if phase["phase_name"] == "reranking"
)
assert reranking_phase["details"] == {
"reranker_type": "rrf-fallback",
"candidates_reranked": len(degraded_result.results),
"degraded": True,
"error_type": "RuntimeError",
"reranker_min_score_skipped": True,
}

span_attributes = {call.args[0]: call.args[1] for call in otel_span.set_attribute.call_args_list}
assert span_attributes["hindsight.reranker_type"] == "rrf-fallback"
assert span_attributes["hindsight.reranker_degraded"] is True
assert span_attributes["hindsight.reranker_error_type"] == "RuntimeError"

assert f"because {ENV_RERANKER_REQUIRED}=false" in caplog.text
assert "private reranker payload" not in caplog.text
finally:
await memory.delete_bank(bank_id, request_context=request_context)


@pytest.mark.asyncio
async def test_optional_reranker_does_not_swallow_cancellation(memory, request_context, monkeypatch):
bank_id = f"test_reranker_cancel_{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,
)

monkeypatch.setenv(ENV_RERANKER_REQUIRED, "false")
clear_config_cache()
memory._cross_encoder_reranker._initialized = True
rerank_mock = AsyncMock(side_effect=OperationCancelledError("client disconnected"))

with patch.object(memory._cross_encoder_reranker, "rerank", rerank_mock):
with pytest.raises(OperationCancelledError, match="client disconnected"):
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)."""
Expand Down
1 change: 1 addition & 0 deletions hindsight-docs/docs/developer/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,7 @@ ZeroEntropy's `zembed-1` supports Matryoshka dimensions: `2560`, `1280`, `640`,
| Variable | Description | Default |
|----------|-------------|---------|
| `HINDSIGHT_API_RERANKER_PROVIDER` | Provider: `local`, `tei`, `cohere`, `openrouter`, `zeroentropy`, `siliconflow`, `alibaba`, `google`, `flashrank`, `litellm`, `litellm-sdk`, `jina-mlx`, or `rrf` | `local` |
| `HINDSIGHT_API_RERANKER_REQUIRED` | Whether reranker initialization or scoring failures fail the recall. Set to `false` to fall back to RRF-based ranking, return `null` reranker scores, and mark traces as degraded. During fallback, a requested `min_scores.reranker` floor is skipped because no reranker score exists. | `true` |
| `HINDSIGHT_API_RERANKER_SEND_BANK_AS_HEADER` | Add `X-Hindsight-Bank-Id: <bank_id>` to remote reranker requests. Enable only for trusted endpoints because this transmits the current bank ID. Covers TEI, Cohere-compatible HTTP, LiteLLM proxy, and LiteLLM SDK transports. | `false` |
| `HINDSIGHT_API_RERANKER_LOCAL_MODEL` | Model for local provider | `cross-encoder/ms-marco-MiniLM-L-6-v2` |
| `HINDSIGHT_API_RERANKER_LOCAL_MAX_CONCURRENT` | Max concurrent local reranking (prevents CPU thrashing under load) | `4` |
Expand Down
3 changes: 3 additions & 0 deletions hindsight-embed/hindsight_embed/env.example
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ HINDSIGHT_API_LOG_LEVEL=info
# Reranker Configuration (Optional - uses local by default)
# Provider: "local" (default) or "tei" (HuggingFace Text Embeddings Inference)
# HINDSIGHT_API_RERANKER_PROVIDER=local
# Fail recall when reranker initialization or scoring fails (default). Set to
# false to return RRF-ranked results and mark the trace as degraded instead.
# HINDSIGHT_API_RERANKER_REQUIRED=true
# Trusted gateway attribution (disabled by default). When enabled, remote
# reranker requests include X-Hindsight-Bank-Id with the current bank ID.
# HINDSIGHT_API_RERANKER_SEND_BANK_AS_HEADER=false
Expand Down
Loading