evaluate_eval_set silently passes when eval_results_by_eval_id is empty (zero eval cases or num_runs=0)

[gaurav-gandhi-2411]

🔴 Required Information

Describe the Bug:
AgentEvaluator.evaluate_eval_set reports overall success on an eval set that
was never actually evaluated. evaluate_eval_set builds eval_results_by_eval_id
from the eval set, iterates it to populate a failures list, and finishes with
assert not failures. If eval_results_by_eval_id ends up empty for any
reason, the loop body never runs, failures stays [], and the assert passes
— the function returns normally with no signal that zero cases were evaluated.

Two independent, ordinary public-API calls reach this with no validation
bypass:

1. An EvalSet with eval_cases=[] — EvalSet.eval_cases has no
   min_length/validator, so this constructs cleanly.
2. evaluate_eval_set(..., num_runs=0) on a non-empty eval set — num_runs
   has no bounds check anywhere in the file, and inference_requests = [InferenceRequest(...)] * num_runs becomes [].

Both silently produce the same "nothing was evaluated, but nothing failed
either" outcome.

Steps to Reproduce: minimal, self-contained repro (writes a real temp
agent module, no third-party packages):

"""Minimal, self-contained repro for AgentEvaluator.evaluate_eval_set's
vacuous pass on an empty eval set (agent_evaluator.py, evaluate_eval_set).
"""
from __future__ import annotations
import asyncio, tempfile, uuid
from pathlib import Path
from google.adk.evaluation.agent_evaluator import AgentEvaluator
from google.adk.evaluation.eval_config import EvalConfig
from google.adk.evaluation.eval_set import EvalSet


def _write_agent_module(tmp_path: Path) -> str:
    package_name = f"vacuous_pass_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=False):\n"
        "        yield LlmResponse(content=genai_types.Content(\n"
        "            parts=[genai_types.Part(text='ok')], role='model'))\n\n\n"
        "root_agent = LlmAgent(name='vacuous_pass_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)
        import sys
        sys.path.insert(0, str(tmp_path))
        try:
            empty_eval_set = EvalSet(eval_set_id="empty_set", eval_cases=[])
            result = await AgentEvaluator.evaluate_eval_set(
                agent_module=module_name,
                eval_set=empty_eval_set,
                eval_config=EvalConfig(criteria={"tool_trajectory_avg_score": 1.0}),
            )
            print(f"RESULT: completed WITHOUT raising. Returned: {result!r}")
            print("VERDICT: PASSED -- vacuous pass on zero eval cases.")
        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: evaluate_eval_set should raise or otherwise signal
that zero cases were evaluated — a "pass" should mean "every requested case
was checked and none failed," not "nothing was checked."

Observed Behavior:

RESULT: completed WITHOUT raising. Returned: None
VERDICT: PASSED -- vacuous pass on zero eval cases.

Process exit code 0. A second, independent repro (non-empty EvalSet,
num_runs=0) reproduces the identical outcome through the same mechanism.

Environment: google-adk main branch (this repo),
src/google/adk/evaluation/agent_evaluator.py:289 (the assert not failures
line), AgentEvaluator.evaluate_eval_set.

CI implication

Any test suite or CI gate that calls AgentEvaluator.evaluate_eval_set/.evaluate()
directly (rather than going through the adk eval CLI) will report a clean
pass for an eval set that was silently emptied by a config bug, a bad filter,
or an accidental num_runs=0, rather than failing loudly.

Note: this is a separate code path from the adk eval CLI command
(cli_tools_click.py::cli_eval, which builds its own eval_run_summary via
LocalEvalService directly and never calls AgentEvaluator) — confirmed by
reading both call chains. The CLI has an analogous "empty collection defaults
to a pass" shape in its own exit-code logic, which is unaffected by this
issue's scope either way.

I'm happy to send a PR for this if useful — the fix is a small, additive
check (e.g. treat an empty eval_results_by_eval_id as NOT_EVALUATED
rather than a silent pass), similar in shape to a couple of other
NOT_EVALUATED-masking fixes already proposed against this same subsystem.

[chelsealong]

Picking this one up now — opening a PR shortly. Flagging it here so nobody duplicates the work; if someone is already on it, say so and I will drop mine.

[gaurav-gandhi-2411]

A second real repro, run at commit 6d145180611956b2065704189517fd6a0ff1a063, confirms num_runs=0 is independently reachable through the public AgentEvaluator.evaluate_eval_set API: with a real, non-empty EvalSet containing one real EvalCase, evaluate_eval_set(..., num_runs=0) returns normally (exit code 0, no AssertionError) despite evaluating nothing. The mechanism is _get_eval_results_by_eval_id's [InferenceRequest(...)] * num_runs (agent_evaluator.py:749), which becomes [] when num_runs=0, leaving eval_results_by_eval_id and failures both empty so assert not failures (line 289) trivially passes; num_runs (agent_evaluator.py:134, 295) is a plain function-parameter default, not a Pydantic field, with no bounds check anywhere in evaluate_eval_set/evaluate.

