From a9e6e89bb0ff0398839fc72984700bf3659dc4b1 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Fri, 31 Jul 2026 07:43:24 +0200 Subject: [PATCH 1/4] test: regression test for silent retrieval-failure swallow (#256) A retriever whose retrieve() raises currently degrades to the dataset-context fallback with nothing logged and nothing in the run's errors, making a broken retriever indistinguishable from a clean empty retrieval. This test asserts the fallback still applies while the failure is recorded in errors and logged. Fails on unfixed code. Co-Authored-By: Kimi K3 --- ...t_pipeline_retrieval_failure_visibility.py | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py 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..b840e1f --- /dev/null +++ b/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py @@ -0,0 +1,91 @@ +"""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), but the failure is now logged naming the exception and recorded +in the run's ``errors`` in the same shape as the other failure modes. +""" + +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 but is surfaced.""" + 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 failure is recorded in the run's errors, in the same shape the + # other failure modes use (item / error / error_type). + assert len(result.errors) == 1 + entry = result.errors[0] + assert entry["error_type"] == "RuntimeError" + assert "vector store unreachable" in entry["error"] + assert entry["item"]["question"] == "What is RAG?" + + # (c) The failure is logged, naming the exception. + assert "RuntimeError" in caplog.text + assert "vector store unreachable" in caplog.text From c83d220593be2fb701ecc46c7cbb46ecf508747c Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Fri, 31 Jul 2026 07:43:24 +0200 Subject: [PATCH 2/4] fix: surface retrieval failures in logs and run errors (#256) Pipeline._retrieve swallowed every exception in a bare except and silently degraded to the dataset-context fallback, making a broken retriever indistinguishable from a clean empty retrieval. The fallback behaviour is unchanged; the failure is now logged naming the exception and recorded in the run's errors in the same shape used by the per-item failure path. Also log when inspect.signature fails and the retriever is assumed to lack ground_truth_contexts support. Co-Authored-By: Kimi K3 --- openagent_eval/core/pipeline.py | 44 +++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/openagent_eval/core/pipeline.py b/openagent_eval/core/pipeline.py index ac0e6c1..f39b016 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: @@ -114,7 +117,7 @@ async def _evaluate_item( try: # 1. Retrieval - contexts = await self._retrieve(question, context, gt_contexts) + contexts = await self._retrieve(question, context, gt_contexts, item, result) # 2. Generation answer, token_usage, latency_ms = await self._generate( @@ -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,19 @@ 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], + item: dict[str, Any], + result: PipelineResult, ) -> 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 and recorded in ``result.errors`` so a + broken retriever is distinguishable from a clean empty retrieval; the + fallback behaviour itself is unchanged. + """ if self._retriever is not None: try: if self._supports_ground_truth_contexts(): @@ -221,8 +241,22 @@ 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, + ) + result.errors.append( + { + "item": {k: v for k, v in item.items() if k != "metadata"}, + "error": str(e), + "error_type": type(e).__name__, + } + ) if context: return [context] return [] From 6e5be9e8dac7f131410129b13bec3c053fd60353 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Fri, 31 Jul 2026 08:10:29 +0200 Subject: [PATCH 3/4] fix: make retrieval-failure surfacing log-only (#256) Recording the non-fatal retrieval degradation in result.errors broke engine.py's summary arithmetic: successful_evaluations is computed as len(results) - len(errors), and the retrieval-failure entry has no paired placeholder result, so a healthy run was mis-counted as failed. That polluted the pipeline summary, every report renderer's failure section, the CLI error count, and the cicd plugin gate metrics, failing CI for users gating on failed_evaluations == 0. Remove the result.errors append; the failure is still logged naming the exception and the dataset-context fallback is unchanged. The regression test now pins the log-only contract: it asserts result.errors is empty and the run still counts as successful, which catches any future change that reintroduces the errors entry. Co-Authored-By: Kimi K3 --- openagent_eval/core/pipeline.py | 17 ++++++------- ...t_pipeline_retrieval_failure_visibility.py | 24 +++++++++++-------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/openagent_eval/core/pipeline.py b/openagent_eval/core/pipeline.py index f39b016..92ec820 100644 --- a/openagent_eval/core/pipeline.py +++ b/openagent_eval/core/pipeline.py @@ -228,9 +228,13 @@ async def _retrieve( ) -> list[str]: """Retrieve contexts for a question, or fall back to dataset context. - A retrieval failure is logged and recorded in ``result.errors`` so a - broken retriever is distinguishable from a clean empty retrieval; the - fallback behaviour itself is unchanged. + 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: @@ -250,13 +254,6 @@ async def _retrieve( type(e).__name__, e, ) - result.errors.append( - { - "item": {k: v for k, v in item.items() if k != "metadata"}, - "error": str(e), - "error_type": type(e).__name__, - } - ) 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 index b840e1f..3405e81 100644 --- a/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py +++ b/tests/unit/test_core/test_pipeline_retrieval_failure_visibility.py @@ -2,8 +2,11 @@ Verifies that a retriever failure inside ``Pipeline._retrieve`` is no longer silently swallowed: the dataset-context fallback still applies (behaviour -unchanged), but the failure is now logged naming the exception and recorded -in the run's ``errors`` in the same shape as the other failure modes. +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 @@ -48,7 +51,7 @@ async def retrieve( async def test_retrieval_failure_is_visible_and_still_falls_back( caplog: pytest.LogCaptureFixture, ) -> None: - """A raising retriever degrades to dataset context but is surfaced.""" + """A raising retriever degrades to dataset context and is logged only.""" from openagent_eval.providers.llm.mock import MockLLMProvider config = Config( @@ -78,13 +81,14 @@ async def test_retrieval_failure_is_visible_and_still_falls_back( # (a) Fallback behaviour is unchanged: the dataset context is still used. assert result.results[0].contexts == ["dataset-provided context"] - # (b) The failure is recorded in the run's errors, in the same shape the - # other failure modes use (item / error / error_type). - assert len(result.errors) == 1 - entry = result.errors[0] - assert entry["error_type"] == "RuntimeError" - assert "vector store unreachable" in entry["error"] - assert entry["item"]["question"] == "What is RAG?" + # (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 From 76edb5a6d616992f6627cb9cd2bd02abd4d9e5e3 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Fri, 31 Jul 2026 08:21:30 +0200 Subject: [PATCH 4/4] refactor: drop dead item/result params from _retrieve (#256) Leftover from the log-only rework: the item and result parameters were added only to support the result.errors append, which was removed when the fix became log-only. Nothing in the body uses either parameter, so revert _retrieve to its original (question, context, gt_contexts) signature and the call site to the original three arguments. Logging, fallback behaviour, and the docstring rationale are unchanged. Co-Authored-By: Kimi K3 --- openagent_eval/core/pipeline.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openagent_eval/core/pipeline.py b/openagent_eval/core/pipeline.py index 92ec820..ab6e017 100644 --- a/openagent_eval/core/pipeline.py +++ b/openagent_eval/core/pipeline.py @@ -117,7 +117,7 @@ async def _evaluate_item( try: # 1. Retrieval - contexts = await self._retrieve(question, context, gt_contexts, item, result) + contexts = await self._retrieve(question, context, gt_contexts) # 2. Generation answer, token_usage, latency_ms = await self._generate( @@ -223,8 +223,6 @@ async def _retrieve( question: str, context: str | None, gt_contexts: list[str], - item: dict[str, Any], - result: PipelineResult, ) -> list[str]: """Retrieve contexts for a question, or fall back to dataset context.