Skip to content

Commit 03be770

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 03be770

5 files changed

Lines changed: 159 additions & 4 deletions

File tree

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

Lines changed: 34 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,35 @@
1819

1920
LOGGER = logging.getLogger(__name__)
2021

22+
# LiteLLM's own default request timeout is 6000s (100 minutes), so a provider
23+
# that stalls — most often on TCP connect, where no response ever arrives —
24+
# blocks the caller effectively forever.
25+
#
26+
# The connect timeout is deliberately much tighter than the read timeout: a
27+
# provider either accepts the connection promptly or it is unreachable, whereas
28+
# generating a long completion legitimately takes time. Splitting them lets a
29+
# stalled connect fail fast without truncating slow-but-healthy generations.
30+
#
31+
# `num_retries` is pinned alongside them because LiteLLM retries internally by
32+
# default (~3 HTTP attempts per call), which silently multiplies the timeout on
33+
# top of the tenacity retries this module and the LLM-judge metric already
34+
# apply. Leaving it unset makes the worst-case wall clock the product of three
35+
# independent retry layers. Retrying stays the callers' job.
36+
#
37+
# All are defaults only — an explicit `timeout`/`num_retries` still wins.
38+
DEFAULT_CONNECT_TIMEOUT_SECONDS = 10.0
39+
DEFAULT_READ_TIMEOUT_SECONDS = 60.0
40+
DEFAULT_NUM_RETRIES = 0
41+
42+
43+
def _default_timeout() -> "httpx.Timeout":
44+
import httpx
45+
46+
return httpx.Timeout(
47+
DEFAULT_READ_TIMEOUT_SECONDS,
48+
connect=DEFAULT_CONNECT_TIMEOUT_SECONDS,
49+
)
50+
2151

