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 <keyurj@google.com>
PiperOrigin-RevId: 850462752
This commit is contained in:
Keyur Joshi
2025-12-30 11:07:25 -08:00
committed by Copybara-Service
parent 38a30a44d2
commit 0918b647df
11 changed files with 48 additions and 24 deletions
+2 -2
View File
@@ -62,8 +62,8 @@ class Evaluator(ABC):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
conversation_scenario: Optional[ConversationScenario], conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
"""Returns EvaluationResult after performing evaluations using actual and expected invocations. """Returns EvaluationResult after performing evaluations using actual and expected invocations.
@@ -60,11 +60,12 @@ class RougeEvaluator(Evaluator):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
_: Optional[ConversationScenario] = None, conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
if expected_invocations is None: if expected_invocations is None:
raise ValueError("expected_invocations is required for this metric.") raise ValueError("expected_invocations is required for this metric.")
del conversation_scenario # not used by this metric.
total_score = 0.0 total_score = 0.0
num_invocations = 0 num_invocations = 0
@@ -720,9 +720,11 @@ class HallucinationsV1Evaluator(Evaluator):
async def evaluate_invocations( async def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
_: Optional[ConversationScenario] = None, conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
del conversation_scenario # not used by this metric.
# expected_invocations are not required by the metric and if they are not # 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. # supplied, we provide a list of None to rest of the code.
expected_invocations = ( expected_invocations = (
@@ -730,6 +732,7 @@ class HallucinationsV1Evaluator(Evaluator):
if expected_invocations is None if expected_invocations is None
else expected_invocations else expected_invocations
) )
per_invocation_results = [] per_invocation_results = []
for actual, expected in zip(actual_invocations, expected_invocations): for actual, expected in zip(actual_invocations, expected_invocations):
step_evaluations = self._get_steps_to_evaluate(actual) step_evaluations = self._get_steps_to_evaluate(actual)
+3 -2
View File
@@ -118,11 +118,12 @@ class LlmAsJudge(Evaluator):
async def evaluate_invocations( async def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
_: Optional[ConversationScenario] = None, conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
if self._expected_invocations_required and expected_invocations is None: if self._expected_invocations_required and expected_invocations is None:
raise ValueError("expected_invocations is needed by this metric.") 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 # If expected_invocation are not required by the metric and if they are not
# supplied, we provide a list of None. # supplied, we provide a list of None.
@@ -370,6 +370,7 @@ class LocalEvalService(BaseEvalService):
return metric_evaluator.evaluate_invocations( return metric_evaluator.evaluate_invocations(
actual_invocations=actual_invocations, actual_invocations=actual_invocations,
expected_invocations=expected_invocations, expected_invocations=expected_invocations,
conversation_scenario=conversation_scenario,
) )
def _generate_final_eval_status( def _generate_final_eval_status(
@@ -100,8 +100,8 @@ class ResponseEvaluator(Evaluator):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
_: Optional[ConversationScenario] = None, conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
# If the metric is response_match_score, just use the RougeEvaluator. # If the metric is response_match_score, just use the RougeEvaluator.
if self._metric_name == PrebuiltMetrics.RESPONSE_MATCH_SCORE.value: 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) EvalMetric(metric_name=self._metric_name, threshold=self._threshold)
) )
return rouge_evaluator.evaluate_invocations( return rouge_evaluator.evaluate_invocations(
actual_invocations, expected_invocations actual_invocations, expected_invocations, conversation_scenario
) )
return _VertexAiEvalFacade( return _VertexAiEvalFacade(
threshold=self._threshold, threshold=self._threshold,
metric_name=self._metric_name, metric_name=self._metric_name,
expected_invocations_required=True, expected_invocations_required=True,
).evaluate_invocations(actual_invocations, expected_invocations) ).evaluate_invocations(
actual_invocations, expected_invocations, conversation_scenario
)
@@ -18,6 +18,7 @@ from typing import Optional
from typing_extensions import override from typing_extensions import override
from .eval_case import ConversationScenario
from .eval_case import Invocation from .eval_case import Invocation
from .eval_metrics import EvalMetric from .eval_metrics import EvalMetric
from .eval_metrics import Interval from .eval_metrics import Interval
@@ -65,11 +66,14 @@ class SafetyEvaluatorV1(Evaluator):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
from ..dependencies.vertexai import vertexai from ..dependencies.vertexai import vertexai
return _VertexAiEvalFacade( return _VertexAiEvalFacade(
threshold=self._eval_metric.threshold, threshold=self._eval_metric.threshold,
metric_name=vertexai.types.PrebuiltMetric.SAFETY, metric_name=vertexai.types.PrebuiltMetric.SAFETY,
).evaluate_invocations(actual_invocations, expected_invocations) ).evaluate_invocations(
actual_invocations, expected_invocations, conversation_scenario
)
@@ -290,10 +290,12 @@ class PerTurnUserSimulatorQualityV1(Evaluator):
async def evaluate_invocations( async def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
conversation_scenario: Optional[ConversationScenario], conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> 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. # Evaluate the first invocation contains the given starting prompt.
results = [ results = [
@@ -22,6 +22,7 @@ from google.genai import types as genai_types
from pydantic import ValidationError from pydantic import ValidationError
from typing_extensions import override from typing_extensions import override
from .eval_case import ConversationScenario
from .eval_case import get_all_tool_calls from .eval_case import get_all_tool_calls
from .eval_case import Invocation from .eval_case import Invocation
from .eval_metrics import EvalMetric from .eval_metrics import EvalMetric
@@ -118,11 +119,13 @@ class TrajectoryEvaluator(Evaluator):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
"""Returns EvaluationResult after performing evaluations using actual and expected invocations.""" """Returns EvaluationResult after performing evaluations using actual and expected invocations."""
if expected_invocations is None: if expected_invocations is None:
raise ValueError("expected_invocations is needed by this metric.") raise ValueError("expected_invocations is needed by this metric.")
del conversation_scenario # not supported for per-invocation evaluation.
total_tool_use_accuracy = 0.0 total_tool_use_accuracy = 0.0
num_invocations = 0 num_invocations = 0
@@ -69,11 +69,12 @@ class _VertexAiEvalFacade(Evaluator):
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
_: Optional[ConversationScenario] = None, conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult: ) -> EvaluationResult:
if self._expected_invocations_required and expected_invocations is None: if self._expected_invocations_required and expected_invocations is None:
raise ValueError("expected_invocations is needed by this metric.") 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 # If expected_invocation are not required by the metric and if they are not
# supplied, we provide a list of None. # supplied, we provide a list of None.
@@ -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 InferenceRequest
from google.adk.evaluation.base_eval_service import InferenceResult from google.adk.evaluation.base_eval_service import InferenceResult
from google.adk.evaluation.base_eval_service import InferenceStatus 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_case import Invocation
from google.adk.evaluation.eval_metrics import EvalMetric from google.adk.evaluation.eval_metrics import EvalMetric
from google.adk.evaluation.eval_metrics import EvalMetricResult 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.adk.models.registry import LLMRegistry
from google.genai import types as genai_types from google.genai import types as genai_types
import pytest import pytest
from typing_extensions import override
@pytest.fixture @pytest.fixture
@@ -97,11 +99,13 @@ class FakeEvaluator(Evaluator):
), ),
) )
@override
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], 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: if expected_invocations is None:
raise ValueError("expected_invocations is required for this metric.") raise ValueError("expected_invocations is required for this metric.")
per_invocation_results = [] per_invocation_results = []
@@ -136,11 +140,13 @@ class FakeSingleSidedEvaluator(Evaluator):
), ),
) )
@override
def evaluate_invocations( def evaluate_invocations(
self, self,
actual_invocations: list[Invocation], actual_invocations: list[Invocation],
expected_invocations: Optional[list[Invocation]], expected_invocations: Optional[list[Invocation]] = None,
): conversation_scenario: Optional[ConversationScenario] = None,
) -> EvaluationResult:
per_invocation_results = [] per_invocation_results = []
for actual in actual_invocations: for actual in actual_invocations:
per_invocation_results.append( per_invocation_results.append(