fix(evaluation): honor each metric's own eval_status in AgentEvaluator.evaluate()

[gaurav-gandhi-2411]

Related

Related: #6725 (distinct — that issue covers a metric that's permanently NOT_EVALUATED;
this covers a metric that reports a real PASSED/FAILED via eval_status, which gets
silently overridden with a backwards one). Not touched by #6682 or #6710 either.

🔴 Required Information

Describe the Bug:
AgentEvaluator._process_metrics_and_get_failures (agent_evaluator.py) recomputes
PASSED/FAILED itself from raw per-invocation scores via overall_score = statistics.mean(scores); overall_eval_status = PASSED if overall_score >= threshold else FAILED — hardcoding a higher-is-better convention for every registered metric
uniformly. This ignores the metric's own already-correct eval_status, which
LocalEvalService._evaluate_metric already copies onto each EvalMetricResult from
the Evaluator's own PerInvocationResult.eval_status (local_eval_service.py:463-469).
Any Evaluator that defines its metric as lower-is-better (a cost, latency, or
error-rate metric — PASSED when score <= threshold) has its own correct verdict
silently discarded and replaced with an inverted one.

I ran into this building a third-party ADK metric (adk-tracegauge, a per-invocation
dollar-cost gauge) that computes eval_status correctly (PASSED iff cost <= threshold).
Registering it and running via AgentEvaluator.evaluate() raises AssertionError for
every real threshold value, including ones where the run is genuinely and verifiably
under budget. adk eval/LocalEvalService, called directly, are unaffected — they read
eval_status directly and never hit this function.

Steps to Reproduce: minimal, self-contained repro below (no third-party packages)
— a synthetic lower-is-better metric (score=10.0, threshold=100.0, correctly PASSED
since 10.0 <= 100.0), registered via the documented
DEFAULT_METRIC_EVALUATOR_REGISTRY.register_evaluator mechanism, run through the real
AgentEvaluator.evaluate().

"""Minimal, self-contained repro for the AgentEvaluator.evaluate() threshold-
directionality bug in agent_evaluator.py::_process_metrics_and_get_failures.
"""
from __future__ import annotations
import asyncio, json, tempfile, uuid
from pathlib import Path
from google.adk.agents.llm_agent import LlmAgent
from google.adk.evaluation.eval_case import ConversationScenario, EvalCase, Invocation
from google.adk.evaluation.eval_metrics import EvalMetric, Interval, MetricInfo, MetricValueInfo
from google.adk.evaluation.eval_rubrics import RubricScore
from google.adk.evaluation.eval_set import EvalSet
from google.adk.evaluation.evaluator import EvalStatus, EvaluationResult, Evaluator, PerInvocationResult
from google.adk.evaluation.metric_evaluator_registry import DEFAULT_METRIC_EVALUATOR_REGISTRY
from google.adk.models.base_llm import BaseLlm
from google.adk.models.llm_response import LlmResponse
from google.genai import types as genai_types

METRIC_NAME = "synthetic_latency_ms"
_SCORE = 10.0
_THRESHOLD = 100.0  # score <= threshold -> PASSED; 10.0 <= 100.0 is genuinely True.

class _LowerIsBetterEvaluator(Evaluator):
    def __init__(self, *, eval_metric: EvalMetric) -> None:
        self._threshold = eval_metric.threshold

    def evaluate_invocations(self, actual_invocations, expected_invocations=None,
                              conversation_scenario=None) -> EvaluationResult:
        results = [
            PerInvocationResult(
                actual_invocation=inv, expected_invocation=None, score=_SCORE,
                eval_status=(EvalStatus.PASSED if _SCORE <= self._threshold else EvalStatus.FAILED),
                rubric_scores=[RubricScore(rubric_id="latency", score=_SCORE,
                    rationale=f"{_SCORE} <= {self._threshold} -> PASSED")],
            )
            for inv in actual_invocations
        ]
        return EvaluationResult(overall_score=_SCORE, overall_eval_status=EvalStatus.PASSED,
                                 per_invocation_results=results)

DEFAULT_METRIC_EVALUATOR_REGISTRY.register_evaluator(
    metric_info=MetricInfo(metric_name=METRIC_NAME,
        description="Synthetic lower-is-better metric for the directionality repro.",
        metric_value_info=MetricValueInfo(interval=Interval(min_value=0.0,
            max_value=1_000_000.0, open_at_max=True))),
    evaluator=_LowerIsBetterEvaluator,
)