2252
def _log_warning(message: str, *args: Any) -> None:
2353
"""Emit a warning to both this module logger and the root logger.
@@ -378,6 +408,8 @@ def generate_provider_response(
378408
# we need to pop messages first, and after we will check the rest params
379409
valid_litellm_params = self._remove_unnecessary_not_supported_params(kwargs)
380410
all_kwargs = {**self._completion_kwargs, **valid_litellm_params}
411+
all_kwargs.setdefault("timeout", _default_timeout())
412+
all_kwargs.setdefault("num_retries", DEFAULT_NUM_RETRIES)
381413
# Conflicts that can only be diagnosed on the merged dict
382414
# (constructor + per-call sources) run here — see the method's
383415
# docstring for the Anthropic reasoning_effort/temperature case.
@@ -469,6 +501,8 @@ async def agenerate_provider_response(
469501

470502
valid_litellm_params = self._remove_unnecessary_not_supported_params(kwargs)
471503
all_kwargs = {**self._completion_kwargs, **valid_litellm_params}
504+
all_kwargs.setdefault("timeout", _default_timeout())
505+
all_kwargs.setdefault("num_retries", DEFAULT_NUM_RETRIES)
472506
# See sync `generate_provider_response` for why the merged
473507
# dict needs its own conflict-resolution pass.
474508
all_kwargs = self._resolve_provider_conflicts(all_kwargs)

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

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

5252
if content is None and not tool_calls:
53+
# An authenticated-but-degraded provider also lands here (empty
54+
# tool_use on the structured-output path), so report the response
55+
# shape that was actually rejected instead of only blaming the key.
5356
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 "
57+
"LLM returned no content and no tool calls "
58+
f"(model={getattr(response, 'model', None)!r}, "
59+
f"finish_reason={_finish_reason(response)!r}). "
60+
"If this persists, verify the API keys for the models in use "
5661
"(e.g., OPENAI_API_KEY) are set correctly."
5762
)
5863

@@ -64,6 +69,13 @@ def parse_assistant_message(
6469
return assistant
6570

6671

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

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

Lines changed: 7 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,13 @@
2222

2323
LOGGER = logging.getLogger(__name__)
2424

25+
# `BaseLLMError` covers the provider returning a structurally empty response
26+
# (no content and no tool_calls) — seen in practice when a provider is briefly
27+
# degraded and the structured-output tool_use call comes back blank. It is
28+
# transient in the same way a malformed parse is, so it earns the same retry
29+
# rather than costing the item its whole score.
2530
_RETRY_POLICY = tenacity.retry(
26-
retry=tenacity.retry_if_exception_type(LLMJudgeParseError),
31+
retry=tenacity.retry_if_exception_type((LLMJudgeParseError, BaseLLMError)),
2732
stop=tenacity.stop_after_attempt(3),
2833
before_sleep=tenacity.before_sleep_log(LOGGER, logging.WARNING),
2934
reraise=True,

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,3 +876,64 @@ 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+
"""A stalled provider must not block forever.
883+
884+
litellm's own defaults are a 6000s timeout and internal retries, so an
885+
unbounded call multiplied across the retry layers can outlive any caller.
886+
Observed stalls sit in TCP connect, hence the separate connect ceiling.
887+
"""
888+
stub = _install_litellm_stub(monkeypatch)
889+
890+
model = litellm_chat_model.LiteLLMChatModel(model_name="openai/gpt-4o-mini")
891+
model.generate_string("hello")
892+
893+
assert stub._calls, "Expected completion to be invoked"
894+
_, _, kwargs = stub._calls[-1]
895+
assert (
896+
kwargs["timeout"].connect == litellm_chat_model.DEFAULT_CONNECT_TIMEOUT_SECONDS
897+
)
898+
assert kwargs["timeout"].read == litellm_chat_model.DEFAULT_READ_TIMEOUT_SECONDS
899+
assert kwargs["num_retries"] == litellm_chat_model.DEFAULT_NUM_RETRIES
900+
901+
902+
def test_litellm_chat_model_explicit_timeout_overrides_default(monkeypatch):
903+
stub = _install_litellm_stub(monkeypatch)
904+
905+
model = litellm_chat_model.LiteLLMChatModel(
906+
model_name="openai/gpt-4o-mini",
907+
timeout=5.0,
908+
num_retries=2,
909+
)
910+
model.generate_string("hello")
911+
912+
_, _, kwargs = stub._calls[-1]
913+
assert kwargs["timeout"] == 5.0
914+
assert kwargs["num_retries"] == 2
915+
916+
917+
@pytest.mark.asyncio
918+
async def test_litellm_chat_model_bounds_async_request_by_default(monkeypatch):
919+
stub = _install_litellm_stub(monkeypatch)
920+
921+
captured = {}
922+
923+
async def acompletion(model, messages, **kwargs):
924+
captured.update(kwargs)
925+
return SimpleNamespace(
926+
choices=[SimpleNamespace(message=SimpleNamespace(content="ok"))]
927+
)
928+
929+
stub.acompletion = acompletion
930+
931+
model = litellm_chat_model.LiteLLMChatModel(model_name="openai/gpt-4o-mini")
932+
await model.agenerate_string("hello")
933+
934+
assert (
935+
captured["timeout"].connect
936+
== litellm_chat_model.DEFAULT_CONNECT_TIMEOUT_SECONDS
937+
)
938+
assert captured["timeout"].read == litellm_chat_model.DEFAULT_READ_TIMEOUT_SECONDS
939+
assert captured["num_retries"] == litellm_chat_model.DEFAULT_NUM_RETRIES

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,46 @@ 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+
"""A transient provider hiccup must not cost the whole item's score.
296+
297+
An empty structured-output response surfaces as ``BaseLLMError`` from the
298+
response parser, which is a different exception type than the parse errors
299+
the retry policy was originally written for.
300+
"""
301+
302+
def _judge_with_model(self, model):
303+
evaluator = llm_judge.LLMJudge(
304+
assertions=["The response contains the literal text PASS."],
305+
track=False,
306+
)
307+
evaluator._model = model
308+
return evaluator
309+
310+
def test_score__transient_empty_llm_response__retries_and_succeeds(self):
311+
from opik import exceptions
312+
313+
calls = {"n": 0}
314+
valid = (
315+
'{"assertion_1": {"score": true, "reason": "contains PASS", '
316+
'"confidence": 1.0}}'
317+
)
318+
319+
class FlakyModel:
320+
def generate_chat_completion(self, messages, response_format=None):
321+
calls["n"] += 1
322+
if calls["n"] == 1:
323+
raise exceptions.BaseLLMError(
324+
"LLM infrastructure error: Received None as the output "
325+
"from the LLM."
326+
)
327+
return {"role": "assistant", "content": valid}
328+
329+
evaluator = self._judge_with_model(FlakyModel())
330+
results = evaluator.score(input="q", output="PASS")
331+
332+
assert calls["n"] == 2, "the empty first response should have been retried"
333+
assert len(results) == 1
334+
assert results[0].value == 1.0

0 commit comments

Comments
 (0)