Skip to content

fix(evaluation): resolve rubric verdicts per invocation, not per evaluator - #7302

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-7301-per-invocation-rubrics-clobbered
Open

chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-7301-per-invocation-rubrics-clobbered

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:

RubricBasedEvaluator kept the "effective rubric list" as instance state:
format_auto_rater_prompt calls create_effective_rubrics_list(actual_invocation.rubrics),
which sets self._effective_rubrics_list, and convert_auto_rater_response_to_score
later reads get_effective_rubrics_list() to map parsed verdicts back to
rubrics.

LlmAsJudge.evaluate_invocations formats the prompt for every invocation
first (the loop at the top of the method), then gathers all sample tasks. So
by the time any response is converted, _effective_rubrics_list is whatever
the last invocation's rubrics were. A verdict for a rubric that is on an
earlier invocation but not on the last one is silently discarded with the
warning Rubric ... not found in the rubrics provided to the metric.. With
the same rubric on every invocation the bug is invisible, which is why
criterion-level rubrics never hit it.

Solution:

Thread the invocation that a given auto-rater response was sampled for into
convert_auto_rater_response_to_score (new optional parameter, plumbed
through LlmAsJudge._evaluate_single_sample), and cache the effective
rubrics list per invocation (keyed by id(actual_invocation)) instead of in
a single shared slot. Each response is now resolved against the rubrics of
the invocation it was actually sampled for, regardless of formatting/gather
order. The existing no-argument behavior of create_effective_rubrics_list/
get_effective_rubrics_list is preserved as a fallback, so callers that
don't pass an invocation (existing unit tests, FinalResponseMatchV2Evaluator)
are unaffected.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added test_evaluate_invocations_scores_each_invocation_against_its_own_rubrics
in tests/unittests/evaluation/test_rubric_based_evaluator.py, which runs
evaluate_invocations end-to-end over two invocations that each carry a
different, non-overlapping rubric and asserts each invocation's result is
scored against its own rubric.

Confirmed the test fails without the fix (checked out the pre-fix source
for the touched files, keeping the new test):

$ git checkout HEAD -- src/google/adk/evaluation/*.py   # (pre-fix state)
$ pytest tests/unittests/evaluation/test_rubric_based_evaluator.py -q
...
E     TypeError: RubricBasedEvaluator.create_effective_rubrics_list() takes 2 positional arguments but 3 were given
1 failed, 68 passed in 0.81s

Also reproduced the exact symptom described in the issue directly against
the real RubricBasedFinalResponseQualityV1Evaluator (not just the test
double), with the pre-fix source:

invocation "t1" (rubric "a"): []          # dropped, with the warning:
  WARNING: Rubric Property A not found in the rubrics provided to the metric.
invocation "t2" (rubric "b"): [('b', 1.0)]  # only the last invocation scores

And with the fix applied, both invocations score correctly:

t1 [('a', 1.0)]
t2 [('b', 1.0)]

Full evaluation test suite after the fix:

$ pytest tests/unittests/evaluation -q
938 passed, 9 warnings in 23.56s

Full unit test suite after the fix:

$ pytest tests/unittests -q
16300 passed, 82 skipped, 26 xfailed, 2 xpassed, ... in 439.72s

pre-commit run (ruff, isort, pyink, addlicense, ADK compliance checks) on
all touched files: all hooks pass.

Manual End-to-End (E2E) Tests:

Not applicable — this is a pure logic fix in the local eval judge pipeline,
covered by the automated tests above and by re-running the exact repro steps
from the issue against the real evaluator (see above).

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This PR was prepared with AI assistance (Claude Code), with all changes
reviewed and verified by re-running the reproduction from the issue against
the real evaluator, before and after the fix.

🤖 Generated with Claude Code

…uator

RubricBasedEvaluator kept the effective rubric list as shared instance
state. LlmAsJudge.evaluate_invocations formats every invocation's
prompt before gathering any of the async judge calls, so by the time a
response is converted back into a score, the shared list held whatever
rubrics the last invocation in the batch had. A verdict for a rubric
that belonged to an earlier invocation, but not the last one, was
silently dropped with a "not found in the rubrics" warning.

Thread the actual invocation through convert_auto_rater_response_to_score
and cache the effective rubrics list per invocation so each response is
matched against the rubrics of the invocation it was sampled for.

Fixes google#7301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RubricBasedEvaluator drops per-invocation rubric verdicts when invocations carry different rubrics

2 participants