class _FakeLlm(BaseLlm):
    model: str = "fake-model"
    @classmethod
    def supported_models(cls) -> list[str]:
        return ["fake-model"]
    async def generate_content_async(self, llm_request, stream: bool = False):
        yield LlmResponse(content=genai_types.Content(parts=[genai_types.Part(text="ok")], role="model"))

def _write_agent_module(tmp_path: Path) -> str:
    package_name = f"directionality_repro_{uuid.uuid4().hex}"
    package_dir = tmp_path / package_name
    package_dir.mkdir()
    (package_dir / "__init__.py").write_text("", encoding="utf-8")
    (package_dir / "agent.py").write_text(
        "from google.adk.agents.llm_agent import LlmAgent\n"
        "from google.adk.models.base_llm import BaseLlm\n"
        "from google.adk.models.llm_response import LlmResponse\n"
        "from google.genai import types as genai_types\n\n\n"
        "class _FakeLlm(BaseLlm):\n"
        "    model: str = 'fake-model'\n\n"
        "    @classmethod\n"
        "    def supported_models(cls):\n"
        "        return ['fake-model']\n\n"
        "    async def generate_content_async(self, llm_request, stream: bool = False):\n"
        "        yield LlmResponse(content=genai_types.Content(\n"
        "            parts=[genai_types.Part(text='ok')], role='model'))\n\n\n"
        "root_agent = LlmAgent(name='directionality_repro_agent', model=_FakeLlm(),\n"
        "                       instruction='Answer briefly.')\n",
        encoding="utf-8",
    )
    return f"{package_name}.agent"

async def main() -> None:
    with tempfile.TemporaryDirectory() as tmp_dir_str:
        tmp_path = Path(tmp_dir_str)
        module_name = _write_agent_module(tmp_path)
        eval_set = EvalSet(eval_set_id="directionality_repro_set", eval_cases=[EvalCase(
            eval_id="case_1", conversation=[Invocation(invocation_id="repro-invocation",
                user_content=genai_types.Content(parts=[genai_types.Part(text="hi")], role="user"))])])
        eval_dir = tmp_path / "eval_data"; eval_dir.mkdir()
        eval_set_path = eval_dir / "repro.test.json"
        eval_set_path.write_text(eval_set.model_dump_json(indent=2), encoding="utf-8")
        (eval_dir / "test_config.json").write_text(
            json.dumps({"criteria": {METRIC_NAME: _THRESHOLD}}), encoding="utf-8")
        import sys
        sys.path.insert(0, str(tmp_path))
        try:
            from google.adk.evaluation.agent_evaluator import AgentEvaluator
            try:
                await AgentEvaluator.evaluate(agent_module=module_name,
                    eval_dataset_file_path_or_dir=str(eval_set_path), num_runs=1,
                    print_detailed_results=True)
                print(f"RESULT: PASS -- evaluate() completed without raising "
                      f"(score={_SCORE} <= threshold={_THRESHOLD}, correctly PASSED)")
            except AssertionError as e:
                print(f"RESULT: BUG REPRODUCED -- evaluate() raised AssertionError: {e}")
        finally:
            sys.path.remove(str(tmp_path))
            sys.modules.pop(module_name, None)
            sys.modules.pop(module_name.rsplit(".", 1)[0], None)

if __name__ == "__main__":
    asyncio.run(main())

Expected Behavior: AgentEvaluator.evaluate() completes without raising, since the
metric's own eval_status is PASSED.

Observed Behavior:

|    | eval_status        |   score |   threshold | prompt   | ... |
|  0 | EvalStatus.PASSED   |      10 |         100 | hi       | ... |

RESULT: BUG REPRODUCED -- evaluate() raised AssertionError:
Following are all the test failures.
synthetic_latency_ms for <agent_module>.agent Failed. Expected 100.0, but got 10.0.

Note the printed per-invocation table already shows EvalStatus.PASSED — the bug is
specifically in the aggregate verdict computed afterward, not in what the metric
itself reported.

Environment: google-adk==2.6.3 (also reproduces on current upstream/main,
3fa71b6 — the bug is unchanged there). Windows 11, Python 3.11/3.13.

Why this fix

Aggregate PASSED/FAILED/NOT_EVALUATED from each invocation's own eval_status instead
of recomputing one from the mean score. Mirrors
LocalEvalService._generate_final_eval_status's existing aggregation convention
(FAILED takes precedence over everything; otherwise PASSED if any result passed; else
NOT_EVALUATED), applied here across a metric's own invocations instead of across an
eval case's metrics — same three-way logic already established elsewhere in this
file, not a new concept. Smaller and more surgical than introducing a new "polarity"
field on Evaluator/BaseCriterion: eval_status is already the one place a metric's own
pass/fail semantics are recorded, this just stops discarding it. overall_score
(mean of available scores) is kept for the human-readable failure message and
_print_details — a fine descriptive statistic, just no longer used to decide polarity.

