Skip to content

refactor(judge): unify LLM judge scoring on a single JSON output contract - #1601

Merged
Yunnglin merged 17 commits into
mainfrom
judge-json-contracts
Aug 20, 2026
Merged

Yunnglin merged 17 commits into
mainfrom
judge-json-contracts

Conversation

@Yunnglin

Copy link
Copy Markdown
Collaborator

Summary

Unifies LLM-judge scoring on a single JSON output contract, replacing hand-rolled, unanchored judge-response parsing (re.search, 'yes' in resp, etc.) that caused false-positive verdicts and silent metric corruption across benchmark adapters.

Adapters now declare judge cases and a Pydantic verdict schema; they never call the judge model and never parse a reply. JudgeExecutor owns retries, position swap, aggregation and fail-closed exclusion — a reply that fails the contract excludes the sample from the metric, never scores 0 or full credit.

Key changes

  • New subsystem evalscope/api/judge/ (contracts, executor, types); scoring routed through LLMJudgeMixin.score_with_judge_contracts.
  • Default binary-correctness path in LLMJudgeMixin (judge_prompt hook + _CorrectnessVerdict contract): single-verdict benchmarks migrate by setting only uses_judge_contracts = True. Migrated baby_vision, world_vqa, zerobench, minerva_math, imo_answerbench, math_verse onto it with zero duplicated hooks.
  • phyx (MC/OE) migrated to the contract; deleted the parse_judge_verdict / _VERDICT_RE parser.
  • Gate cleared: PENDING_MIGRATION is now empty. job_bench / wide_search remain PERMANENTLY_EXEMPT (judge used as a JSON data transformer producing scorecards/tables, not a single verdict).
  • Docs: added ## Key Features to researchrubrics and air_bench_chat, regenerated their _meta + en/zh pages; documented the judge-contract authoring flow in AGENTS.md.
  • Tests: structural gate, contract, executor, and per-adapter migration tests.

Validation

  • make lint passes; tests/api/judge + affected benchmark suites (235 tests) pass; test_ci_lite passes.
  • Real-API end-to-end (qwen-plus): simple_qa, researchrubrics (full agent + rubric judge + chunk/synthesis), minerva_math, imo_answerbench — judge naturally emits the JSON verdict; parse failures / transport errors exclude rather than mis-score.

Notes

  • 35 contract adapters total (was 28).
  • Draft: opening for review of the unified path before finalizing.

…ract

Replace hand-rolled, unanchored judge-response parsing across benchmark
adapters with one JSON OutputContract + JudgeExecutor path. Adapters declare
judge cases and a Pydantic verdict schema; they never call the judge or parse
a reply, so a malformed reply excludes the sample instead of silently scoring.

- Add evalscope/api/judge (contracts, executor, types) and route scoring
  through LLMJudgeMixin.score_with_judge_contracts.
- Add a default binary-correctness contract path in LLMJudgeMixin
  (judge_prompt hook) so single-verdict benchmarks migrate by only setting
  uses_judge_contracts=True; migrate baby_vision, world_vqa, zerobench,
  minerva_math, imo_answerbench, math_verse onto it.
- Migrate phyx (MC/OE) to the contract and delete parse_judge_verdict.
- Clear PENDING_MIGRATION; keep job_bench/wide_search exempt (judge used as a
  JSON data transformer, not a single verdict).
- Add Key Features to researchrubrics/air_bench_chat descriptions, regenerate
  their _meta + en/zh docs, and document the judge-contract flow in AGENTS.md.
- Add gate + contract + migrated-adapter tests.
Apply isort/yapf/flake8 (E302) to the two files run-pre-commit flagged; they
were untracked when make lint first ran, so CI caught the formatting.
@Yunnglin
Yunnglin marked this pull request as ready for review August 18, 2026 16:05
Copilot AI lite review requested due to automatic review settings August 18, 2026 16:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…i-judge

Completes the judge-contract migration by deleting the unanchored fallback rather
than gating it, and turns the executor's reserved multi-judge/repeat structure into
a working feature.

Built-in contracts (must land before the deletion):
- `score_type` now selects a built-in contract: `pattern` grades one of the
  `score_mapping` labels, `numeric` takes a bounded 0-1 rating. This keeps the
  documented `general_qa` + `score_type` workflow working, which still relied on the
  legacy path.
- `score_mapping` drives the verdict `Literal`, so a custom mapping is honoured in
  both the prompt and the score instead of being silently ignored.
- Judge prompt templates lost their format tails and are now the single source; the
  reply format comes only from `OutputContract.instruction()`, so the prompt can no
  longer contradict the parser.

Legacy path removal:
- `llm_match_score` always scores through the contract; `uses_judge_contracts` is
  gone from the mixin and 34 adapters.
- Deleted `LLMJudge.get_score`, `score_extractors.py` and the `BaseJudge.get_score`
  abstract hook. `get_score` returned 0.0 on `[ERROR]`, the exact "failure is a zero"
  behaviour this subsystem exists to prevent. `score_pattern` is accepted but warns.
- The structural gate now scans every file under `evalscope/benchmarks/` instead of
  only `*_adapter.py`, closing the bypass of moving a parser into `utils.py`.

Repeats and multi-judge:
- `judge_model_args` accepts one mapping or a list; new `judge_repeats`. Verdicts are
  averaged with equal weight, and only usable observations count, so a failed judge
  or repeat shrinks the average instead of dragging it to zero.
