Skip to content

Commit 7983cd1

Browse files
[OPIK-8012] [SDK] fix: bound LLM judge provider calls and retry empty responses
An empty structured-output response raises BaseLLMError, which the judge's retry policy did not cover (LLMJudgeParseError extends OpikException, not BaseLLMError), so a transient blank cost the item its whole score. LiteLLMChatModel also forwarded no timeout and no num_retries, leaving litellm's 6000s default plus its ~3 internal HTTP retries stacked under two tenacity layers — a stalled provider blocked effectively forever. Observed stalls sit in TCP connect, hence the tighter connect ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e50e4a5 commit 7983cd1

5 files changed

Lines changed: 132 additions & 4 deletions

File tree

sdks/python/src/opik/evaluation/models/litellm/litellm_chat_model.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import tenacity
88

99
if TYPE_CHECKING:
10+
import httpx
1011
from litellm.types.utils import ModelResponse
1112

1213
import opik.semantic_version as semantic_version
@@ -18,6 +19,22 @@
1819

1920
LOGGER = logging.getLogger(__name__)
2021

22+
# LiteLLM defaults to a 6000s timeout and retries internally, which multiplies
23+
# against the tenacity retries here and in the LLM-judge metric. Connect is
24+
# tighter than read: reaching a provider is fast or hopeless, generating isn't.
25+
DEFAULT_CONNECT_TIMEOUT_SECONDS = 10.0
26+
DEFAULT_READ_TIMEOUT_SECONDS = 60.0
27+
DEFAULT_NUM_RETRIES = 0
28+
29+
30+
def _default_timeout() -> "httpx.Timeout":
31+
import httpx
32+
33+
return httpx.Timeout(
34+
DEFAULT_READ_TIMEOUT_SECONDS,
35+
connect=DEFAULT_CONNECT_TIMEOUT_SECONDS,
36+
)
37+
2138

