From 4a79cd36506e582c00eec571dbc02aac32826228 Mon Sep 17 00:00:00 2001 From: George Weale Date: Mon, 17 Aug 2026 23:12:01 +0000 Subject: [PATCH] fix: Scope custom eval metrics per registry and trust only the config path (v1) Ports "fix: scope custom metrics per registry and trust only the config path" to the v1 branch. MetricEvaluatorRegistry declared _registry in the class body, so every instance ever constructed shared one dict and a custom metric registered anywhere in the process resolved from every other registry. It is now built in __init__, and each new instance is seeded with the standard metrics. get_evaluator read the module path for a custom metric off the EvalMetric it was handed, which on a serving deployment is built from the request body and then passed to importlib.import_module. It now uses only a path recorded on the registry at registration time, or one carried on the metric as a private attribute written by get_eval_metrics_from_config. The public custom_function_path field is no longer consulted. Adapted for v1: upstream reworks register_custom_metrics_from_config, which does not exist on this branch. The equivalent registration loop in cli_tools_click.py is updated instead, so `adk eval` passes the config's declared path through when it registers a custom metric. The RubricBasedMultiTurnTrajectory registration is omitted, as that evaluator is not on v1. Breaking change. A custom metric whose function path was not declared in an eval config now raises NotFoundError from get_evaluator instead of importing the path supplied with the metric. Code that reads or writes MetricEvaluatorRegistry._registry on the class, or that relies on one registry instance seeing another's registrations, also breaks. --- src/google/adk/cli/cli_tools_click.py | 4 +- src/google/adk/errors/not_found_error.py | 4 +- src/google/adk/evaluation/eval_config.py | 32 ++- src/google/adk/evaluation/eval_metrics.py | 6 + .../evaluation/metric_evaluator_registry.py | 69 ++++- .../cli/utils/test_cli_tools_click.py | 53 ++++ .../test_metric_evaluator_registry.py | 257 ++++++++++++++++++ 7 files changed, 400 insertions(+), 25 deletions(-) diff --git a/src/google/adk/cli/cli_tools_click.py b/src/google/adk/cli/cli_tools_click.py index fda251da10a..a80efa2646c 100644 --- a/src/google/adk/cli/cli_tools_click.py +++ b/src/google/adk/cli/cli_tools_click.py @@ -930,8 +930,8 @@ def cli_eval( metric_name=metric_name, description=config.description ) - metric_evaluator_registry.register_evaluator( - metric_info, _CustomMetricEvaluator + metric_evaluator_registry._register( # pylint: disable=protected-access + metric_info, _CustomMetricEvaluator, config.code_config.name ) eval_service = LocalEvalService( diff --git a/src/google/adk/errors/not_found_error.py b/src/google/adk/errors/not_found_error.py index 4c7ff22d7ab..faaaf316b77 100644 --- a/src/google/adk/errors/not_found_error.py +++ b/src/google/adk/errors/not_found_error.py @@ -18,7 +18,9 @@ class NotFoundError(Exception): """Represents an error that occurs when an entity is not found.""" - def __init__(self, message="The requested item was not found."): + def __init__( + self, message: str = "The requested item was not found." + ) -> None: """Initializes the NotFoundError exception. Args: diff --git a/src/google/adk/evaluation/eval_config.py b/src/google/adk/evaluation/eval_config.py index ead2303cebd..c19d30da3ab 100644 --- a/src/google/adk/evaluation/eval_config.py +++ b/src/google/adk/evaluation/eval_config.py @@ -192,22 +192,18 @@ def get_eval_metrics_from_config(eval_config: EvalConfig) -> list[EvalMetric]: custom_function_path = config.code_config.name if isinstance(criterion, float): - eval_metric_list.append( - EvalMetric( - metric_name=metric_name, - threshold=criterion, - criterion=BaseCriterion(threshold=criterion), - custom_function_path=custom_function_path, - ) + eval_metric = EvalMetric( + metric_name=metric_name, + threshold=criterion, + criterion=BaseCriterion(threshold=criterion), + custom_function_path=custom_function_path, ) elif isinstance(criterion, BaseCriterion): - eval_metric_list.append( - EvalMetric( - metric_name=metric_name, - threshold=criterion.threshold, - criterion=criterion, - custom_function_path=custom_function_path, - ) + eval_metric = EvalMetric( + metric_name=metric_name, + threshold=criterion.threshold, + criterion=criterion, + custom_function_path=custom_function_path, ) else: raise ValueError( @@ -215,4 +211,12 @@ def get_eval_metrics_from_config(eval_config: EvalConfig) -> list[EvalMetric]: " supported." ) + # The config is written by the developer running the eval, so the path it + # declares is the one honoured when the metric runs. It travels with the + # metric rather than in a registry keyed by metric name, so two apps in + # one process can declare the same metric name and each still gets its + # own function. + eval_metric._config_custom_function_path = custom_function_path # pylint: disable=protected-access + eval_metric_list.append(eval_metric) + return eval_metric_list diff --git a/src/google/adk/evaluation/eval_metrics.py b/src/google/adk/evaluation/eval_metrics.py index 50c3473c3ae..e5f6acd4e21 100644 --- a/src/google/adk/evaluation/eval_metrics.py +++ b/src/google/adk/evaluation/eval_metrics.py @@ -25,6 +25,7 @@ from pydantic import ConfigDict from pydantic import Field from pydantic import field_validator +from pydantic import PrivateAttr from pydantic.json_schema import SkipJsonSchema from typing_extensions import TypeAlias @@ -282,6 +283,11 @@ class EvalMetric(EvalBaseModel): description="""Path to custom function, if this is a custom metric.""", ) + # The path declared for this metric in the eval config it was built from. + # Private, so that a metric parsed from an inbound payload cannot carry one: + # the public field above is settable by whoever built that payload. + _config_custom_function_path: Optional[str] = PrivateAttr(default=None) + class EvalMetricResultDetails(EvalBaseModel): rubric_scores: Optional[list[RubricScore]] = Field( diff --git a/src/google/adk/evaluation/metric_evaluator_registry.py b/src/google/adk/evaluation/metric_evaluator_registry.py index b005bd36e6b..b242aedb3ab 100644 --- a/src/google/adk/evaluation/metric_evaluator_registry.py +++ b/src/google/adk/evaluation/metric_evaluator_registry.py @@ -15,6 +15,7 @@ from __future__ import annotations import logging +from typing import Optional from ..errors.not_found_error import NotFoundError from ..utils.feature_decorator import experimental @@ -53,7 +54,15 @@ class MetricEvaluatorRegistry: """A registry for metric Evaluators.""" - _registry: dict[str, tuple[type[Evaluator], MetricInfo]] = {} + def __init__(self) -> None: + # Each registry instance owns its mappings, so a custom metric registered + # for one app is not resolvable from another app's registry. The standard + # metrics are seeded into every instance, as they are the same everywhere. + self._registry: dict[str, tuple[type[Evaluator], MetricInfo]] = {} + # Module path of the custom function backing a metric, keyed by metric + # name. Only ever written from an eval config. + self._custom_function_paths: dict[str, str] = {} + _register_standard_metrics(self) def get_evaluator(self, eval_metric: EvalMetric) -> Evaluator: """Returns an Evaluator for the given metric. @@ -69,14 +78,34 @@ def get_evaluator(self, eval_metric: EvalMetric) -> Evaluator: if eval_metric.metric_name not in self._registry: raise NotFoundError(f"{eval_metric.metric_name} not found in registry.") - evaluator_type = self._registry[eval_metric.metric_name][0] + evaluator_type, _ = self._registry[eval_metric.metric_name] if issubclass(evaluator_type, _CustomMetricEvaluator): + custom_function_path = self._custom_function_path(eval_metric) + if custom_function_path is None: + raise NotFoundError( + f"No custom function registered for {eval_metric.metric_name}." + ) return evaluator_type( eval_metric=eval_metric, - custom_function_path=eval_metric.custom_function_path, + custom_function_path=custom_function_path, ) return evaluator_type(eval_metric=eval_metric) + def _custom_function_path(self, eval_metric: EvalMetric) -> Optional[str]: + """Returns the module path to import for a custom metric, if known. + + Both sources are eval config entries: one recorded when the metric was + registered from a config, the other carried on a metric built from a + config. The `custom_function_path` field on the incoming metric is not + consulted, as it can be set by whoever built the request. + + Args: + eval_metric: The metric whose custom function is being resolved. + """ + if path := self._custom_function_paths.get(eval_metric.metric_name): + return path + return eval_metric._config_custom_function_path # pylint: disable=protected-access + def register_evaluator( self, metric_info: MetricInfo, @@ -86,6 +115,25 @@ def register_evaluator( If a mapping already exist, then it is updated. """ + self._register(metric_info, evaluator, custom_function_path=None) + + def _register( + self, + metric_info: MetricInfo, + evaluator: type[Evaluator], + custom_function_path: Optional[str], + ) -> None: + """Registers an evaluator, along with the function path it may need. + + A path already recorded for the metric is kept when this registration does + not carry one, so re-registering an evaluator does not drop it. + + Args: + metric_info: Info for the metric the evaluator is registered against. + evaluator: The evaluator class to register. + custom_function_path: Module path of the function backing a custom + metric, taken from an eval config, or None. + """ metric_name = metric_info.metric_name if metric_name in self._registry: logger.info( @@ -96,6 +144,8 @@ def register_evaluator( ) self._registry[str(metric_name)] = (evaluator, metric_info) + if custom_function_path is not None: + self._custom_function_paths[str(metric_name)] = custom_function_path def get_registered_metrics( self, @@ -107,10 +157,10 @@ def get_registered_metrics( ] -def _get_default_metric_evaluator_registry() -> MetricEvaluatorRegistry: - """Returns an instance of MetricEvaluatorRegistry with standard metrics already registered in it.""" - metric_evaluator_registry = MetricEvaluatorRegistry() - +def _register_standard_metrics( + metric_evaluator_registry: MetricEvaluatorRegistry, +) -> None: + """Registers the metrics that ship with ADK into the given registry.""" metric_evaluator_registry.register_evaluator( metric_info=TrajectoryEvaluatorMetricInfoProvider().get_metric_info(), evaluator=TrajectoryEvaluator, @@ -165,7 +215,10 @@ def _get_default_metric_evaluator_registry() -> MetricEvaluatorRegistry: evaluator=PerTurnUserSimulatorQualityV1, ) - return metric_evaluator_registry + +def _get_default_metric_evaluator_registry() -> MetricEvaluatorRegistry: + """Returns an instance of MetricEvaluatorRegistry with standard metrics already registered in it.""" + return MetricEvaluatorRegistry() DEFAULT_METRIC_EVALUATOR_REGISTRY = _get_default_metric_evaluator_registry() diff --git a/tests/unittests/cli/utils/test_cli_tools_click.py b/tests/unittests/cli/utils/test_cli_tools_click.py index ad47c9ecbe6..19c2542a5b0 100644 --- a/tests/unittests/cli/utils/test_cli_tools_click.py +++ b/tests/unittests/cli/utils/test_cli_tools_click.py @@ -18,6 +18,7 @@ import builtins import json +import math from pathlib import Path from types import SimpleNamespace from typing import Any @@ -32,9 +33,11 @@ from google.adk.agents.base_agent import BaseAgent from google.adk.cli import cli_tools_click from google.adk.evaluation.eval_case import EvalCase +from google.adk.evaluation.eval_metrics import EvalMetric from google.adk.evaluation.eval_set import EvalSet from google.adk.evaluation.local_eval_set_results_manager import LocalEvalSetResultsManager from google.adk.evaluation.local_eval_sets_manager import LocalEvalSetsManager +from google.adk.evaluation.metric_evaluator_registry import DEFAULT_METRIC_EVALUATOR_REGISTRY from pydantic import BaseModel import pytest @@ -668,6 +671,56 @@ def test_cli_eval_with_eval_set_id( assert len(eval_set_results) == 2 +def test_cli_eval_registers_the_custom_metric_path_from_the_config( + mock_load_eval_set_from_file, + mock_get_root_agent, + tmp_path, +): + """A custom metric declared in the config resolves without a metric path.""" + metric_name = "custom_metric_from_cli_config" + agent_path = tmp_path / "my_agent" + agent_path.mkdir() + (agent_path / "__init__.py").touch() + + eval_set_file = tmp_path / "my_evals.json" + eval_set_file.write_text("{}") + mock_load_eval_set_from_file.return_value = EvalSet( + eval_set_id="my_evals", + eval_cases=[EvalCase(eval_id="case1", conversation=[])], + ) + + config_file = tmp_path / "eval_config.json" + config_file.write_text( + json.dumps({ + "criteria": {"tool_trajectory_avg_score": 1.0}, + "customMetrics": {metric_name: {"codeConfig": {"name": "math.sqrt"}}}, + }) + ) + + try: + result = CliRunner().invoke( + cli_tools_click.cli_eval, + [ + str(agent_path), + str(eval_set_file), + "--config_file_path", + str(config_file), + ], + ) + + assert result.exit_code == 0 + # The metric carries no path of its own; the config's path is the one used. + evaluator = DEFAULT_METRIC_EVALUATOR_REGISTRY.get_evaluator( + EvalMetric(metric_name=metric_name, threshold=0.5) + ) + assert evaluator._metric_function is math.sqrt + finally: + DEFAULT_METRIC_EVALUATOR_REGISTRY._registry.pop(metric_name, None) + DEFAULT_METRIC_EVALUATOR_REGISTRY._custom_function_paths.pop( + metric_name, None + ) + + def test_cli_create_eval_set(tmp_path: Path): app_name = "test_app" eval_set_id = "test_eval_set" diff --git a/tests/unittests/evaluation/test_metric_evaluator_registry.py b/tests/unittests/evaluation/test_metric_evaluator_registry.py index cb5361f4274..de054f59966 100644 --- a/tests/unittests/evaluation/test_metric_evaluator_registry.py +++ b/tests/unittests/evaluation/test_metric_evaluator_registry.py @@ -14,13 +14,22 @@ from __future__ import annotations +import math + +from google.adk.agents.common_configs import CodeConfig from google.adk.errors.not_found_error import NotFoundError +from google.adk.evaluation.custom_metric_evaluator import _CustomMetricEvaluator +from google.adk.evaluation.eval_config import CustomMetricConfig +from google.adk.evaluation.eval_config import EvalConfig +from google.adk.evaluation.eval_config import get_eval_metrics_from_config +from google.adk.evaluation.eval_metrics import BaseCriterion from google.adk.evaluation.eval_metrics import EvalMetric from google.adk.evaluation.eval_metrics import Interval from google.adk.evaluation.eval_metrics import MetricInfo from google.adk.evaluation.eval_metrics import MetricValueInfo from google.adk.evaluation.eval_metrics import PrebuiltMetrics from google.adk.evaluation.evaluator import Evaluator +from google.adk.evaluation.metric_evaluator_registry import DEFAULT_METRIC_EVALUATOR_REGISTRY from google.adk.evaluation.metric_evaluator_registry import FinalResponseMatchV2EvaluatorMetricInfoProvider from google.adk.evaluation.metric_evaluator_registry import HallucinationsV1EvaluatorMetricInfoProvider from google.adk.evaluation.metric_evaluator_registry import MetricEvaluatorRegistry @@ -29,7 +38,9 @@ from google.adk.evaluation.metric_evaluator_registry import RubricBasedFinalResponseQualityV1EvaluatorMetricInfoProvider from google.adk.evaluation.metric_evaluator_registry import RubricBasedToolUseV1EvaluatorMetricInfoProvider from google.adk.evaluation.metric_evaluator_registry import SafetyEvaluatorV1MetricInfoProvider +from google.adk.evaluation.metric_evaluator_registry import TrajectoryEvaluator from google.adk.evaluation.metric_evaluator_registry import TrajectoryEvaluatorMetricInfoProvider +from pydantic import ValidationError import pytest _DUMMY_METRIC_NAME = "dummy_metric_name" @@ -104,6 +115,44 @@ def test_register_evaluator_updates_existing(self, registry): _ANOTHER_DUMMY_METRIC_INFO, ) + def test_a_new_registry_has_the_standard_metrics(self): + registry = MetricEvaluatorRegistry() + + registered = { + metric_info.metric_name + for metric_info in registry.get_registered_metrics() + } + assert { + PrebuiltMetrics.TOOL_TRAJECTORY_AVG_SCORE.value, + PrebuiltMetrics.RESPONSE_MATCH_SCORE.value, + PrebuiltMetrics.SAFETY_V1.value, + PrebuiltMetrics.FINAL_RESPONSE_MATCH_V2.value, + PrebuiltMetrics.HALLUCINATIONS_V1.value, + } <= registered + assert isinstance( + registry.get_evaluator( + EvalMetric( + metric_name=PrebuiltMetrics.TOOL_TRAJECTORY_AVG_SCORE.value, + threshold=0.5, + ) + ), + TrajectoryEvaluator, + ) + + def test_registrations_are_not_shared_across_instances(self, registry): + registry.register_evaluator( + _DUMMY_METRIC_INFO, + DummyEvaluator, + ) + + other_registry = MetricEvaluatorRegistry() + + assert _DUMMY_METRIC_NAME not in other_registry._registry + with pytest.raises(NotFoundError): + other_registry.get_evaluator( + EvalMetric(metric_name=_DUMMY_METRIC_NAME, threshold=0.5) + ) + def test_get_evaluator(self, registry): registry.register_evaluator( _DUMMY_METRIC_INFO, @@ -119,6 +168,214 @@ def test_get_evaluator_not_found(self, registry): registry.get_evaluator(eval_metric) +def _custom_metric_info(metric_name: str) -> MetricInfo: + return MetricInfo( + metric_name=metric_name, + description="Custom metric registered by hand.", + metric_value_info=MetricValueInfo( + interval=Interval(min_value=0.0, max_value=1.0) + ), + ) + + +class TestCustomFunctionPathResolution: + """How the module path imported for a custom metric is chosen. + + The path has to come from an eval config, and never from the metric handed + to `get_evaluator`, which on a served eval is built from the request. + """ + + _CUSTOM_METRIC_NAME = "custom_metric_for_resolution_test" + + @pytest.fixture + def registry(self): + return MetricEvaluatorRegistry() + + def test_a_path_recorded_at_registration_is_used(self, registry): + # This is what `adk eval` does with the custom metrics in an eval config. + registry._register( + _custom_metric_info(self._CUSTOM_METRIC_NAME), + _CustomMetricEvaluator, + "math.sqrt", + ) + + evaluator = registry.get_evaluator( + EvalMetric(metric_name=self._CUSTOM_METRIC_NAME, threshold=0.5) + ) + + assert evaluator._metric_function is math.sqrt + + def test_ignores_custom_function_path_on_the_eval_metric(self, registry): + registry._register( + _custom_metric_info(self._CUSTOM_METRIC_NAME), + _CustomMetricEvaluator, + "math.sqrt", + ) + + evaluator = registry.get_evaluator( + EvalMetric( + metric_name=self._CUSTOM_METRIC_NAME, + threshold=0.5, + custom_function_path="math.floor", + ) + ) + + assert evaluator._metric_function is math.sqrt + + def test_a_recorded_path_survives_re_registration(self, registry): + registry._register( + _custom_metric_info(self._CUSTOM_METRIC_NAME), + _CustomMetricEvaluator, + "math.sqrt", + ) + registry.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + + evaluator = registry.get_evaluator( + EvalMetric(metric_name=self._CUSTOM_METRIC_NAME, threshold=0.5) + ) + + assert evaluator._metric_function is math.sqrt + + def test_hand_registered_evaluator_uses_the_float_criterion_config( + self, registry + ): + registry.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + eval_config = EvalConfig( + criteria={self._CUSTOM_METRIC_NAME: 0.8}, + custom_metrics={ + self._CUSTOM_METRIC_NAME: CustomMetricConfig( + code_config=CodeConfig(name="math.sqrt") + ) + }, + ) + + [eval_metric] = get_eval_metrics_from_config(eval_config) + evaluator = registry.get_evaluator(eval_metric) + + assert evaluator._metric_function is math.sqrt + + def test_hand_registered_evaluator_uses_the_criterion_object_config( + self, registry + ): + registry.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + eval_config = EvalConfig( + criteria={self._CUSTOM_METRIC_NAME: BaseCriterion(threshold=1.0)}, + custom_metrics={ + self._CUSTOM_METRIC_NAME: CustomMetricConfig( + code_config=CodeConfig(name="math.sqrt") + ) + }, + ) + + [eval_metric] = get_eval_metrics_from_config(eval_config) + evaluator = registry.get_evaluator(eval_metric) + + assert evaluator._metric_function is math.sqrt + + def test_each_metric_resolves_its_own_config_function(self, registry): + other_metric_name = "another_custom_metric_for_resolution_test" + for metric_name in (self._CUSTOM_METRIC_NAME, other_metric_name): + registry.register_evaluator( + _custom_metric_info(metric_name), _CustomMetricEvaluator + ) + eval_config = EvalConfig( + criteria={self._CUSTOM_METRIC_NAME: 1.0, other_metric_name: 1.0}, + custom_metrics={ + self._CUSTOM_METRIC_NAME: CustomMetricConfig( + code_config=CodeConfig(name="math.sqrt") + ), + other_metric_name: CustomMetricConfig( + code_config=CodeConfig(name="math.floor") + ), + }, + ) + + evaluators = { + eval_metric.metric_name: registry.get_evaluator(eval_metric) + for eval_metric in get_eval_metrics_from_config(eval_config) + } + + assert evaluators[self._CUSTOM_METRIC_NAME]._metric_function is math.sqrt + assert evaluators[other_metric_name]._metric_function is math.floor + + def test_hand_registration_on_the_default_registry_resolves(self): + eval_config = EvalConfig( + criteria={self._CUSTOM_METRIC_NAME: 1.0}, + custom_metrics={ + self._CUSTOM_METRIC_NAME: CustomMetricConfig( + code_config=CodeConfig(name="math.sqrt") + ) + }, + ) + + try: + DEFAULT_METRIC_EVALUATOR_REGISTRY.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + + [eval_metric] = get_eval_metrics_from_config(eval_config) + evaluator = DEFAULT_METRIC_EVALUATOR_REGISTRY.get_evaluator(eval_metric) + + assert evaluator._metric_function is math.sqrt + finally: + DEFAULT_METRIC_EVALUATOR_REGISTRY._registry.pop( + self._CUSTOM_METRIC_NAME, None + ) + DEFAULT_METRIC_EVALUATOR_REGISTRY._custom_function_paths.pop( + self._CUSTOM_METRIC_NAME, None + ) + + def test_a_metric_without_a_config_is_rejected(self, registry): + registry.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + + with pytest.raises(NotFoundError): + registry.get_evaluator( + EvalMetric( + metric_name=self._CUSTOM_METRIC_NAME, + threshold=0.5, + custom_function_path="math.floor", + ) + ) + + def test_a_config_path_does_not_carry_to_another_configs_metric( + self, registry + ): + registry.register_evaluator( + _custom_metric_info(self._CUSTOM_METRIC_NAME), _CustomMetricEvaluator + ) + trusted_config = EvalConfig( + criteria={self._CUSTOM_METRIC_NAME: 1.0}, + custom_metrics={ + self._CUSTOM_METRIC_NAME: CustomMetricConfig( + code_config=CodeConfig(name="math.sqrt") + ) + }, + ) + [trusted_metric] = get_eval_metrics_from_config(trusted_config) + assert registry.get_evaluator(trusted_metric)._metric_function is math.sqrt + + with pytest.raises(NotFoundError): + registry.get_evaluator( + EvalMetric(metric_name=self._CUSTOM_METRIC_NAME, threshold=0.5) + ) + + def test_the_config_path_cannot_be_set_through_the_metric(self): + with pytest.raises(ValidationError): + EvalMetric.model_validate({ + "metric_name": self._CUSTOM_METRIC_NAME, + "threshold": 0.5, + "_config_custom_function_path": "math.floor", + }) + + class TestMetricInfoProviders: """Test cases for MetricInfoProviders."""