Repository navigation
Give rubric_based_* metrics a public rubric path (#132) - #229
Conversation
erauner12
left a comment
erauner12
left a comment
There was a problem hiding this comment.
I tested this draft on a few real multi-turn traces and left three notes on behavior that seems worth deciding before the design is finalized. The rubric input path itself works but the main open questions are how case-level rubrics should apply across invocations, and whether tool declarations should be handled here or in a separate follow-up.
My preference is final-invocation-only behavior or an explicit applies_to option, but I’m happy to follow the maintainers’ direction. The per-rubric result shape stays the same either way, so downstream consumers are not blocked on that decision.
krisztianfekete
left a comment
krisztianfekete
left a comment
There was a problem hiding this comment.
Thanks for this, and for testing it on real multi-turn traces, that was really useful!
The approach is what I had in mind around this use-case, so let's get it in. A few things before merge:
- Tool declarations: already fixed on main by #234. Every builtin metric now goes through
_enrich_app_details, so a rebase picks it up and no follow-up is needed. - CI: test_evaluate_sessions_posts_builtin_evaluator_overrides now sees 'rubrics': None in the payload. Either updating the expectation or not sending the unset field is fine.
- Rubric types: ADK drops case and invocation rubrics with no type, but this PR gives them the metric's type. Let's match ADK, and have your pre-check list any untyped rubrics that would be dropped.
typeonRubricDef: ADK never filters criterion rubrics by type, so this field has no effect. Let's drop it, which also keeps ADK enum values out of the config.applies_to: let's go with option 1 for now (ADK parity plus your doc note). I'm working on a turn-aware expectations format where "final turn only" will be first-class, and I'd love your input on it when it's up.
erauner12
left a comment
erauner12
left a comment
There was a problem hiding this comment.
Thanks for the feedback @krisztianfekete! I’ll rebase onto current main, address the config and CI issues, and retain ADK parity. I’ll add regression coverage for the public rubric path, including stable property IDs, per-invocation scores and explanations, and clear handling of unusable rubric inputs.
I’d also like to contribute synthetic clarification, completion, and correction examples to the turn-aware expectations work, including cases that intentionally end at a clarification request. Please tag me on that issue/PR when you have it ready :)
The rubric-based metrics have been reachable only through the internal
build_eval_metric(..., rubrics=...) call: the metric builder accepted
rubrics, the eval-set format documented them on cases and invocations,
and nothing on the config, CLI or API path passed any of them through,
so the metrics were listed as not working.
Rubrics can now be declared on a builtin evaluator in the eval config,
as {id, text} mappings or plain strings with positional ids, with
duplicate ids refused. At run time the runner matches the eval case,
not only its conversation, and the matched case's rubrics and each
expected invocation's rubrics are attached to the actual invocations
the way ADK's own eval service attaches them, with the metric's rubric
type given to rubrics that have none, since ADK applies invocation
rubrics by type. A rubric metric with no rubric from the config, the
case or an invocation reports an error before any judge call rather
than an empty evaluation. Its result carries per-rubric scores per
invocation and overall in details. The API lists the rubric metrics as
working.
a123284 to
ff6e970
Compare
krisztianfekete
left a comment
krisztianfekete
left a comment
There was a problem hiding this comment.
Thanks, one small thing before merge:
- could you please replace
METRICS_NEEDING_RUBRICSwith a singleRUBRIC_METRIC_TYPESdictin config.py and use it everywhere?
The two metric names are repeated in mltipleplaces right now. It also lets BuiltinMetricDef reject rubrics on other metrics, which currently accept them and quietly ignore them.
I'll add rubrics to the gRPC contract in #251 once this is in.
# Conflicts: # src/agentevals/api/routes.py
|
Merged #251 and pushed the conflict fix plus the |
2 participants
Rubric-based response quality can now be configured through the public evaluation path, with identifiable property results. The shared plumbing also supports the existing tool-use rubric metric. Closes #132.
Rebased onto current main, including the conversation model, case matching and tool-declaration enrichment from #234.
Behavior
{id, text}properties or strings with positional IDs. It has no rubrictypefield, and supplied properties survive judge/threshold overrides.This retains ADK scoring semantics. Whole-conversation rubric support and new turn-aware expectations are follow-up work. The docs explain why completion requirements belong on the intended invocation and why the last observed response may correctly be a clarification question.
Validation
At
a5e39b9: 1,674 model-free tests passed, 8 skipped and 18 e2e tests deselected; Ruff check and formatting passed. Regression tests cover config, matched-case and invocation inputs, both existing rubric metrics, overrides, typed/untyped/mixed properties, unusable inputs, and serialized property results. No live model calls were made.Non-trivial AI assistance was used for implementation and test drafting.