From 0918b647df6f88b95974d486a3161121a6514901 Mon Sep 17 00:00:00 2001 From: Keyur Joshi Date: Tue, 30 Dec 2025 11:06:46 -0800 Subject: [PATCH] fix: fix inconsistent method signatures for evaluate_invocations The evaluate_invocations method override in Evaluator subclasses was not consistent, leading to errors during calls, especially when using kwargs. Made the overrides and calls consistent to resolve this issue. Co-authored-by: Keyur Joshi PiperOrigin-RevId: 850462752 --- src/google/adk/evaluation/evaluator.py | 4 ++-- .../adk/evaluation/final_response_match_v1.py | 5 +++-- src/google/adk/evaluation/hallucinations_v1.py | 7 +++++-- src/google/adk/evaluation/llm_as_judge.py | 5 +++-- src/google/adk/evaluation/local_eval_service.py | 1 + src/google/adk/evaluation/response_evaluator.py | 10 ++++++---- src/google/adk/evaluation/safety_evaluator.py | 8 ++++++-- .../per_turn_user_simulator_quality_v1.py | 8 +++++--- src/google/adk/evaluation/trajectory_evaluator.py | 5 ++++- src/google/adk/evaluation/vertex_ai_eval_facade.py | 5 +++-- .../evaluation/test_local_eval_service.py | 14 ++++++++++---- 11 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/google/adk/evaluation/evaluator.py b/src/google/adk/evaluation/evaluator.py index c41fed74..c3c29ba4 100644 --- a/src/google/adk/evaluation/evaluator.py +++ b/src/google/adk/evaluation/evaluator.py @@ -62,8 +62,8 @@ class Evaluator(ABC): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - conversation_scenario: Optional[ConversationScenario], + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: """Returns EvaluationResult after performing evaluations using actual and expected invocations. diff --git a/src/google/adk/evaluation/final_response_match_v1.py b/src/google/adk/evaluation/final_response_match_v1.py index 06a64408..07365211 100644 --- a/src/google/adk/evaluation/final_response_match_v1.py +++ b/src/google/adk/evaluation/final_response_match_v1.py @@ -60,11 +60,12 @@ class RougeEvaluator(Evaluator): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - _: Optional[ConversationScenario] = None, + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: if expected_invocations is None: raise ValueError("expected_invocations is required for this metric.") + del conversation_scenario # not used by this metric. total_score = 0.0 num_invocations = 0 diff --git a/src/google/adk/evaluation/hallucinations_v1.py b/src/google/adk/evaluation/hallucinations_v1.py index 84a4a115..8ed87d2c 100644 --- a/src/google/adk/evaluation/hallucinations_v1.py +++ b/src/google/adk/evaluation/hallucinations_v1.py @@ -720,9 +720,11 @@ class HallucinationsV1Evaluator(Evaluator): async def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - _: Optional[ConversationScenario] = None, + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: + del conversation_scenario # not used by this metric. + # expected_invocations are not required by the metric and if they are not # supplied, we provide a list of None to rest of the code. expected_invocations = ( @@ -730,6 +732,7 @@ class HallucinationsV1Evaluator(Evaluator): if expected_invocations is None else expected_invocations ) + per_invocation_results = [] for actual, expected in zip(actual_invocations, expected_invocations): step_evaluations = self._get_steps_to_evaluate(actual) diff --git a/src/google/adk/evaluation/llm_as_judge.py b/src/google/adk/evaluation/llm_as_judge.py index 0f2d8901..b4252dfa 100644 --- a/src/google/adk/evaluation/llm_as_judge.py +++ b/src/google/adk/evaluation/llm_as_judge.py @@ -118,11 +118,12 @@ class LlmAsJudge(Evaluator): async def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - _: Optional[ConversationScenario] = None, + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: if self._expected_invocations_required and expected_invocations is None: raise ValueError("expected_invocations is needed by this metric.") + del conversation_scenario # not supported for per-invocation evaluation. # If expected_invocation are not required by the metric and if they are not # supplied, we provide a list of None. diff --git a/src/google/adk/evaluation/local_eval_service.py b/src/google/adk/evaluation/local_eval_service.py index 5acbff06..bc1d3886 100644 --- a/src/google/adk/evaluation/local_eval_service.py +++ b/src/google/adk/evaluation/local_eval_service.py @@ -370,6 +370,7 @@ class LocalEvalService(BaseEvalService): return metric_evaluator.evaluate_invocations( actual_invocations=actual_invocations, expected_invocations=expected_invocations, + conversation_scenario=conversation_scenario, ) def _generate_final_eval_status( diff --git a/src/google/adk/evaluation/response_evaluator.py b/src/google/adk/evaluation/response_evaluator.py index 5052aca2..3f7f309a 100644 --- a/src/google/adk/evaluation/response_evaluator.py +++ b/src/google/adk/evaluation/response_evaluator.py @@ -100,8 +100,8 @@ class ResponseEvaluator(Evaluator): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - _: Optional[ConversationScenario] = None, + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: # If the metric is response_match_score, just use the RougeEvaluator. if self._metric_name == PrebuiltMetrics.RESPONSE_MATCH_SCORE.value: @@ -109,11 +109,13 @@ class ResponseEvaluator(Evaluator): EvalMetric(metric_name=self._metric_name, threshold=self._threshold) ) return rouge_evaluator.evaluate_invocations( - actual_invocations, expected_invocations + actual_invocations, expected_invocations, conversation_scenario ) return _VertexAiEvalFacade( threshold=self._threshold, metric_name=self._metric_name, expected_invocations_required=True, - ).evaluate_invocations(actual_invocations, expected_invocations) + ).evaluate_invocations( + actual_invocations, expected_invocations, conversation_scenario + ) diff --git a/src/google/adk/evaluation/safety_evaluator.py b/src/google/adk/evaluation/safety_evaluator.py index e85d62dd..8c049803 100644 --- a/src/google/adk/evaluation/safety_evaluator.py +++ b/src/google/adk/evaluation/safety_evaluator.py @@ -18,6 +18,7 @@ from typing import Optional from typing_extensions import override +from .eval_case import ConversationScenario from .eval_case import Invocation from .eval_metrics import EvalMetric from .eval_metrics import Interval @@ -65,11 +66,14 @@ class SafetyEvaluatorV1(Evaluator): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: from ..dependencies.vertexai import vertexai return _VertexAiEvalFacade( threshold=self._eval_metric.threshold, metric_name=vertexai.types.PrebuiltMetric.SAFETY, - ).evaluate_invocations(actual_invocations, expected_invocations) + ).evaluate_invocations( + actual_invocations, expected_invocations, conversation_scenario + ) diff --git a/src/google/adk/evaluation/simulation/per_turn_user_simulator_quality_v1.py b/src/google/adk/evaluation/simulation/per_turn_user_simulator_quality_v1.py index 5624bc0e..1e97cf75 100644 --- a/src/google/adk/evaluation/simulation/per_turn_user_simulator_quality_v1.py +++ b/src/google/adk/evaluation/simulation/per_turn_user_simulator_quality_v1.py @@ -290,10 +290,12 @@ class PerTurnUserSimulatorQualityV1(Evaluator): async def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - conversation_scenario: Optional[ConversationScenario], + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: - del expected_invocations + del expected_invocations # not used by this metric. + if conversation_scenario is None: + raise ValueError("conversation_scenario is needed by this metric.") # Evaluate the first invocation contains the given starting prompt. results = [ diff --git a/src/google/adk/evaluation/trajectory_evaluator.py b/src/google/adk/evaluation/trajectory_evaluator.py index 8c14d381..f795459c 100644 --- a/src/google/adk/evaluation/trajectory_evaluator.py +++ b/src/google/adk/evaluation/trajectory_evaluator.py @@ -22,6 +22,7 @@ from google.genai import types as genai_types from pydantic import ValidationError from typing_extensions import override +from .eval_case import ConversationScenario from .eval_case import get_all_tool_calls from .eval_case import Invocation from .eval_metrics import EvalMetric @@ -118,11 +119,13 @@ class TrajectoryEvaluator(Evaluator): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: """Returns EvaluationResult after performing evaluations using actual and expected invocations.""" if expected_invocations is None: raise ValueError("expected_invocations is needed by this metric.") + del conversation_scenario # not supported for per-invocation evaluation. total_tool_use_accuracy = 0.0 num_invocations = 0 diff --git a/src/google/adk/evaluation/vertex_ai_eval_facade.py b/src/google/adk/evaluation/vertex_ai_eval_facade.py index 92ac8fad..bb91054a 100644 --- a/src/google/adk/evaluation/vertex_ai_eval_facade.py +++ b/src/google/adk/evaluation/vertex_ai_eval_facade.py @@ -69,11 +69,12 @@ class _VertexAiEvalFacade(Evaluator): def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - _: Optional[ConversationScenario] = None, + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, ) -> EvaluationResult: if self._expected_invocations_required and expected_invocations is None: raise ValueError("expected_invocations is needed by this metric.") + del conversation_scenario # not supported for per-invocation evaluation. # If expected_invocation are not required by the metric and if they are not # supplied, we provide a list of None. diff --git a/tests/unittests/evaluation/test_local_eval_service.py b/tests/unittests/evaluation/test_local_eval_service.py index 66080828..da5a1736 100644 --- a/tests/unittests/evaluation/test_local_eval_service.py +++ b/tests/unittests/evaluation/test_local_eval_service.py @@ -26,6 +26,7 @@ from google.adk.evaluation.base_eval_service import InferenceConfig from google.adk.evaluation.base_eval_service import InferenceRequest from google.adk.evaluation.base_eval_service import InferenceResult from google.adk.evaluation.base_eval_service import InferenceStatus +from google.adk.evaluation.conversation_scenarios import ConversationScenario from google.adk.evaluation.eval_case import Invocation from google.adk.evaluation.eval_metrics import EvalMetric from google.adk.evaluation.eval_metrics import EvalMetricResult @@ -46,6 +47,7 @@ from google.adk.evaluation.metric_evaluator_registry import DEFAULT_METRIC_EVALU from google.adk.models.registry import LLMRegistry from google.genai import types as genai_types import pytest +from typing_extensions import override @pytest.fixture @@ -97,11 +99,13 @@ class FakeEvaluator(Evaluator): ), ) + @override def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - ): + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, + ) -> EvaluationResult: if expected_invocations is None: raise ValueError("expected_invocations is required for this metric.") per_invocation_results = [] @@ -136,11 +140,13 @@ class FakeSingleSidedEvaluator(Evaluator): ), ) + @override def evaluate_invocations( self, actual_invocations: list[Invocation], - expected_invocations: Optional[list[Invocation]], - ): + expected_invocations: Optional[list[Invocation]] = None, + conversation_scenario: Optional[ConversationScenario] = None, + ) -> EvaluationResult: per_invocation_results = [] for actual in actual_invocations: per_invocation_results.append(