Skip to content

fix(optimization): treat explicit empty eval case ID lists as empty in LocalEvalSampler - #7170

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-7168-local-eval-sampler-empty-list
Open

chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-7168-local-eval-sampler-empty-list

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Fixes #7168

Problem

LocalEvalSamplerConfig documents train_eval_case_ids and
validation_eval_case_ids as optional lists where all cases are
selected when a field is not provided. However,
LocalEvalSampler.__init__ used truthiness checks (or / if config.x:),
so an explicitly passed [] was indistinguishable from None and
silently expanded to every eval case in the set — turning a
"run zero cases" configuration into a full evaluation run.

Fix

Changed both checks in LocalEvalSampler.__init__ to is not None,
so only an omitted/None field triggers the "use all eval cases"
fallback. An explicitly empty list now stays empty, consistent with
the documented behavior and with sample_and_score(batch=[]).

# before
self._train_eval_case_ids = (
    self._config.train_eval_case_ids
    or self._get_eval_case_ids(self._train_eval_set)
)
...
if self._config.validation_eval_case_ids:
    ...

# after
self._train_eval_case_ids = (
    self._config.train_eval_case_ids
    if self._config.train_eval_case_ids is not None
    else self._get_eval_case_ids(self._train_eval_set)
)
...
if self._config.validation_eval_case_ids is not None:
    ...

Testing plan

Added two parametrized cases to the existing
test_local_eval_service_interface_init test in
tests/unittests/optimization/local_eval_sampler_test.py, covering an
explicit empty train_eval_case_ids and an explicit empty
validation_eval_case_ids.

Confirmed the new cases fail without the fix (reverted only the
source file via git checkout HEAD~1 -- src/google/adk/optimization/local_eval_sampler.py):

FAILED ...test_local_eval_service_interface_init[config_kwargs5-expected_attrs5] - AssertionError: assert ['train_set_1', 'train_set_2'] == []
FAILED ...test_local_eval_service_interface_init[config_kwargs6-expected_attrs6] - AssertionError: assert ['train_set_1', 'train_set_2'] == []
2 failed, 5 passed, 7 deselected

With the fix restored, the full test file and the rest of the
optimization suite pass:

$ pytest tests/unittests/optimization -q
35 passed, 12 warnings in 13.16s

Also ran isort --check-only and pyink --check on both changed
files — no issues.

AI assistance disclosure

This change was authored with the assistance of Claude Code (Anthropic).

…n LocalEvalSampler

LocalEvalSampler.__init__ used truthiness checks on
train_eval_case_ids/validation_eval_case_ids, so an explicitly passed
[] was indistinguishable from None and silently expanded to every
eval case. Switch to `is not None` checks so only an omitted/None
field falls back to "all cases", matching the documented behavior.

Fixes google#7168
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.

[Bug]: LocalEvalSampler treats explicit empty eval case ID lists as all cases

2 participants