diff --git a/src/google/adk/evaluation/local_eval_service.py b/src/google/adk/evaluation/local_eval_service.py index 16dcc514c8..bc025515ae 100644 --- a/src/google/adk/evaluation/local_eval_service.py +++ b/src/google/adk/evaluation/local_eval_service.py @@ -495,6 +495,7 @@ def _generate_final_eval_status( self, overall_eval_metric_results: list[EvalMetricResult] ) -> EvalStatus: final_eval_status = EvalStatus.NOT_EVALUATED + has_not_evaluated_metric = False # Go over all the eval statuses and mark the final eval status as # passed if all of them pass; otherwise, mark the final eval status to # failed. @@ -503,6 +504,7 @@ def _generate_final_eval_status( if overall_eval_status == EvalStatus.PASSED: final_eval_status = EvalStatus.PASSED elif overall_eval_status == EvalStatus.NOT_EVALUATED: + has_not_evaluated_metric = True continue elif overall_eval_status == EvalStatus.FAILED: final_eval_status = EvalStatus.FAILED @@ -510,6 +512,13 @@ def _generate_final_eval_status( else: raise ValueError(f"Unknown eval status: {overall_eval_status}.") + # A metric that never produced a verdict (e.g. a judge call that crashed) + # must not be silently masked by another metric that happened to pass -- + # only report PASSED when every requested metric was actually evaluated. + # A genuine FAILED is real evidence and still takes precedence. + if has_not_evaluated_metric and final_eval_status == EvalStatus.PASSED: + final_eval_status = EvalStatus.NOT_EVALUATED + return final_eval_status async def _perform_inference_single_eval_item( diff --git a/tests/unittests/evaluation/test_local_eval_service.py b/tests/unittests/evaluation/test_local_eval_service.py index 8223adc341..291e32d158 100644 --- a/tests/unittests/evaluation/test_local_eval_service.py +++ b/tests/unittests/evaluation/test_local_eval_service.py @@ -629,6 +629,96 @@ def test_generate_final_eval_status_doesn_t_throw_on(eval_service): eval_service._generate_final_eval_status([eval_metric_result]) +def test_generate_final_eval_status_not_evaluated_then_passed_is_not_evaluated( + eval_service, +): + """A metric that never produced a verdict must not be masked by a later PASSED. + + If metric_1 crashed (NOT_EVALUATED) and metric_2 passed, the eval case did + not actually pass every requested metric -- it should be reported as + NOT_EVALUATED, not PASSED. + """ + results = [ + EvalMetricResult( + metric_name="metric1", + threshold=0.5, + eval_status=EvalStatus.NOT_EVALUATED, + ), + EvalMetricResult( + metric_name="metric2", threshold=0.5, eval_status=EvalStatus.PASSED + ), + ] + + assert ( + eval_service._generate_final_eval_status(results) + == EvalStatus.NOT_EVALUATED + ) + + +def test_generate_final_eval_status_passed_then_not_evaluated_is_not_evaluated( + eval_service, +): + """Same as above with the metrics in the opposite order. + + Both orderings of [PASSED, NOT_EVALUATED] must produce the same final + status. Prior to this fix they didn't: this ordering silently returned + PASSED, which is exactly the evidence that the old behavior was a bug + and not intended. + """ + results = [ + EvalMetricResult( + metric_name="metric1", threshold=0.5, eval_status=EvalStatus.PASSED + ), + EvalMetricResult( + metric_name="metric2", + threshold=0.5, + eval_status=EvalStatus.NOT_EVALUATED, + ), + ] + + assert ( + eval_service._generate_final_eval_status(results) + == EvalStatus.NOT_EVALUATED + ) + + +def test_generate_final_eval_status_failed_dominates_not_evaluated( + eval_service, +): + """A genuine FAILED is real evidence and must still win over NOT_EVALUATED, + regardless of order -- only the PASSED-vs-NOT_EVALUATED case was buggy. + """ + failed_then_not_evaluated = [ + EvalMetricResult( + metric_name="metric1", threshold=0.5, eval_status=EvalStatus.FAILED + ), + EvalMetricResult( + metric_name="metric2", + threshold=0.5, + eval_status=EvalStatus.NOT_EVALUATED, + ), + ] + not_evaluated_then_failed = [ + EvalMetricResult( + metric_name="metric1", + threshold=0.5, + eval_status=EvalStatus.NOT_EVALUATED, + ), + EvalMetricResult( + metric_name="metric2", threshold=0.5, eval_status=EvalStatus.FAILED + ), + ] + + assert ( + eval_service._generate_final_eval_status(failed_then_not_evaluated) + == EvalStatus.FAILED + ) + assert ( + eval_service._generate_final_eval_status(not_evaluated_then_failed) + == EvalStatus.FAILED + ) + + @pytest.mark.asyncio async def test_mcp_stdio_agent_no_runtime_error(mocker): """Test that LocalEvalService can handle MCP stdio agents without RuntimeError.