- `judge_repeats > 1` requires an explicit non-zero judge temperature, and several
  judges must agree on `score_type` / `score_mapping` / `prompt_template` /
  `system_prompt` and use distinct `model_id`s: the hooks build one request shape per
  sample, so a differently configured judge would be graded against the primary
  judge's contract.
- Judge fingerprint bumped to v2 (scoring semantics changed, so old review caches
  must not be reused).

Defects fixed:
- `CaseVerdict` now carries its case metadata, so researchrubrics reads `rubric_idx`
  from it instead of parsing `case_id` - the same fragile string-shaped scoring the
  contract was meant to remove.
- researchrubrics computed its chunking condition in two places; a drift would emit
  chunk cases with no synthesis to fold them. Now one helper, plus a chunk cache that
  reads its slot once (samples are scored on worker threads).
- `_merge_scores` overwrote rule metadata on the success path while merging it on the
  failure path.
- arena_hard counted excluded samples in `num`.
- A parse retry now tells the judge what was wrong; resending the identical prompt
  only relied on the provider not being bit-deterministic.
- Judge fingerprint is checked once per cache file, not once per row.
- `test_wide_search::test_rule_only_judge_is_rejected` called a method that does not
  exist and was already failing; `get_benchmark` is the raising construction path.

Dropped as unused: `JudgeCase.required` (no production caller) and a `model_dump`
exclusion for a field `JudgeAttempt` never had. `MAX_STAGES` 8 -> 4.

Breaking: `LLMJudge.get_score` and `score_extractors` are removed, `score_pattern` no
longer has an effect, judge review caches are invalidated, and a metric's `num` can be
below the sample count because failures are excluded rather than scored 0.
An unusable judge review carries an empty Score.value; the custom
aggregators that bypass the generic Mean must drop such a sample from a
metric instead of counting it as 0. This fixes:

- researchrubrics / mcp_atlas / drivel_writing / data_collection:
  skip samples missing a metric key instead of crashing or diluting the
  mean; a group with no usable sample yields no metric row.
- data_collection: return empty report levels when every sample was
  excluded, instead of letting an empty DataFrame groupby raise KeyError.
- cl_bench: accept the quoted '0'/'1' the contract instruction renders,
  so a compliant judge parses on the first attempt without a wasted retry.
- hipho: drop the dangling '' after two prompt templates.

Add tests/api/judge/test_aggregation_excludes_unusable.py covering the
four custom aggregators (including the all-excluded case) and a cl_bench
quoted-score test.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

BingH225 and others added 13 commits August 19, 2026 15:28
The dashboard delivered every judge diagnostic (judge_detail plus the
per-attempt judge_attempts) inside an untyped Score blob, so a reader
could only inspect it as raw JSON. Add a first-class JUDGE REVIEW panel
below EVALUATION RESULT and give the payload real types.

- schemas: type Score as a loose object with judge_detail / judge_attempts
  (judgeDetailSchema, judgeAttemptSchema, predictionScoreSchema) instead of
  z.record(unknown); export JudgeAttempt / JudgeDetail / PredictionScore.
- domain: selectJudgeReview folds attempt-oriented data into case-oriented
  rows (verdict, per-placement verdicts, contract-violation retry count,
  status); scoreWithoutJudgeAttempts drops the raw replies from Score Detail
  so a rubric sample's payload is not rendered twice.
- component: JudgeReviewPanel renders the overview bar (judge models,
  observations, failures, latency), one collapsible row per case (single
  case degrades to one line), the parsed verdict and raw reply on expand,
  a show-all toggle past 8 cases, and the adapter explanation.
- The row summary leads with the decisive field (verdict / overall_score /
  ...), shows both sides of a pairwise case, and keeps the key for a numeric
  verdict so a bare 0 never stands for a score.
- i18n: add en/zh strings for the panel.

data_collection: return columned empty frames and guard the category level
so an all-excluded judge run yields empty report levels instead of a
column-less DataFrame that raises KeyError in aggregate_scores and again in
gen_collection_report.

Verified against a live service on real e2e output (researchrubrics,
arena_hard, cl_bench, mia_bench, simple_qa); tsc / eslint / vitest (513) /
vite build all pass.
…rate summary

- Drop judge_revision, judge_cache_*, fingerprint, provenance from adapters, mixin, cache and scorer

- Remove REVIEW_CACHE_SCHEMA_VERSION and compute_judge_fingerprint

- Move judge summary aggregation to evalscope/api/judge/summary.py

- Add ConfigDict(extra='ignore') to ReviewResult for old cache compatibility

- Fix frontend pairwise case grouping by placement

- Update tests and docs accordingly
@Yunnglin
Yunnglin merged commit bbd8d03 into main Aug 20, 2026
4 checks passed
@Yunnglin
Yunnglin deleted the judge-json-contracts branch September 10, 2026 07:43
Yunnglin pushed a commit that referenced this pull request Sep 28, 2026
Since #1601, the AlpacaEval adapter builds each pairwise outcome with
the default weak strength. A weak win scores 0.75 and a weak loss
scores 0.25, so the reported win rate is 0.25 + 0.5 x the true rate.

The AlpacaEval judge picks one output and has no slight preference.
Mark every outcome as strong, so a win counts as 1 and a loss counts
as 0, as it did before #1601. Raise evaluation_version to v1.1
because the score changes.

Co-authored-by: Jiang Wu <245355730+jiangwu25@users.noreply.github.com>
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.

3 participants