Skip to content

fix(eval): default RubricContent.text_property to None so rubrics save without 422 - #7026

Open
ALDRIN121 wants to merge 1 commit into
google:mainfrom
ALDRIN121:fix-eval-rubric-text-property-optional
Open

ALDRIN121 wants to merge 1 commit into
google:mainfrom
ALDRIN121:fix-eval-rubric-text-property-optional

Conversation

@ALDRIN121

Copy link
Copy Markdown
Contributor

Summary

Fixes a Pydantic-v2 "required Optional" trap that makes saving an eval case fail with HTTP 422 when a rubric's text_property is omitted or empty.

RubricContent.text_property is typed Optional[str] but has no default. Under Pydantic v2, an Optional field without a default is still required — so when the Web UI's GET serializes an eval case with response_model_exclude_none=True (dropping text_property: None) and the browser re-submits it on the save PUT, the field is missing and validation fails with 422 Unprocessable Entity.

This is the exact same bug class as #6336 / the #6515 fix (fix: make InvocationEvent.content optional so the Web UI can save eval cases), which patched InvocationEvent.content but left RubricContent.text_property unaddressed. Related to #7019 (422 persists across 2.6.0–2.8.0 despite #6515).

Root cause

src/google/adk/evaluation/eval_rubrics.py:

class RubricContent(EvalBaseModel):
  text_property: Optional[str] = Field(
      description="..."          # no default -> Pydantic v2 treats as required
  )

Fix

Default text_property to None, matching the accepted InvocationEvent.content fix:

text_property: Optional[str] = Field(
    default=None,
    description="...",
)

Reproduction (before the fix)

rubric = Rubric(rubric_id="r1", rubric_content=RubricContent(text_property=None))
case = EvalCase(eval_id="c1", conversation=[], rubrics=[rubric])
wire = case.model_dump(by_alias=True, exclude_none=True)  # GET
EvalCase.model_validate(wire)                              # PUT -> ValidationError
# pydantic_core.ValidationError: rubricContent.textProperty — Field required

After the fix, the round-trip succeeds and text_property is None.

Testing plan

  • pytest tests/unittests/evaluation/test_eval_case.py24 passed (2 new tests: test_rubric_content_text_property_defaults_to_none, test_eval_case_with_rubric_missing_text_property_round_trips).
  • pytest tests/unittests/evaluation/test_rubric_based_tool_use_quality_v1.pypassed (no regression in rubric consumers).

Note: I ran the focused evaluation tests; the broader tests/unittests/evaluation/ suite requires optional GCS/Vertex deps not installed in the minimal venv.

Signed-off-by: Aldrin Joseph yoaldrinjoseph@gmail.com

copybara-service Bot pushed a commit that referenced this pull request Sep 16, 2026
Merge #7096

**Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.**

### Link to Issue or Description of Change

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

- Closes: #7019
- Related: #7026 (different 422: missing `RubricContent.text_property` default; does not cover this extra-fields case)

**2. Or, if no issue exists, describe the change:**

**Problem:**
Saving an eval case from the `adk web` editor returns HTTP 422 Unprocessable Entity. The editor serializes UI-only transcript fields (`invocationIndex`, `toolUseIndex`) onto each `InvocationEvent`. `EvalBaseModel` uses `extra="forbid"`, so FastAPI rejects the PUT body as `extra_forbidden`.

**Solution:**
Override `InvocationEvent` with `extra="ignore"` (same ConfigDict pattern as `EvalCase` / `SessionInput`). Unknown UI fields are dropped at parse time, the save succeeds, and those indices are not persisted in the eval-case JSON.

### Testing Plan

**Unit Tests:**

- [x] I have added or updated unit tests for my change.
- [x] All unit tests pass locally.

`PYTHONPATH=src pytest tests/unittests/evaluation/test_eval_case.py`

```
23 passed
```

Added `test_eval_case_put_accepts_web_ui_transcript_indices`, which validates the reporter payload from #7019 and asserts `invocationIndex` / `toolUseIndex` are not stored.

**Manual End-to-End (E2E) Tests:**

- Run `adk web` for an agent that uses tools
- Create a session, add it to an eval set, edit a message, click Save
- Expect HTTP 200 (not 422)
- Confirm the stored eval-case JSON does not contain `invocationIndex` or `toolUseIndex`

### Checklist

- [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] 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

`#7026` only defaults `RubricContent.text_property`. The reporter confirmed that does not fix this save. Maintainer diagnosis on #7019: `extra_forbidden` on `invocationIndex` and `toolUseIndex` inside `conversation[0].intermediateData.invocationEvents`.

Co-authored-by: Yi Liu <yiliuly@google.com>
COPYBARA_INTEGRATE_REVIEW=#7096 from Anusha0501:fix/eval-ignore-web-ui-invocation-indices 782eb0c
PiperOrigin-RevId: 982699482
…e without 422

RubricContent.text_property was typed Optional[str] but had no default.
Under Pydantic v2 an Optional field without a default is still required, so
saving an eval case whose rubric omits text_property failed model validation
and the PUT returned HTTP 422 — the same required-Optional trap that google#6515
fixed for InvocationEvent.content but left unaddressed here. Default it to
None so an omitted or empty text_property is accepted and round-trips
through the GET(exclude_none=True) -> PUT cycle.

Related google#7019
@ALDRIN121
ALDRIN121 force-pushed the fix-eval-rubric-text-property-optional branch from 9da22f0 to df74ba7 Compare September 17, 2026 18:03
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.

2 participants