Review what the metric said instead of expecting a verdict

[akwasigroch]

Purpose

A tuning case used to record the verdict the metric should return, and a run compared for equality. That model does not survive contact with this deployment: 45 numeric metrics, 2 categorical, 0 binary. Authoring the expected verdict for a numeric metric means answering "what number would you have given?", which nobody can answer honestly — while judging whether 0.2 is wrong is easy.

So the judgement moves to after the run. A reviewer looks at what the metric actually said and either accepts it or rejects it with a written comment, and those comments are the point of the whole feature: they are what someone reads when rewriting an evaluation prompt. Ticket 05 of the tuning-runs effort; the model is fixed by ADR-0005, which supersedes ADR-0002.

What Changed

• Reviews replace expected verdicts. POST /metrics/{id}/tuning/cases/{case_id}/review records one judgement; a rejection without a comment is refused. POST /metrics/{id}/tuning/reviews/accept-rest accepts every case still unreviewed, so forty cases are not forty decisions.
• Reviews accumulate on the case, in test.test_metadata beside the run's result — no new table, same reasoning as ADR-0004. Capped at ten per case, and eviction takes an accept, never a review carrying a comment. Re-judging a verdict that has not moved replaces that reviewer's last review rather than appending, so a corrected mis-click does not spend two slots.
• A review stands until the metric's decision actually changes. material_change.py buckets the judged verdict and the current one by the metric's threshold (or its passing_categories) and compares buckets, so 0.79 → 0.81 under a 0.5 threshold is noise and the same pair under 0.8 is a reversal. The bucket is derived on read and never stored, so moving a threshold re-evaluates existing reviews instead of freezing yesterday's arithmetic. Changing score_type invalidates every review for the metric and keeps the comments.
• Four outcomes, never collapsed: accepted, rejected, errored, unreviewed. Unreviewed is never counted as accepted — a set nobody looked at must not report itself as perfect — and an unreviewed case says whether nobody has judged it yet or a material change took its review away.
• Removed with the old model: services/metric_tuning/verdict.py and its per-score-type validation, the expected verdict, the case rationale, the stale and unlabelled markers, and every write to prompt.expected_response. expected_output is now reference_answer, still read under the old key so cases written earlier keep their text.
• Frontend: the grid reads left to right as the case, what the metric output, then the review. Thumb up accepts in one click, thumb down opens the required comment box, and the thumbs are the state as well as the control — the pressed one is filled and coloured, so no chip repeats it. An amber mark flags a review a material change invalidated. Columns nothing fills are left out. The case drawer loses the verdict control and the rationale and asks for the reference answer only when the metric needs one. BaseDataGrid gains an optional columnGroupingModel.

Additional Context

• Targets feat/metric-tuning-test-sets, not main — that branch is the integration branch for the whole metric tuning effort, and main sees exactly one merge when the feature is finished.
• Supersedes the expected-verdict work from Add metric tuning for custom metrics [feature branch] #2446 and builds on the tuning runs from Add tuning runs for custom metrics #2470. Unblocks ticket 03 (the agreement number), which is deliberately not in this PR: this one produces the per-case outcome the ratio will be computed from.
• Behind NEXT_PUBLIC_METRIC_TUNING, and the routes still refuse anything that is not a custom metric.
• Two things reviewers should know are deliberate. The cap is soft: at ten reviews with no accept to evict the list grows rather than dropping a comment, because a cap that silently destroyed the comments would destroy what the feature produces. And accept-the-rest accepts invalidated cases too — they are "still unreviewed", which is what the ticket asks for, but it does mean one click re-accepts a verdict that just moved.
• One fix that is not strictly in scope: get_tuning_cases ordered only by created_at, and cases created inside one transaction share it exactly, so list order was left to the planner. It now breaks ties on id.
• The interface departs from the spec in two places, on review: the band over the run columns is labelled Metric output even though Output also names the answer being judged one band to the left, and the review column carries no band and no chip. Both are recorded in the ticket.

Testing

181 backend tests pass, run three times over to shake out order dependence, plus 45 frontend tests for the tab and the client.

cd apps/backend
uv run pytest ../../tests/backend/routes/test_metric_tuning.py \
  ../../tests/backend/routes/test_metric_tuning_runs.py \
  ../../tests/backend/routes/test_metric_tuning_reviews.py \
  ../../tests/backend/schemas/test_metric_tuning_metadata.py \
  ../../tests/backend/services/metric_tuning -v

cd apps/frontend
npx jest 'src/app/\(protected\)/metrics' src/utils/api-client/__tests__/metric-tuning-client.test.ts

By hand, with NEXT_PUBLIC_METRIC_TUNING=true: add two cases to a custom metric, press Run metric, then reject one with a comment and accept the other from its row. Edit the evaluation prompt so one verdict crosses the metric's threshold and run again — that case comes back unreviewed with the amber mark and its old comment still in storage, while the case that only drifted keeps its judgement. Accept the rest then clears whatever is left.

The test that matters most is test_the_metric_sees_the_case_and_nothing_else: route the metric under test through the normal evaluation path and nothing raises, the scorecard just comes out flattering. That test is what fails loudly instead.

[peqy]

[Improvement] reviewer_id may be None, which can make distinct anonymous reviews overwrite each other.

[Improvement] Avoid mutable default list for MetricTuningCaseMetadata.reviews.

Found 2 issues (0 critical, 2 improvements).

[peqy]

[Improvement] reviewer_id can be None (see get_tenant_context), and then existing.reviewer_id != review.reviewer_id will treat all anonymous reviews as the same reviewer, causing later reviews to overwrite earlier ones.

Fix: ensure reviewer_id is always set (e.g., pass current_user.id from the router) or treat None as “always append” (never replace).

[peqy]

[Improvement] reviews: List[MetricTuningReview] = [] is a mutable default.

Fix: use Field(default_factory=list) to avoid any risk of shared state between instances.