From 022539de68a950cf2f018b7b80ec263b749e13ff Mon Sep 17 00:00:00 2001 From: Yupeng Lin Date: Tue, 23 Jun 2026 09:17:04 +0800 Subject: [PATCH] fix(guardrails): stop llm_as_a_judge failing open on missing score The llm_as_a_judge guardrail decided pass/fail solely from a scalar overall_score that defaulted to 100 when absent, so a judge returning only failing per-criterion verdicts (no top-level score) was silently allowed. Derive overall_score from verdict score/weight when the top-level field is missing or unparseable, and add an opt-in `fail_closed` flag that blocks when no score can be determined or the judge call errors. Default stays fail-open for backward compatibility, but the previously-silent path now logs a warning. Fixes #30731 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../llm_as_a_judge/__init__.py | 126 ++++++- .../proxy/guardrails/test_llm_as_a_judge.py | 309 +++++++++++++++++- 2 files changed, 423 insertions(+), 12 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py b/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py index 45bdd6dd09f..b03ae70225e 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py +++ b/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py @@ -1,6 +1,7 @@ """LLM-as-a-Judge guardrail: uses an LLM to score responses against weighted criteria.""" import json +import math from datetime import datetime from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional, Union @@ -83,6 +84,75 @@ def _build_judge_prompt( ) +def _derive_overall_score(judge_result: Dict[str, Any]) -> Optional[float]: + """Resolve the overall 0-100 score for a judge result. + + Prefers the top-level ``overall_score``. When that field is missing, not a + number, or non-finite, derives a weighted average from per-criterion + ``verdicts`` (falling back to a simple mean when weights are absent) so that a + judge which returns only verdicts cannot silently pass. All scores are + validated as finite and clamped to ``[0, 100]`` before use, so injected + ``NaN``/``Infinity`` or out-of-range values cannot inflate the result. + Returns ``None`` when no usable score can be determined (including a non-dict + judge result), leaving the fail-open/closed decision to the caller. + """ + if not isinstance(judge_result, dict): + return None + + raw = judge_result.get("overall_score") + if raw is not None: + try: + value = float(raw) + except (TypeError, ValueError): + value = None + if value is not None and math.isfinite(value): + return max(0.0, min(100.0, value)) + # unparseable or non-finite -> fall through to verdict-derived score + + verdicts = judge_result.get("verdicts") + if not isinstance(verdicts, list) or not verdicts: + return None + + weighted_sum = 0.0 + weight_total = 0.0 + scores: List[float] = [] + all_weighted = True + for verdict in verdicts: + if not isinstance(verdict, dict): + continue + raw_score = verdict.get("score") + if raw_score is None: + continue + try: + score = float(raw_score) + except (TypeError, ValueError): + continue + if not math.isfinite(score): + continue + score = max(0.0, min(100.0, score)) # clamp per verdict before averaging + scores.append(score) + try: + weight = float(verdict.get("weight", 0)) + except (TypeError, ValueError): + weight = 0.0 + if math.isfinite(weight) and weight > 0: + weighted_sum += score * weight + weight_total += weight + else: + all_weighted = False + + # Only use the weighted average when *every* scored verdict carries a + # positive weight. If any verdict is missing/zero weight, weighting would + # silently drop that verdict's score (e.g. a failing criterion the judge + # forgot to weight), inflating the result -- the same fail-open class this + # guardrail guards against. Fall back to a simple mean so no score is lost. + if all_weighted and weight_total > 0: + return max(0.0, min(100.0, weighted_sum / weight_total)) + if scores: + return max(0.0, min(100.0, sum(scores) / len(scores))) + return None + + class LLMAsAJudgeGuardrail(CustomGuardrail): """Post-call guardrail that judges response quality via an LLM.""" @@ -93,6 +163,7 @@ class LLMAsAJudgeGuardrail(CustomGuardrail): criteria: List[Dict[str, Any]], overall_threshold: float = 80.0, on_failure: Literal["block", "log"] = "block", + fail_closed: bool = False, event_hook: Optional[ Union[GuardrailEventHooks, List[GuardrailEventHooks]] ] = None, @@ -126,6 +197,24 @@ class LLMAsAJudgeGuardrail(CustomGuardrail): self.criteria = criteria self.overall_threshold = overall_threshold self.on_failure = on_failure + self.fail_closed = fail_closed + + def _should_block_on_failure(self, reason: str) -> bool: + """Log an evaluation failure and report whether the response must block. + + Returns ``True`` only when configured to fail closed *and* ``on_failure`` + is ``"block"``. ``fail_closed`` therefore only takes effect in blocking + mode: with ``on_failure="log"`` the guardrail is observe-only and never + blocks -- consistent with a below-threshold score only being logged in + that mode -- so ``fail_closed=True`` combined with ``on_failure="log"`` + logs the failure without blocking. The caller sets run status and raises + so guardrail-status logging stays accurate. With ``fail_closed=False`` + (the default) the guardrail fails open, preserving prior behaviour, but + the previously-silent missing-score path now emits a warning. + """ + mode = "closed" if self.fail_closed else "open" + verbose_logger.warning(f"llm_as_a_judge: {reason}; failing {mode}") + return self.fail_closed and self.on_failure == "block" async def _run_judge( self, @@ -174,20 +263,30 @@ class LLMAsAJudgeGuardrail(CustomGuardrail): try: judge_result = await self._run_judge(messages, response_text) except Exception as judge_err: - verbose_logger.warning( - f"llm_as_a_judge guardrail: judge call failed, failing open. Error: {judge_err}" - ) + if self._should_block_on_failure(f"judge call failed: {judge_err}"): + status = "guardrail_intervened" + raise HTTPException( + status_code=422, + detail={ + "error": "LLM judge guardrail failed closed: judge call failed", + }, + ) status = "guardrail_failed_to_respond" return inputs - try: - overall_score = max( - 0.0, min(100.0, float(judge_result.get("overall_score", 100))) - ) - except (TypeError, ValueError): - verbose_logger.warning( - "llm_as_a_judge: invalid overall_score from judge, failing open" - ) + overall_score = _derive_overall_score(judge_result) + if overall_score is None: + if self._should_block_on_failure( + "could not determine overall_score (missing/invalid score and no usable verdicts)" + ): + status = "guardrail_intervened" + raise HTTPException( + status_code=422, + detail={ + "error": "LLM judge guardrail failed closed: could not evaluate response", + }, + ) + status = "guardrail_failed_to_respond" return inputs passed = overall_score >= self.overall_threshold @@ -275,6 +374,10 @@ def initialize_guardrail( _get_litellm_param(litellm_params, guardrail, "overall_threshold", 80.0) ) + fail_closed = bool( + _get_litellm_param(litellm_params, guardrail, "fail_closed", False) + ) + mode = _get_litellm_param(litellm_params, guardrail, "mode") event_hook: Optional[GuardrailEventHooks] = None if isinstance(mode, str) and mode in {e.value for e in GuardrailEventHooks}: @@ -286,6 +389,7 @@ def initialize_guardrail( criteria=criteria, overall_threshold=overall_threshold, on_failure=on_failure, + fail_closed=fail_closed, event_hook=event_hook, default_on=bool( _get_litellm_param(litellm_params, guardrail, "default_on", False) diff --git a/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py b/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py index c9fde4ffbae..26b8ecddc44 100644 --- a/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py +++ b/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py @@ -9,6 +9,7 @@ from fastapi import HTTPException from litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge import ( LLMAsAJudgeGuardrail, _build_judge_prompt, + _derive_overall_score, _extract_text_from_content, initialize_guardrail, ) @@ -90,7 +91,7 @@ def test_build_judge_prompt_missing_name_and_weight(): def _make_litellm_params(**overrides): params = MagicMock() - for attr in ("guardrail_name", "judge_model", "criteria", "on_failure", "overall_threshold", "mode", "default_on"): + for attr in ("guardrail_name", "judge_model", "criteria", "on_failure", "overall_threshold", "fail_closed", "mode", "default_on"): setattr(params, attr, None) for k, v in overrides.items(): setattr(params, k, v) @@ -222,3 +223,309 @@ async def test_apply_guardrail_clamps_score(mock_completion): result = await guardrail.apply_guardrail(inputs, request_data, "response") assert result is inputs assert request_data["metadata"]["eval_information"]["overall_score"] == 100.0 + + +# --------------------------------------------------------------------------- +# _derive_overall_score — score resolution (issue #30731) +# --------------------------------------------------------------------------- + + +def test_derive_overall_score_prefers_top_level(): + assert _derive_overall_score({"overall_score": 73, "verdicts": []}) == 73.0 + + +def test_derive_overall_score_clamps_top_level(): + assert _derive_overall_score({"overall_score": 150}) == 100.0 + assert _derive_overall_score({"overall_score": -10}) == 0.0 + + +def test_derive_overall_score_weighted_average_from_verdicts(): + # Missing overall_score -> weighted mean: (2*60 + 5*40) / 100 = 3.2 + result = _derive_overall_score( + { + "verdicts": [ + {"criterion_name": "Safety", "score": 2, "passed": False, "weight": 60}, + {"criterion_name": "Policy", "score": 5, "passed": False, "weight": 40}, + ] + } + ) + assert result == pytest.approx(3.2) + + +def test_derive_overall_score_simple_mean_when_no_weights(): + result = _derive_overall_score( + {"verdicts": [{"score": 2}, {"score": 5}, {"score": 8}]} + ) + assert result == pytest.approx(5.0) + + +def test_derive_overall_score_falls_back_to_verdicts_when_unparseable(): + result = _derive_overall_score( + { + "overall_score": "not-a-number", + "verdicts": [{"score": 90, "weight": 100}], + } + ) + assert result == pytest.approx(90.0) + + +def test_derive_overall_score_none_when_no_data(): + assert _derive_overall_score({}) is None + assert _derive_overall_score({"verdicts": []}) is None + assert _derive_overall_score({"verdicts": [{"reasoning": "no score"}]}) is None + + +def test_derive_overall_score_skips_non_dict_verdict_and_bad_weight(): + # Non-dict entries are skipped; verdicts whose weight is non-numeric fall + # back to a simple mean of the parsed scores instead of crashing. + result = _derive_overall_score( + { + "verdicts": [ + "not-a-dict", # skipped + {"score": "abc", "weight": 50}, # non-numeric score -> skipped + {"score": 40, "weight": "heavy"}, # bad weight -> ignored weight + {"score": 60, "weight": "x"}, # bad weight -> ignored weight + ] + } + ) + assert result == pytest.approx(50.0) + + +def test_derive_overall_score_mixed_weighted_and_unweighted_uses_simple_mean(): + # A verdict missing its weight must NOT be silently dropped from the score. + # A(90, weight 60) + B(0, no weight) -> simple mean 45, NOT weighted 90. + result = _derive_overall_score( + { + "verdicts": [ + {"criterion_name": "A", "score": 90, "weight": 60}, + {"criterion_name": "B", "score": 0}, # weight missing + ] + } + ) + assert result == pytest.approx(45.0) + + +# --------------------------------------------------------------------------- +# _derive_overall_score — malformed / manipulated judge output (veria-ai) +# --------------------------------------------------------------------------- + + +def test_derive_overall_score_rejects_non_finite_top_level(): + # NaN/Infinity must not clamp to 100. With no verdicts -> indeterminate. + assert _derive_overall_score({"overall_score": float("nan")}) is None + assert _derive_overall_score({"overall_score": float("inf")}) is None + assert _derive_overall_score({"overall_score": "nan"}) is None + + +def test_derive_overall_score_non_finite_top_level_falls_back_to_verdicts(): + result = _derive_overall_score( + {"overall_score": float("nan"), "verdicts": [{"score": 10, "weight": 100}]} + ) + assert result == pytest.approx(10.0) + + +def test_derive_overall_score_skips_non_finite_verdict_score(): + # A NaN verdict score is skipped, not averaged in as 100. + result = _derive_overall_score( + {"verdicts": [{"score": "nan", "weight": 60}, {"score": 0, "weight": 40}]} + ) + assert result == pytest.approx(0.0) + + +def test_derive_overall_score_clamps_inflated_verdict_score(): + # An out-of-range score is clamped per verdict so it cannot mask a failure. + # clamp(10000) = 100, simple mean(100, 0) = 50. + result = _derive_overall_score({"verdicts": [{"score": 10000}, {"score": 0}]}) + assert result == pytest.approx(50.0) + + +def test_derive_overall_score_none_for_non_dict(): + assert _derive_overall_score([]) is None + assert _derive_overall_score("not a dict") is None + assert _derive_overall_score(123) is None + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_non_dict_judge_output_fail_closed_blocks(mock_completion): + # A top-level JSON array is valid JSON but unusable -> indeterminate -> + # fail_closed must block instead of slipping through the outer except. + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps([])))] + ) + guardrail = _make_guardrail(fail_closed=True, on_failure="block") + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_inflated_verdict_scores_still_block(mock_completion): + # Weighted path: clamp(10000)=100, (100*60 + 0*40)/100 = 60 < 80 -> block. + payload = { + "verdicts": [ + {"criterion_name": "A", "score": 10000, "weight": 60}, + {"criterion_name": "B", "score": 0, "weight": 40}, + ] + } + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps(payload)))] + ) + guardrail = _make_guardrail(overall_threshold=80.0, on_failure="block") + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_missing_weight_on_failing_verdict_still_blocks(mock_completion): + # End-to-end: judge drops the weight on a failing criterion. The failing + # score must still pull the derived score below threshold and block. + payload = { + "verdicts": [ + {"criterion_name": "Accuracy", "score": 100, "weight": 60}, + {"criterion_name": "Safety", "score": 0}, # failing, weight missing + ] + } + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps(payload)))] + ) + guardrail = _make_guardrail(overall_threshold=80.0, on_failure="block") + inputs = {"texts": ["partially unsafe response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +# --------------------------------------------------------------------------- +# apply_guardrail — missing overall_score (fail-open regression, issue #30731) +# --------------------------------------------------------------------------- + + +def _verdicts_only_response(scores_weights) -> dict: + """Build a judge response with verdicts but NO top-level overall_score.""" + return { + "verdicts": [ + { + "criterion_name": name, + "score": score, + "reasoning": "r", + "passed": score >= 80, + "weight": weight, + } + for name, score, weight in scores_weights + ] + } + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_missing_overall_score_with_failing_verdicts_blocks(mock_completion): + # The core bug: judge omits overall_score but every verdict fails. Previously + # defaulted to 100 (pass); now derived from verdicts -> 3.2 -> blocked. + payload = _verdicts_only_response([("Safety", 2, 60), ("Policy", 5, 40)]) + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps(payload)))] + ) + guardrail = _make_guardrail(overall_threshold=80.0, on_failure="block") + inputs = {"texts": ["unsafe response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_missing_overall_score_with_passing_verdicts_passes(mock_completion): + payload = _verdicts_only_response([("Accuracy", 95, 60), ("Safety", 90, 40)]) + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps(payload)))] + ) + guardrail = _make_guardrail(overall_threshold=80.0) + inputs = {"texts": ["good response"]} + request_data: dict = {"messages": [], "metadata": {}} + result = await guardrail.apply_guardrail(inputs, request_data, "response") + assert result is inputs + assert request_data["metadata"]["eval_information"]["passed"] is True + assert request_data["metadata"]["eval_information"]["overall_score"] == pytest.approx(93.0) + + +# --------------------------------------------------------------------------- +# apply_guardrail — fail_closed configuration +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_no_score_no_verdicts_fails_open_by_default(mock_completion): + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps({})))] + ) + guardrail = _make_guardrail() # fail_closed defaults to False + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + result = await guardrail.apply_guardrail(inputs, request_data, "response") + assert result is inputs + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_no_score_no_verdicts_fail_closed_blocks(mock_completion): + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps({})))] + ) + guardrail = _make_guardrail(fail_closed=True, on_failure="block") + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_fail_closed_log_mode_does_not_block(mock_completion): + mock_completion.return_value = MagicMock( + choices=[MagicMock(message=MagicMock(content=json.dumps({})))] + ) + guardrail = _make_guardrail(fail_closed=True, on_failure="log") + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + result = await guardrail.apply_guardrail(inputs, request_data, "response") + assert result is inputs + + +@pytest.mark.asyncio +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion") +async def test_judge_error_fail_closed_blocks(mock_completion): + mock_completion.side_effect = RuntimeError("judge down") + guardrail = _make_guardrail(fail_closed=True, on_failure="block") + inputs = {"texts": ["response"]} + request_data: dict = {"messages": [], "metadata": {}} + with pytest.raises(HTTPException) as exc_info: + await guardrail.apply_guardrail(inputs, request_data, "response") + assert exc_info.value.status_code == 422 + + +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.logging_callback_manager") +def test_initialize_guardrail_parses_fail_closed(mock_mgr): + lp = _make_litellm_params() + g = _make_guardrail_dict(fail_closed=True) + instance = initialize_guardrail(lp, g) + assert instance.fail_closed is True + + +@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.logging_callback_manager") +def test_initialize_guardrail_fail_closed_defaults_false(mock_mgr): + lp = _make_litellm_params() + g = _make_guardrail_dict() + instance = initialize_guardrail(lp, g) + assert instance.fail_closed is False