Skip to content

Commit 8ca9128

Browse files
GWealecopybara-github
authored andcommitted
fix: reject incomplete evaluation inputs
Several evaluators paired actual and expected invocations with a plain zip, so extra turns were silently dropped and a truncated run could still pass. The rubric parser likewise discarded incomplete fields, and empty multi-turn inputs could crash. This validates invocation counts and pairs strictly, rejects partially parsed rubric responses, and returns a not-evaluated result for empty multi-turn inputs. Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 949253981
1 parent 2919bf5 commit 8ca9128

13 files changed

Lines changed: 92 additions & 14 deletions

src/google/adk/evaluation/evaluator.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@
2626
from .eval_rubrics import RubricScore
2727

2828

29+
def _validate_invocation_lengths(
30+
actual_invocations: list[Invocation],
31+
expected_invocations: Optional[list[Invocation]],
32+
) -> None:
33+
"""Rejects invocation lists that cannot be paired without truncation."""
34+
if expected_invocations is not None and len(actual_invocations) != len(
35+
expected_invocations
36+
):
37+
raise ValueError(
38+
"actual_invocations and expected_invocations must have the same"
39+
f" length; got {len(actual_invocations)} and"
40+
f" {len(expected_invocations)}."
41+
)
42+
43+
2944
class PerInvocationResult(BaseModel):
3045
"""Metric evaluation score per invocation."""
3146

src/google/adk/evaluation/final_response_match_v1.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from .eval_case import ConversationScenario
2424
from .eval_case import Invocation
2525
from .eval_metrics import EvalMetric
26+
from .evaluator import _validate_invocation_lengths
2627
from .evaluator import EvalStatus
2728
from .evaluator import EvaluationResult
2829
from .evaluator import Evaluator
@@ -47,12 +48,15 @@ def evaluate_invocations(
4748
) -> EvaluationResult:
4849
if expected_invocations is None:
4950
raise ValueError("expected_invocations is required for this metric.")
51+
_validate_invocation_lengths(actual_invocations, expected_invocations)
5052
del conversation_scenario # not used by this metric.
5153

5254
total_score = 0.0
5355
num_invocations = 0
5456
per_invocation_results = []
55-
for actual, expected in zip(actual_invocations, expected_invocations):
57+
for actual, expected in zip(
58+
actual_invocations, expected_invocations, strict=True
59+
):
5660
reference = _get_text_from_content(expected.final_response)
5761
response = _get_text_from_content(actual.final_response)
5862
rouge_1_scores = _calculate_rouge_1_scores(response, reference)