2239
def _log_warning(message: str, *args: Any) -> None:
2340
"""Emit a warning to both this module logger and the root logger.
@@ -378,6 +395,8 @@ def generate_provider_response(
378395
# we need to pop messages first, and after we will check the rest params
379396
valid_litellm_params = self._remove_unnecessary_not_supported_params(kwargs)
380397
all_kwargs = {**self._completion_kwargs, **valid_litellm_params}
398+
all_kwargs.setdefault("timeout", _default_timeout())
399+
all_kwargs.setdefault("num_retries", DEFAULT_NUM_RETRIES)
381400
# Conflicts that can only be diagnosed on the merged dict
382401
# (constructor + per-call sources) run here — see the method's
383402
# docstring for the Anthropic reasoning_effort/temperature case.
@@ -469,6 +488,8 @@ async def agenerate_provider_response(
469488

470489
valid_litellm_params = self._remove_unnecessary_not_supported_params(kwargs)
471490
all_kwargs = {**self._completion_kwargs, **valid_litellm_params}
491+
all_kwargs.setdefault("timeout", _default_timeout())
492+
all_kwargs.setdefault("num_retries", DEFAULT_NUM_RETRIES)
472493
# See sync `generate_provider_response` for why the merged
473494
# dict needs its own conflict-resolution pass.
474495
all_kwargs = self._resolve_provider_conflicts(all_kwargs)

sdks/python/src/opik/evaluation/models/litellm/response_parser.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,13 @@ def parse_assistant_message(
5050
content = _extract_response_format_arguments(message)
5151

5252
if content is None and not tool_calls:
53+
# A degraded provider lands here with a valid key, so report the
54+
# rejected response shape rather than only blaming the key.
5355
raise exceptions.BaseLLMError(
54-
"Received None as the output from the LLM. Please verify your environment "
55-
"configuration and ensure that the API keys for the models in use "
56+
"LLM returned no content and no tool calls "
57+
f"(model={getattr(response, 'model', None)!r}, "
58+
f"finish_reason={_finish_reason(response)!r}). "
59+
"If this persists, verify the API keys for the models in use "
5660
"(e.g., OPENAI_API_KEY) are set correctly."
5761
)
5862

@@ -64,6 +68,13 @@ def parse_assistant_message(
6468
return assistant
6569

6670

71+
def _finish_reason(response: "ModelResponse") -> Optional[str]:
72+
choices = getattr(response, "choices", None)
73+
if not isinstance(choices, list) or not choices:
74+
return None
75+
return _get_str(_as_mapping(util.normalise_choice(choices[0])), "finish_reason")
76+
77+
6778
def _normalise_message(response: "ModelResponse") -> Dict[str, Any]:
6879
choices = getattr(response, "choices", None)
6980
if not isinstance(choices, list) or not choices:

sdks/python/src/opik/evaluation/suite_evaluators/llm_judge/metric.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
from opik.evaluation.models import base_model, models_factory
1515
from opik.evaluation.metrics import score_result
16-
from opik.exceptions import LLMJudgeParseError
16+
from opik.exceptions import BaseLLMError, LLMJudgeParseError
1717

1818
from opik.evaluation.suite_evaluators import base
1919
from . import config as llm_judge_config
@@ -22,8 +22,10 @@
2222

2323
LOGGER = logging.getLogger(__name__)
2424

25+
# BaseLLMError covers a structurally empty provider response, which is as
26+
# transient as a malformed parse and shouldn't cost the item its score.
2527
_RETRY_POLICY = tenacity.retry(
26-
retry=tenacity.retry_if_exception_type(LLMJudgeParseError),
28+
retry=tenacity.retry_if_exception_type((LLMJudgeParseError, BaseLLMError)),
2729
stop=tenacity.stop_after_attempt(3),
2830
before_sleep=tenacity.before_sleep_log(LOGGER, logging.WARNING),
2931
reraise=True,

sdks/python/tests/unit/evaluation/models/test_litellm_chat_model.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,3 +876,59 @@ def decorator(func):
876876

877877
# Verify that track_completion decorator was applied the expected number of times
878878
assert decorator_calls == expected_calls
879+
880+
881+
def test_litellm_chat_model_bounds_request_by_default(monkeypatch):
882+
"""Without these, litellm's 6000s default and internal retries apply."""
883+
stub = _install_litellm_stub(monkeypatch)
884+
885+
model = litellm_chat_model.LiteLLMChatModel(model_name="openai/gpt-4o-mini")
886+
model.generate_string("hello")
887+
888+
assert stub._calls, "Expected completion to be invoked"
889+
_, _, kwargs = stub._calls[-1]
890+
assert (
891+
kwargs["timeout"].connect == litellm_chat_model.DEFAULT_CONNECT_TIMEOUT_SECONDS
892+
)
893+
assert kwargs["timeout"].read == litellm_chat_model.DEFAULT_READ_TIMEOUT_SECONDS
894+
assert kwargs["num_retries"] == litellm_chat_model.DEFAULT_NUM_RETRIES
895+
896+
897+
def test_litellm_chat_model_explicit_timeout_overrides_default(monkeypatch):
898+
stub = _install_litellm_stub(monkeypatch)
899+
900+
model = litellm_chat_model.LiteLLMChatModel(
901+
model_name="openai/gpt-4o-mini",
902+
timeout=5.0,
903+
num_retries=2,
904+
)
905+
model.generate_string("hello")
906+
907+
_, _, kwargs = stub._calls[-1]
908+
assert kwargs["timeout"] == 5.0
909+
assert kwargs["num_retries"] == 2
910+
911+
912+
@pytest.mark.asyncio
913+
async def test_litellm_chat_model_bounds_async_request_by_default(monkeypatch):
914+
stub = _install_litellm_stub(monkeypatch)
915+
916+
captured = {}
917+
918+
async def acompletion(model, messages, **kwargs):
919+
captured.update(kwargs)
920+
return SimpleNamespace(
921+
choices=[SimpleNamespace(message=SimpleNamespace(content="ok"))]
922+
)
923+
924+
stub.acompletion = acompletion
925+
926+
model = litellm_chat_model.LiteLLMChatModel(model_name="openai/gpt-4o-mini")
927+
await model.agenerate_string("hello")
928+
929+
assert (
930+
captured["timeout"].connect
931+
== litellm_chat_model.DEFAULT_CONNECT_TIMEOUT_SECONDS
932+
)
933+
assert captured["timeout"].read == litellm_chat_model.DEFAULT_READ_TIMEOUT_SECONDS
934+
assert captured["num_retries"] == litellm_chat_model.DEFAULT_NUM_RETRIES

sdks/python/tests/unit/evaluation/suite_evaluators/test_llm_judge.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,41 @@ def test_from_config__config_model_params__temperature_and_seed_preserved(self):
289289
"seed": 123,
290290
"customParameters": {"reasoning_effort": "low"},
291291
}
292+
293+
294+
class TestLLMJudgeRetries:
295+
"""An empty provider response raises BaseLLMError, not a parse error."""
296+
297+
def _judge_with_model(self, model):
298+
evaluator = llm_judge.LLMJudge(
299+
assertions=["The response contains the literal text PASS."],
300+
track=False,
301+
)
302+
evaluator._model = model
303+
return evaluator
304+
305+
def test_score__transient_empty_llm_response__retries_and_succeeds(self):
306+
from opik import exceptions
307+
308+
calls = {"n": 0}
309+
valid = (
310+
'{"assertion_1": {"score": true, "reason": "contains PASS", '
311+
'"confidence": 1.0}}'
312+
)
313+
314+
class FlakyModel:
315+
def generate_chat_completion(self, messages, response_format=None):
316+
calls["n"] += 1
317+
if calls["n"] == 1:
318+
raise exceptions.BaseLLMError(
319+
"LLM infrastructure error: Received None as the output "
320+
"from the LLM."
321+
)
322+
return {"role": "assistant", "content": valid}
323+
324+
evaluator = self._judge_with_model(FlakyModel())
325+
results = evaluator.score(input="q", output="PASS")
326+
327+
assert calls["n"] == 2, "the empty first response should have been retried"
328+
assert len(results) == 1
329+
assert results[0].value == 1.0

0 commit comments

Comments
 (0)