From 8ab4572d107c21d64b17610263c5d1f46d757445 Mon Sep 17 00:00:00 2001 From: guangyu-reflexio Date: Fri, 11 Sep 2026 23:50:23 -0700 Subject: [PATCH] fix(tests): stop a live-provider eval test from grading the mock Five tests under `tests/eval/` build a real `LiteLLMClient` and say so -- "Real end-to-end smoke: live extractors + real LLM", "Smoke test against a real judge model". None of them can reach a provider. Per `.claude/rules/testing-conventions.md` a live test does that only if it lives under `tests/e2e_tests/` AND carries `requires_credentials`; these satisfy neither, so `_is_e2e_test_run` installs the session-wide `litellm.completion` patch and they talk to canned text. Measured with `RUN_LOW_PRIORITY=1`, which is the only way these ever run: before: 4 failed, 44 passed after: 5 failed, 43 passed The single test that flipped is `test_live_extraction_provider_real`. It was PASSING -- reporting a green live-LLM smoke while grading mock output. That is the bug. The other four were already failing, but for a reason that named the wrong thing: `StructuredOutputParseError` / `StructuredOutputRepairError`, i.e. "the canned response does not fit the schema", when the actual problem is that a canned response is being used at all. Each now calls `assert_litellm_unpatched()` -- the backstop this repo already ships for exactly this, and which the e2e live tests already use. One false pass becomes an honest failure; four confusing failures name their real cause: AssertionError: litellm.completion is patched, so this live-provider test would assert against canned mock text. Live tests belong under tests/e2e_tests/ and must carry the requires_credentials marker; the e2e conftest lifts the session patch for those. Not moved under `tests/e2e_tests/`, which is the other half of the documented recipe and would make them actually run: they depend on `tests/eval/`'s own fixtures and providers, so relocating them is a larger change than making them honest. They remain `@skip_low_priority`, so the default suite is unaffected -- `tests/eval` is 93 passed, 5 skipped, and the full OSS suite 6220 passed. --- tests/eval/consolidation/test_consolidation_eval.py | 9 +++++++++ tests/eval/extraction/test_extraction_eval.py | 9 +++++++++ tests/eval/scenarios/test_scenario_eval.py | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/tests/eval/consolidation/test_consolidation_eval.py b/tests/eval/consolidation/test_consolidation_eval.py index 8b4a88d77..c9f85a230 100644 --- a/tests/eval/consolidation/test_consolidation_eval.py +++ b/tests/eval/consolidation/test_consolidation_eval.py @@ -21,6 +21,7 @@ RejectNewDecision, UnifyDecision, ) +from reflexio.test_support.llm_mock import assert_litellm_unpatched from reflexio.test_support.skip_decorators import skip_low_priority from tests.eval.consolidation.case import ( ConsolidationEvalCase, @@ -519,6 +520,10 @@ def test_live_consolidation_provider_real(tmp_path): # pragma: no cover - manua the fixture. Asserts only pipeline mechanics (every case produced one of the four kinds), never exact kinds. Run manually with API keys + RUN_LOW_PRIORITY=1.""" + # This test lives outside ``tests/e2e_tests/``, so the session-wide + # ``litellm.completion`` patch is installed for it. Without this it would + # grade canned mock text while reporting a real model. + assert_litellm_unpatched() from reflexio.server.api_endpoints.request_context import RequestContext from reflexio.server.llm.litellm_client import LiteLLMClient, LiteLLMConfig @@ -544,6 +549,10 @@ def test_live_consolidation_provider_real(tmp_path): # pragma: no cover - manua @skip_low_priority def test_real_judge_smoke(): # pragma: no cover - manual, costs money """Smoke test against a real judge model. Run manually with API keys.""" + # This test lives outside ``tests/e2e_tests/``, so the session-wide + # ``litellm.completion`` patch is installed for it. Without this it would + # grade canned mock text while reporting a real model. + assert_litellm_unpatched() from reflexio.server.llm.litellm_client import LiteLLMClient, LiteLLMConfig client = LiteLLMClient(LiteLLMConfig(model="claude-haiku-4-5")) diff --git a/tests/eval/extraction/test_extraction_eval.py b/tests/eval/extraction/test_extraction_eval.py index 5a6481fc0..7c1a8df34 100644 --- a/tests/eval/extraction/test_extraction_eval.py +++ b/tests/eval/extraction/test_extraction_eval.py @@ -14,6 +14,7 @@ import pytest from pydantic import BaseModel +from reflexio.test_support.llm_mock import assert_litellm_unpatched from reflexio.test_support.skip_decorators import skip_low_priority from tests.eval.conftest import _load, _load_rubric from tests.eval.extraction.providers import make_extraction_provider @@ -180,6 +181,10 @@ def test_score_golden_case(extraction_case, extraction_judge): @skip_low_priority def test_real_judge_smoke(): # pragma: no cover - manual, costs money """Smoke test against a real judge model. Run manually with API keys.""" + # This test lives outside ``tests/e2e_tests/``, so the session-wide + # ``litellm.completion`` patch is installed for it. Without this it would + # grade canned mock text while reporting a real model. + assert_litellm_unpatched() from reflexio.server.llm.litellm_client import LiteLLMClient, LiteLLMConfig rubric = _load_rubric("extraction_rubric.yaml") @@ -298,6 +303,10 @@ def test_live_extraction_provider_real(tmp_path): # pragma: no cover - manual in range), never exact scores. Run manually with API keys + RUN_LOW_PRIORITY=1. """ + # This test lives outside ``tests/e2e_tests/``, so the session-wide + # ``litellm.completion`` patch is installed for it. Without this it would + # grade canned mock text while reporting a real model. + assert_litellm_unpatched() from reflexio.server.api_endpoints.request_context import RequestContext from reflexio.server.llm.litellm_client import LiteLLMClient, LiteLLMConfig diff --git a/tests/eval/scenarios/test_scenario_eval.py b/tests/eval/scenarios/test_scenario_eval.py index 9cf4e85a4..93e11f62a 100644 --- a/tests/eval/scenarios/test_scenario_eval.py +++ b/tests/eval/scenarios/test_scenario_eval.py @@ -14,6 +14,7 @@ RejectNewDecision, UnifyDecision, ) +from reflexio.test_support.llm_mock import assert_litellm_unpatched from reflexio.test_support.skip_decorators import skip_low_priority from tests.eval.consolidation.judge import ConsolidationVerdict from tests.eval.scenarios.book import _next_id, apply_consolidation @@ -172,6 +173,10 @@ def test_wrong_consolidation_verdict_fails_scenario() -> None: @skip_low_priority def test_scenario_real(tmp_path) -> None: # pragma: no cover - manual, costs money + # This test lives outside ``tests/e2e_tests/``, so the session-wide + # ``litellm.completion`` patch is installed for it. Without this it would + # grade canned mock text while reporting a real model. + assert_litellm_unpatched() from reflexio.server.api_endpoints.request_context import RequestContext from reflexio.server.llm.litellm_client import LiteLLMClient, LiteLLMConfig from tests.eval.consolidation.providers import (