src/google/adk/evaluation/final_response_match_v2.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def _parse_critique(response: str) -> Label:
102102
)
103103
# Remove any trailing whitespace, commas, or end-brackets from the label.
104104
if label_match_is_response_valid:
105-
label = label_match_is_response_valid.group(1).strip(r"\s,\}")
105+
label = label_match_is_response_valid.group(1).strip().rstrip(",}")
106106
if label in [
107107
Label.INVALID.value,
108108
Label.ALMOST.value,
@@ -115,7 +115,7 @@ def _parse_critique(response: str) -> Label:
115115
else:
116116
label = Label.NOT_FOUND
117117
elif label_match_is_response_invalid:
118-
label = label_match_is_response_invalid.group(1).strip(r"\s,\}")
118+
label = label_match_is_response_invalid.group(1).strip().rstrip(",}")
119119
label = (
120120
Label.INVALID
121121
if label in [Label.TRUE.value, Label.INVALID.value]

src/google/adk/evaluation/hallucinations_v1.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
from .eval_case import InvocationEvents
4040
from .eval_metrics import EvalMetric
4141
from .eval_metrics import HallucinationsCriterion
42+
from .evaluator import _validate_invocation_lengths
4243
from .evaluator import EvalStatus
4344
from .evaluator import EvaluationResult
4445
from .evaluator import Evaluator
@@ -704,6 +705,7 @@ async def evaluate_invocations(
704705
conversation_scenario: Optional[ConversationScenario] = None,
705706
) -> EvaluationResult:
706707
del conversation_scenario # not used by this metric.
708+
_validate_invocation_lengths(actual_invocations, expected_invocations)
707709

708710
# expected_invocations are not required by the metric and if they are not
709711
# supplied, we provide a list of None to rest of the code.
@@ -714,7 +716,9 @@ async def evaluate_invocations(
714716
)
715717

716718
per_invocation_results = []
717-
for actual, expected in zip(actual_invocations, expected_invocations):
719+
for actual, expected in zip(
720+
actual_invocations, expected_invocations, strict=True
721+
):
718722
step_evaluations = self._get_steps_to_evaluate(actual)
719723

720724
if not step_evaluations:

src/google/adk/evaluation/llm_as_judge.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from .eval_metrics import BaseCriterion
3535
from .eval_metrics import EvalMetric
3636
from .eval_metrics import RubricScore
37+
from .evaluator import _validate_invocation_lengths
3738
from .evaluator import EvaluationResult
3839
from .evaluator import Evaluator
3940
from .evaluator import PerInvocationResult
@@ -123,6 +124,7 @@ async def evaluate_invocations(
123124
) -> EvaluationResult:
124125
if self._expected_invocations_required and expected_invocations is None:
125126
raise ValueError("expected_invocations is needed by this metric.")
127+
_validate_invocation_lengths(actual_invocations, expected_invocations)
126128
del conversation_scenario # not supported for per-invocation evaluation.
127129

128130
# If expected_invocation are not required by the metric and if they are not
@@ -134,7 +136,9 @@ async def evaluate_invocations(
134136
)
135137

136138
per_invocation_results = []
137-
for actual, expected in zip(actual_invocations, expected_invocations):
139+
for actual, expected in zip(
140+
actual_invocations, expected_invocations, strict=True
141+
):
138142
auto_rater_prompt = self.format_auto_rater_prompt(actual, expected)
139143
llm_request = LlmRequest(
140144
model=self._judge_model_options.judge_model,

src/google/adk/evaluation/rubric_based_evaluator.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,12 @@ def parse(self, auto_rater_response: str) -> list[RubricResponse]:
8080

8181
scores.append(score)
8282

83+
# A partial parse can silently omit a failed rubric and inflate the score.
84+
if not len(properties) == len(rationales) == len(scores):
85+
return []
86+
8387
rubric_responses = []
84-
for p, r, s in zip(properties, rationales, scores):
88+
for p, r, s in zip(properties, rationales, scores, strict=True):
8589
rubric_responses.append(
8690
RubricResponse(property_text=p.strip(), rationale=r.strip(), score=s)
8791
)

src/google/adk/evaluation/rubric_based_multi_turn_trajectory_evaluator.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from .eval_metrics import EvalMetric
2828
from .eval_metrics import EvalStatus
2929
from .eval_metrics import RubricsBasedCriterion
30+
from .evaluator import _validate_invocation_lengths
3031
from .evaluator import EvaluationResult
3132
from .evaluator import PerInvocationResult
3233
from .rubric_based_evaluator import RubricBasedEvaluator
@@ -278,6 +279,9 @@ async def evaluate_invocations(
278279
"Local evaluator start (invocations: %d)",
279280
len(actual_invocations),
280281
)
282+
_validate_invocation_lengths(actual_invocations, expected_invocations)
283+
if not actual_invocations:
284+
return EvaluationResult()
281285
self._assemble_dialogue_history(actual_invocations)
282286

283287
# If expected_invocations are not supplied, provide a list of None.
@@ -290,7 +294,7 @@ async def evaluate_invocations(
290294
# Mark the first N-1 turns as NOT_EVALUATED.
291295
per_invocation_results = []
292296
for actual, expected in zip(
293-
actual_invocations[:-1], expected_invocations[:-1]
297+
actual_invocations[:-1], expected_invocations[:-1], strict=True
294298
):
295299
per_invocation_results.append(
296300
PerInvocationResult(

src/google/adk/evaluation/trajectory_evaluator.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from .eval_case import Invocation
2828
from .eval_metrics import EvalMetric
2929
from .eval_metrics import ToolTrajectoryCriterion
30+
from .evaluator import _validate_invocation_lengths
3031
from .evaluator import EvalStatus
3132
from .evaluator import EvaluationResult
3233
from .evaluator import Evaluator
@@ -105,13 +106,16 @@ def evaluate_invocations(
105106
"""Returns EvaluationResult after performing evaluations using actual and expected invocations."""
106107
if expected_invocations is None:
107108
raise ValueError("expected_invocations is needed by this metric.")
109+
_validate_invocation_lengths(actual_invocations, expected_invocations)
108110
del conversation_scenario # not supported for per-invocation evaluation.
109111

110112
total_tool_use_accuracy = 0.0
111113
num_invocations = 0
112114
per_invocation_results = []
113115

114-
for actual, expected in zip(actual_invocations, expected_invocations):
116+
for actual, expected in zip(
117+
actual_invocations, expected_invocations, strict=True
118+
):
115119
tool_use_accuracy = self._calculate_tool_use_accuracy(actual, expected)
116120
per_invocation_results.append(
117121
PerInvocationResult(
@@ -259,7 +263,9 @@ def _are_tool_calls_exact_match(
259263
if len(actual_tool_calls) != len(expected_tool_calls):
260264
return False
261265

262-
for actual, expected in zip(actual_tool_calls, expected_tool_calls):
266+
for actual, expected in zip(
267+
actual_tool_calls, expected_tool_calls, strict=True
268+
):
263269
if actual.name != expected.name or actual.args != expected.args:
264270
return False
265271

src/google/adk/evaluation/vertex_ai_eval_facade.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from .eval_case import ConversationScenario
3131
from .eval_case import Invocation
3232
from .eval_case import InvocationEvent
33+
from .evaluator import _validate_invocation_lengths
3334
from .evaluator import EvalStatus
3435
from .evaluator import EvaluationResult
3536
from .evaluator import Evaluator
@@ -157,6 +158,7 @@ def evaluate_invocations(
157158
) -> EvaluationResult:
158159
if self._expected_invocations_required and expected_invocations is None:
159160
raise ValueError("expected_invocations is needed by this metric.")
161+
_validate_invocation_lengths(actual_invocations, expected_invocations)
160162
del conversation_scenario # not supported for per-invocation evaluation.
161163

162164
# If expected_invocation are not required by the metric and if they are not
@@ -170,7 +172,9 @@ def evaluate_invocations(
170172
total_score = 0.0
171173
num_invocations = 0
172174
per_invocation_results = []
173-
for actual, expected in zip(actual_invocations, expected_invocations):
175+
for actual, expected in zip(
176+
actual_invocations, expected_invocations, strict=True
177+
):
174178
prompt = self._get_text(actual.user_content)
175179
reference = self._get_text(expected.final_response) if expected else None
176180
response = self._get_text(actual.final_response)
@@ -224,6 +228,9 @@ def evaluate_invocations(
224228
conversation_scenario: Optional[ConversationScenario] = None,
225229
) -> EvaluationResult:
226230
del conversation_scenario
231+
_validate_invocation_lengths(actual_invocations, expected_invocations)
232+
if not actual_invocations:
233+
return EvaluationResult()
227234

228235
per_invocation_results = []
229236
# If expected_invocation are not required by the metric and if they are not
@@ -236,7 +243,7 @@ def evaluate_invocations(
236243

237244
# We mark all the n-1 turns as NOT-EVALUATED for these metrics.
238245
for actual, expected in zip(
239-
actual_invocations[:-1], expected_invocations[:-1]
246+
actual_invocations[:-1], expected_invocations[:-1], strict=True
240247
):
241248
per_invocation_results.append(
242249
PerInvocationResult(

tests/unittests/evaluation/test_final_response_match_v1.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,25 @@ def test_rouge_evaluator_multiple_invocations(
139139
expected_score, rel=1e-3
140140
)
141141
assert evaluation_result.overall_eval_status == expected_status
142+
143+
144+
@pytest.mark.parametrize(
145+
"actual_count, expected_count",
146+
[
147+
pytest.param(2, 1, id="extra-actual-turn"),
148+
pytest.param(1, 2, id="missing-actual-turn"),
149+
],
150+
)
151+
def test_rouge_evaluator_rejects_mismatched_invocation_lengths(
152+
actual_count: int, expected_count: int
153+
):
154+
actual, expected = _create_test_invocations("same", "same")
155+
rouge_evaluator = _create_test_rouge_evaluator(threshold=0.8)
156+
157+
with pytest.raises(
158+
ValueError,
159+
match=f"same length; got {actual_count} and {expected_count}",
160+
):
161+
rouge_evaluator.evaluate_invocations(
162+
[actual] * actual_count, [expected] * expected_count
163+
)

0 commit comments

Comments
 (0)