The same "empty collection silently defaults to a pass" shape exists in the CLI path too: PR #6740 (open, not yet merged) adds total_failed = sum(pass_fail_count[1] for pass_fail_count in eval_run_summary.values()) then sys.exit(1 if total_failed else 0) to cli_eval — if eval_run_summary is empty, sum over an empty .values() is 0, so the process would still exit 0 with nothing evaluated, independent of any num_runs fix.

A sweep of every count/limit-like int parameter across src/google/adk/evaluation/*.py for missing lower-bound enforcement found this isn't isolated to num_runs: ConversationGenerationConfig.count (conversation_scenarios.py:83) silently generates zero scenarios via the generate_eval_cases CLI command at 0; EvaluateConfig.parallelism/InferenceConfig.parallelism (base_eval_service.py:46,72) feed asyncio.Semaphore(value=parallelism), where 0 hangs indefinitely rather than raising (verified directly — -1 raises immediately, 0 doesn't); and EvaluationGenerator.generate_responses's repeat_num (evaluation_generator.py:544) silently returns zero responses at 0. That's 6 floor-less count/limit parameters across the subpackage total, spanning both the direct-API and CLI surfaces, suggesting a shared validation layer would be a more durable fix than one-off guards on individual parameters.

[surajksharma07]

@gaurav-gandhi-2411 Empty eval_cases and num_runs=0 both hit the same vacuous assert-not-failures pass exactly as described.

As a workaround raising ValueError right after eval_results_by_eval_id is built (before the pass/fail check) turns both into a real error instead of a silent None, verified this locally and it holds for both cases. That's basically what #6952 already proposes so probably worth reviewing/testing that PR rather than duplicating it.
If you get a chance to try it against your own setup would be good to confirm it covers your case before anyone pushes further.

[gaurav-gandhi-2411]

Confirmed against a direct repro on #6952's branch (798d4beb): both triggers I reported (empty EvalSet.eval_cases, and num_runs=0 on a non-empty set) now raise ValueError instead of returning silently, since both produce an empty eval_results_by_eval_id, which #6952's guard catches before the pass/fail check runs. #6952 fully resolves the AgentEvaluator.evaluate_eval_set path. Three gaps from my earlier sweep are outside that function and remain open: (1) PR #6740 (not yet merged) computes total_failed = sum(pass_fail_count[1] for pass_fail_count in eval_run_summary.values()) over the CLI's own eval_run_summary and exits 0 when it's empty — a blind spot in that PR's own diff, not on main today; (2) EvaluateConfig.parallelism/InferenceConfig.parallelism (base_eval_service.py:46,72) at 0 feed asyncio.Semaphore(value=0) (local_eval_service.py:186,216), which hangs indefinitely on .acquire() rather than raising, verified directly; (3) ConversationGenerationConfig.count (conversation_scenarios.py:83), reachable via the CLI's scenario-generation command, has no local floor before being forwarded to the Vertex AI scenario API. JudgeModelOptions.parallelism_limit (eval_metrics.py:110), previously on this list, now has ge=1 and is closed. Happy to open a PR covering the remaining three.

[gaurav-gandhi-2411]

Correcting two of the three gaps from my last comment, since I filed this issue and want the record accurate.

1. The CLI path was never a live bug on `main` — it was a gap in my own open PR fix(cli): adk eval process exit code now reflects PASSED/FAILED #6740's diff (`eval_run_summary` empty → `sys.exit(0)`). Now fixed there: fix(cli): adk eval process exit code now reflects PASSED/FAILED #6740 (comment).
2. `ConversationGenerationConfig.count` (`conversation_scenarios.py:83`): ADK itself has no floor on it, but the value is forwarded to an external Vertex AI scenario-generation API and I haven't verified what that API actually does with `count=0`. Withdrawing this as a confirmed gap — it's unverified, not established.

The one gap that stands, reaffirmed with fresh evidence: `EvaluateConfig.parallelism` / `InferenceConfig.parallelism` (`base_eval_service.py:46,72`) have no `ge=` constraint and both feed `asyncio.Semaphore(value=parallelism)` directly (`local_eval_service.py:186,216`) with no clamping in between. At `0` this hangs forever rather than raising:

```
Semaphore(-1) raised ValueError immediately: ValueError('Semaphore initial value must be >= 0')
Semaphore(0) constructed without error: <asyncio.locks.Semaphore object at 0x0000019077E3ED50 [locked]>
Semaphore(0).acquire() TIMED OUT after 5.0s (hung, as expected)
```
(verified against pinned `main` `c506ddf3`)