Changes

• src/google/adk/evaluation/agent_evaluator.py: _process_metrics_and_get_failures
  now aggregates from each invocation's own eval_metric_result.eval_status.
• tests/unittests/evaluation/test_agent_evaluator.py: new TestProcessMetricsAndGetFailures
  (6 tests) — the directionality repro, a higher-is-better regression-safety-net pair,
  FAILED-takes-precedence-across-invocations, and the pre-existing
  NOT_EVALUATED-with-no-scores-still-reported-as-failure behavior (deliberately unchanged).

Testing Plan

Unit Tests:

• Added TestProcessMetricsAndGetFailures (6 tests).
• All unit tests pass locally.
• 3 of the 6 new tests fail against pre-fix code (verified by stashing the fix) —
  confirming they actually catch the bug.

Manual E2E: ran the repro script above directly against this repo's own editable
install (confirmed via python -c "import google.adk.evaluation.agent_evaluator as m; print(m.__file__)" pointing at src/). Pre-fix: BUG REPRODUCED. Post-fix: RESULT: PASS.

pytest tests/unittests/evaluation/test_agent_evaluator.py -k TestProcessMetricsAndGetFailures: 6 passed
pytest tests/unittests/evaluation/: 844 passed
pre-commit (ruff, isort, pyink, addlicense, codespell): all pass on changed files

Risk & rollback

Confined to one function's internal aggregation logic; no change to
EvalMetricResult/EvalStatus/any public schema. Behavior is unchanged for every metric
that already reports eval_status per ADK's own established per-invocation convention
(every built-in ADK metric does) — only metrics whose aggregate verdict was previously
being silently recomputed backwards are affected, and only for the better. Purely
additive/corrective; revert is a clean single-commit revert.

[varunbiluri]

This changes more than metric polarity: it replaces the existing aggregate contract ( compared with the criterion threshold) with “any failed invocation fails the metric.” For an ordinary higher-is-better metric with per-invocation scores and threshold , the current code passes because the mean is ; this patch fails because one invocation is FAILED. That is a backwards-incompatible result change for multi-invocation evaluations, and the new tests only cover all-pass/all-fail plus one mixed case that asserts the new behavior rather than preserving the old aggregate semantics. Please retain aggregation-over-the-mean and represent directionality explicitly (or otherwise apply the evaluator's polarity to the aggregate) instead of deriving the aggregate verdict from per-invocation statuses.

[varunbiluri]

This changes more than metric polarity: it replaces the existing aggregate contract of comparing mean scores with the criterion threshold with an any-failed-invocation rule. For a normal higher-is-better metric with invocation scores 0 and 1 at threshold 0.5, current behavior passes because the mean is 0.5; this patch fails because one invocation is FAILED. That is a backwards-incompatible result change for multi-invocation evaluations. Please preserve aggregation over the mean and represent directionality explicitly, or otherwise apply evaluator polarity to the aggregate, rather than deriving the aggregate verdict from per-invocation statuses.

[gaurav-gandhi-2411]

You're right, and I appreciate the specificity of the example — it made the bug obvious.

The original PR replaced mean-vs-threshold aggregation with an any-invocation-fails rule, which is a real behavior change for any multi-invocation eval of a single metric, independent of polarity. That wasn't the intent — the actual bug was narrower (a hardcoded higher-is-better >= comparison that's backwards for a lower-is-better metric) and didn't need a new aggregation contract to fix.

Pushed a revision that keeps overall_score = mean(scores) and the mean-vs-threshold comparison exactly as it was, and only corrects which comparison operator applies — inferred from one invocation's own (score, eval_status) pair, since Evaluator/EvalMetric don't carry an explicit polarity field anywhere in this module. Added the counter-example from your comment as a test (response_match_score, scores [0.0, 1.0], threshold 0.5 — mean clears the threshold, so it now correctly passes), plus its lower-is-better mirror, plus the corresponding "mean genuinely fails" cases for both. The old test_failed_takes_precedence_over_passed_across_invocations test happened to still pass under the any-fails rule for an unrelated reason (its failing score was extreme enough to also fail on the mean) — replaced it since its name asserted a rule that's no longer the actual contract.