diff --git a/openagent_eval/core/pipeline.py b/openagent_eval/core/pipeline.py index ac0e6c1..ab6e017 100644 --- a/openagent_eval/core/pipeline.py +++ b/openagent_eval/core/pipeline.py @@ -12,12 +12,15 @@ from __future__ import annotations import inspect +import logging from dataclasses import dataclass, field from typing import Any from openagent_eval.config.models import Config from openagent_eval.metrics.base import BaseMetric +logger = logging.getLogger(__name__) + @dataclass class EvaluationResult: @@ -193,7 +196,14 @@ def _supports_ground_truth_contexts(self) -> bool: try: sig = inspect.signature(retrieve_method) - except (ValueError, TypeError): + except (ValueError, TypeError) as e: + logger.warning( + "Could not inspect signature of %r (%s: %s); " + "assuming no ground_truth_contexts support", + retrieve_method, + type(e).__name__, + e, + ) self._retriever_supports_ground_truth_contexts = False return False @@ -209,9 +219,21 @@ def _supports_ground_truth_contexts(self) -> bool: return False async def _retrieve( - self, question: str, context: str | None, gt_contexts: list[str] + self, + question: str, + context: str | None, + gt_contexts: list[str], ) -> list[str]: - """Retrieve contexts for a question, or fall back to dataset context.""" + """Retrieve contexts for a question, or fall back to dataset context. + + A retrieval failure is logged (log-only) so a broken retriever is + distinguishable from a clean empty retrieval; the fallback behaviour + itself is unchanged. The failure is deliberately NOT recorded in + ``result.errors``: engine.py derives ``successful_evaluations`` from + ``len(results) - len(errors)``, and an entry here has no paired + placeholder result, so recording it would mis-count a healthy run + as failed. + """ if self._retriever is not None: try: if self._supports_ground_truth_contexts(): @@ -221,8 +243,15 @@ async def _retrieve( else: docs = await self._retriever.retrieve(question, k=self._k) return [doc.content for doc in docs] - except Exception: + except Exception as e: # Retrieval failure -> fall back to any dataset-provided context. + logger.warning( + "Retrieval failed for question %r (%s: %s); " + "falling back to dataset-provided context", + question, + type(e).__name__, + e, + ) if context: return [context] return [] diff --git a/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py b/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py new file mode 100644 index 0000000..3405e81 --- /dev/null +++ b/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py @@ -0,0 +1,95 @@ +"""Regression test for issue #256. + +Verifies that a retriever failure inside ``Pipeline._retrieve`` is no longer +silently swallowed: the dataset-context fallback still applies (behaviour +unchanged), and the failure is logged naming the exception. The degradation +is log-only: it must NOT be recorded in the run's ``errors``, because +engine.py computes ``successful_evaluations`` as ``len(results) - +len(errors)`` and an entry without a paired placeholder result would +mis-count a healthy run as failed (breaking cicd gate metrics). +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +import pytest + +from openagent_eval.config.models import ( + Config, + DatasetConfig, + LLMConfig, + MetricsConfig, + ReportConfig, + RetrieverConfig, +) +from openagent_eval.core.pipeline import Pipeline +from openagent_eval.providers.base.retriever import Retriever + +if TYPE_CHECKING: + from openagent_eval.providers.models import Document + + +class _FailingRetriever(Retriever): + """Retriever whose ``retrieve`` always raises, simulating a broken backend.""" + + name = "failing" + description = "Always raises from retrieve()" + + async def retrieve( + self, + query: str, + k: int = 5, + *, + ground_truth_contexts: list[str] | None = None, + ) -> list[Document]: + raise RuntimeError("vector store unreachable") + + +@pytest.mark.asyncio +async def test_retrieval_failure_is_visible_and_still_falls_back( + caplog: pytest.LogCaptureFixture, +) -> None: + """A raising retriever degrades to dataset context and is logged only.""" + from openagent_eval.providers.llm.mock import MockLLMProvider + + config = Config( + dataset=DatasetConfig(path="data/questions.json"), + llm=LLMConfig(provider="mock", model="mock-model"), + retriever=RetrieverConfig(provider="failing"), + metrics=MetricsConfig(), + report=ReportConfig(), + parallel=False, + ) + + pipeline = Pipeline( + config, retriever=_FailingRetriever(), llm=MockLLMProvider() + ) + + with caplog.at_level(logging.WARNING, logger="openagent_eval.core.pipeline"): + result = await pipeline.execute( + [ + { + "question": "What is RAG?", + "ground_truth": "RAG is retrieval augmented generation.", + "context": "dataset-provided context", + } + ] + ) + + # (a) Fallback behaviour is unchanged: the dataset context is still used. + assert result.results[0].contexts == ["dataset-provided context"] + + # (b) The degradation is log-only: it must NOT appear in result.errors. + # engine.py computes successful_evaluations as + # len(results) - len(errors), so an errors entry without a paired + # placeholder result would mis-count this healthy run as failed and + # trip cicd gate metrics. This pins that property. + assert result.errors == [] + assert len(result.results) == 1 + assert len(result.results) - len(result.errors) == 1 + + # (c) The failure is logged, naming the exception. + assert "RuntimeError" in caplog.text + assert "vector store unreachable" in caplog.text