Skip to content

feat(llm_judge): multi-sample verdicts with median aggregation#53

Open
IgnazioDS wants to merge 1 commit into
UiPath:mainfrom
IgnazioDS:feat/llm-judge-multi-sample
Open

feat(llm_judge): multi-sample verdicts with median aggregation#53
IgnazioDS wants to merge 1 commit into
UiPath:mainfrom
IgnazioDS:feat/llm-judge-multi-sample

Conversation

@IgnazioDS

Copy link
Copy Markdown

Why

Judge verdicts are a per-criterion noise source that today can't be isolated or damped:

  • temperature already defaults to 0.0, but temperature-0 sampling doesn't guarantee repeatability — the same artifacts can draw a strict or a lenient reading of the rubric on different calls. The test suite says as much: the comment at tests/test_agent_judge_criterion.py:1023 notes a trailing "\n" made small models "flip between a strict and lenient reading of 'contains', flaking the score", and the live integration test asserts score >= 0.7 rather than a point value for exactly this reason.
  • A single flaky verdict has outsized blast radius: one "did not call submit_verdict" response zeroes the whole criterion, and one outlier reading moves the weighted score.
  • Experiment-level replicates re-run the whole agent, so they can't tell judge variance from agent variance — the two mix into the same spread. Per-verdict sampling grades the SAME artifacts N times, so the spread across samples is judge variance by construction, at the cost of a few extra judge calls instead of full agent re-runs.

What

New samples field on llm_judge (1–9, default 1):

  • Default 1 is byte-identical. The single-call path is untouched — samples: 1 never enters the aggregation code. No existing test was modified; the pre-existing test_llm_judge_criterion.py suite passes unchanged.
  • N > 1: the judge grades the same rendered prompt N times (prompt built once — no re-rendering between samples). The criterion scores the median of the sampled scores. The representative sample — score closest to the median, earliest on ties — supplies rationale, findings, and the persisted transcript, so the audit trail is always a real verdict, never a synthetic blend.
  • Per-sample scores in details (sample_scores: [0.800, 0.600]) so a reviewer can see the spread that produced the median.
  • Token usage sums across samples — including samples that returned a response but no verdict; the cost is real either way.
  • Partial-failure tolerance: with N ≥ 2, one sample that produces no verdict (no submit_verdict call, or a transport failure) degrades to the median of the valid samples, with a note in details (1/3 judge samples produced no verdict; median over 2 valid samples). This kills the "one flaky verdict zeroes the criterion" failure mode without touching single-sample semantics.
  • All-samples-failed keeps today's failure semantics exactly: JudgeInfrastructureError still escalates to FinalStatus.ERROR (judge infra failure is not an agent failure — same principle checker.py states), unexpected exceptions still reach @handle_criterion_errors, and all-parse-failures still score 0.0 with the diagnostic.

Scope is deliberately llm_judge-only: agent_judge spawns a full SDK sub-agent per verdict, so multi-sampling it has a very different cost profile — better considered separately if wanted.

Tests

18 new mocked tests (no live calls) pin the aggregation semantics: default single-invocation with no aggregation artifacts, field bounds, odd/even median, deterministic first-index tie-break, identical prompt across samples, partial-failure degradation (no-verdict / transport / infra, including the minimal N=2 case), all-fail behavior per failure class (infra escalation, @handle_criterion_errors, first-sample parse diagnostic), usage summing, representative findings + transcript, and reference scrubbing across every multi-sample surface.

ruff format --check, ruff check, pyright (0 errors), the CE lint suite, and the full test suite pass locally.

Docs

TASK_DEFINITION_GUIDE.md gains a samples row in the llm_judge field table and a short "Sampling the judge" paragraph covering when to reach for it and how it differs from replicates.

Add `samples` (1-9, default 1 = unchanged single-call behavior) to
llm_judge. With N > 1 the judge grades the same rendered prompt N times;
the criterion scores the median verdict, the sample closest to the median
(earliest on ties) supplies rationale/findings/transcript, per-sample
scores render in details, and token usage sums across samples. A sample
that produces no verdict degrades to the median of the valid samples with
a degraded note; when every sample fails, single-sample failure semantics
apply unchanged (JudgeInfrastructureError still escalates, parse failures
still score 0.0). agent_judge stays single-sample.

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: coder_eval — pr:53 (4 files) axis:1,2,3,4,5,6,7,8

Scope: pr:53 (4 files) axis:1,2,3,4,5,6,7,8 · branch feat/llm-judge-multi-sample (PR #53 head; local review branch ci/publish-github-releases) · 2de482f · 2026-07-26T04:54Z · workflow variant

Change class: complex — introduces a new multi-sample aggregation code path in a scoring-relevant criterion (median selection, per-sample degradation, and error-escalation semantics), so correctness requires reasoning about how a task's score is derived

The judge-sampling feature lands on a healthy codebase — clean security, type safety, and test posture (9.6/10 overall, zero critical/high-severity defects outside one observability gap) — but the real risks are concentrated in criteria/llm_judge.py: N samples run serially inside the shared task_timeout watchdog (so a large samples value can flip an otherwise-passing row to FinalStatus.TIMEOUT for identical agent output, and multiplies judge spend that max_usd cannot see), and the sample loop's bare except Exception silently swallows first-party bugs into a still-green median; fix those two plus the duplicated single-sample/multi-sample construction tails and this is ready to ship.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 8.9 / 10 0 0 2 1 _grade_with_sampling duplicates _check_impl's result-construction tail instead of treating N=1 as the general case (two parallel judge paths, CC 15)
2. Type Safety 9.8 / 10 0 0 0 2 _build_transcript takes 4 positional params, two of them adjacent same-typed str — a swap is invisible to pyright and breaks the module's keyword-only convention
3. Test Health 9.9 / 10 0 0 0 1 samples>1 tested only on the Direct/Anthropic arm with default toggles
4. Security 10 / 10 0 0 0 0
5. Architecture & Design 9.9 / 10 0 0 0 1 Judge sampling is asymmetric: samples exists only on LLMJudgeCriterion, with no recorded rationale and no doc/error pointer for agent_judge
6. Error Handling & Resilience 9 / 10 0 1 0 0 Sample-loop exception handling discards diagnostics: blanket except Exception + swallowed partial infra errors are never logged and never surfaced in error= (only prose in details)
7. API Surface & Maintainability 9.8 / 10 0 0 0 2 Field name samples collides with the existing dataset-row "sample" vocabulary in the same YAML/CLI surface
8. Evaluation Harness Quality 9.5 / 10 0 0 1 0 samples runs N judge calls serially inside the task_timeout watchdog — up to 9x judge wall-clock (and 9x judge spend max_usd cannot see), with the guide documenting neither

Overall Score: 9.6 / 10 · Weakest Axis: Code Quality & Style at 8.9 / 10
Totals: 🔴 0 · 🟠 1 · 🟡 3 · 🔵 7 across 8 axes.

Blockers

  1. [Axis 6] Sample-loop exception handling discards diagnostics: blanket except Exception + swallowed partial infra errors are never logged and never surfaced in error= (only prose in details) (src/coder_eval/criteria/llm_judge.py:271) — Lines 271-272 are an unqualified catch-all with no logging and no comment:

    except Exception as exc:
    unexpected_errors.append(exc)

At samples: 1 an unexpected exception from our own code (e.g. a TypeError/AttributeError in extract_verdict_from_anthropic_response, a pydantic ValidationError building JudgeVerdict, or a bug in token_usage_from_anthropic_dict) is loud: handle_criterion_errors (criteria/base.py:79-96) does logger.error(..., exc_info=True) and returns error=exc_info. At samples: 3 with one such failure and two good samples, the exception is appended to a list that is then read only at lines 281-282 — never logged, never in error (stays None), and its type/message appears nowhere in details; the only artifact is the generic count note at line 304. tests/test_llm_judge_criterion.py::test_judge_multi_sample_transport_exception_degrades locks that in (RuntimeError("gateway down")result.error is None). So a systematic first-party regression that trips on, say, 1-in-3 responses ships as a green criterion with a slightly different median. Two secondary losses on the same handler: (a) precedence at lines 279-282 raises infra_errors[0] before unexpected_errors[0], so a genuine code bug is masked whenever any sample also hit a transient infra failure; (b) only element [0] is re-raised, so the remaining N−1 exception messages are discarded. Fix: log each swallowed exception at WARNING with exc_info=True, record type(exc).__name__: exc in the degraded note (or in a non-fatal error string) so the report names the failure, and narrow the catch to the exception classes the transports can actually surface rather than bare Exception. Candidate lint rule: forbid except Exception bodies in src/coder_eval/ that neither log nor re-raise (would also cover future accumulate-and-continue loops).

Non-blocking, but please consider before merge

  1. [Axis 1] _grade_with_sampling duplicates _check_impl's result-construction tail instead of treating N=1 as the general case (two parallel judge paths, CC 15) (src/coder_eval/criteria/llm_judge.py:229) — Both JudgeCriterionResult(...) construction sites are near-verbatim copies of the single-sample path. Parse-failure: lines 145-154 (score=0.0, details=scrubbed[:500], error=scrub_reference(parse_error, scrub_key), transcript=_build_transcript(criterion, raw_verdict_text, user_msg, scrub_key), token_usage=judge_usage) vs lines 286-294 (identical modulo the first. prefix and token_usage=token_usage). Success: lines 158-166 vs lines 309-317 — identical except score=verdict.score vs score=median_score and the rep-verdict source. The assert ... # parser contract: verdict is set iff parse_error is None comment is copied verbatim too (line 155 → line 284). Prefer collapsing to ONE path: run the loop range(criterion.samples) unconditionally (N=1 is a one-element list whose median is its own score, and _sum_usage of one usage is that usage), and gate only the two N>1-specific renderings — the sample_scores: line at 307-308 and the degraded note at 303-304 — on criterion.samples > 1. That deletes the if criterion.samples > 1: dispatch at 122-129, removes both duplicated construction blocks, and drops _grade_with_sampling well below its current radon C(15). While there, infra_errors: list[JudgeInfrastructureError] = [] / unexpected_errors: list[Exception] = [] (lines 255-256) accumulate every sample's exception but are only ever read at index 0 (raise infra_errors[0] line 280, raise unexpected_errors[0] line 282) — two accumulator lists for a value that is 'the first error seen', which is machinery the goal does not need and part of what pushes the branch count to 15. n/a
  2. [Axis 1] _SampleOutcome duplicates _invoke_tool_channel's 4-tuple return shape and is populated by an order-dependent positional splat (src/coder_eval/criteria/llm_judge.py:220) — The 4-field shape is now declared twice: as a bare tuple annotation at line 175 (-> tuple[JudgeVerdict | None, str | None, str, TokenUsage | None]) and again as the NamedTuple at lines 220-226, whose own docstring admits the coupling — """One judge invocation's outcome, in ``_invoke_tool_channel`` return order.""". It is then filled by splat at the one call site (lines 259-268: _SampleOutcome(*_invoke_tool_channel(...))), so field identity is carried purely by position. Fix in two lines: annotate _invoke_tool_channel(...) -> _SampleOutcome (move the class above it) and have its three return statements (lines 213, 216, 217) construct _SampleOutcome(...) by keyword. A NamedTuple unpacks positionally, so the existing single-sample destructuring at line 133 (verdict, parse_error, raw_verdict_text, response_usage = _invoke_tool_channel(...)) keeps working unchanged. That leaves one declaration of the shape, deletes the splat and the 'return order' comment, and makes the abstraction load-bearing instead of a second copy of a type that already existed. n/a
  3. [Axis 8] samples runs N judge calls serially inside the task_timeout watchdog — up to 9x judge wall-clock (and 9x judge spend max_usd cannot see), with the guide documenting neither (src/coder_eval/criteria/llm_judge.py:257) — The samples run strictly serially — for _ in range(criterion.samples): (line 257) with a blocking call per iteration — and each call carries a 120 s transport timeout (timeout_seconds: float = 120.0 at src/coder_eval/evaluation/judge_anthropic.py:29 and src/coder_eval/evaluation/judge_bedrock.py:51, the latter plus its retry loop). check_all runs inside the task-timeout watchdog (src/coder_eval/orchestrator.py:468-475 wraps self._evaluation_loop(), which calls check_all at orchestrator.py:1408-1414), and the shipped default is task_timeout: 600 (experiments/default.yaml:29). So samples: 9 admits up to ~18 minutes of judge time against a 600 s task budget shared with the agent — enough to flip a row to FinalStatus.TIMEOUT for identical agent output, which is a status change from a knob the guide presents purely as a scoring-quality dial.

Cost is multiplied with no gate at all: judge usage is deliberately excluded from the budget aggregate — src/coder_eval/orchestrator.py:848-852 states judge usage "is intentionally NOT included in this aggregate — it represents the main agent's bill" — so run_limits.max_usd cannot see it, and samples: 9 on a dataset-fanned suite silently multiplies ungated judge spend up to 9× per row.

Fix: state the trade in docs/TASK_DEFINITION_GUIDE.md:840 (which currently sells samples: 3 with no mention of latency, task_timeout, or cost) — concretely, that N samples multiply judge wall-clock inside task_timeout and multiply judge spend, which max_usd does not gate. Cheapest real mitigation: run the samples concurrently (they are independent and check_all is already off the event loop via asyncio.to_thread), which removes the latency multiplier entirely and leaves only the cost multiplier to document. n/a

Nits

  1. [Axis 1] 🔵 Median-aggregation semantics restated in four places, against the codebase's 'field descriptions defined once in the Pydantic model' rule (src/coder_eval/models/criteria.py:661) — The same behavior is written out four times: the class-docstring paragraph at criteria.py:661-663 ('Set samples > 1 to invoke the judge several times ... (see the field description for the aggregation semantics)' — which explicitly defers to the SSOT it is duplicating), the 12-line field description at criteria.py:771-782, the 17-line _grade_with_sampling docstring at llm_judge.py:237-253, and the guide (docs/TASK_DEFINITION_GUIDE.md table row + the 'Sampling the judge' paragraph). CLAUDE.md's Design Principles state 'Field descriptions, validation rules, and documentation defined once in Pydantic models'; a later change (e.g. median → trimmed mean, or a tie-break tweak) now needs four synchronized edits and will silently drift. Drop the class-docstring paragraph (it adds nothing over the field description it points at) and trim the function docstring to the parts that are implementation-only (the exception-triage order), leaving user-facing semantics to the field description and the guide. n/a
  2. [Axis 2] _build_transcript takes 4 positional params, two of them adjacent same-typed str — a swap is invisible to pyright and breaks the module's keyword-only convention (src/coder_eval/criteria/llm_judge.py:331) — The refactor turned the zero-arg closure def _maybe_transcript() -> JudgeTranscript | None (origin/main llm_judge.py:130, which captured raw_verdict_text/user_msg/scrub_key so mis-ordering was impossible) into a module-level positional helper: def _build_transcript(criterion, raw_verdict_text, user_msg, scrub_key) (line 331), now called from 4 sites — _build_transcript(criterion, raw_verdict_text, user_msg, scrub_key) (lines 152 and 164), _build_transcript(criterion, first.raw_verdict_text, user_msg, scrub_key) (line 292), _build_transcript(criterion, rep_raw, user_msg, scrub_key) (line 315). Params 2 and 3 are both plain str, so swapping them type-checks cleanly (pyright reports 0 diagnostics on this file) and would silently persist the prompt as JudgeTranscript.raw_verdict and the verdict as judge_prompt. Both other helpers in this module are keyword-only — def _invoke_tool_channel(*, criterion, route, system_msg, user_msg) (line 169) and def _grade_with_sampling(*, criterion, route, user_msg, judge_ctx, scrub_key) (line 229). Add *, to _build_transcript and pass by keyword at all 4 sites.
  3. [Axis 2] Redundant float() cast around median(scores)statistics.median over list[float] is already float (src/coder_eval/criteria/llm_judge.py:297) — Line 297 is median_score = float(median(scores)), where scores = [verdict.score for verdict, _ in valid] (line 296) and JudgeVerdict.score is declared score: float (src/coder_eval/models/judge.py:32). I verified with pyright reveal_type on an isolated repro of this exact pattern: Type of "scores" is "list[float]" and Type of "median(scores)" is "float", so the cast is a no-op at both the type and runtime level (the even-N branch returns (a + b) / 2, also a float). Drop the wrapper (median_score = median(scores)) so the line doesn't imply the return type is uncertain — or keep it with a one-line comment if it is deliberately guarding a future non-float numeric element.
  4. [Axis 3] samples>1 tested only on the Direct/Anthropic arm with default toggles (tests/test_llm_judge_criterion.py:1131) — Every test in the new # --- multi-sample judging (samples > 1) --- block (line 1084 onward) patches coder_eval.criteria.llm_judge.invoke_anthropic_judge with route=DirectRoute(), e.g. line 1135: with patch("coder_eval.criteria.llm_judge.invoke_anthropic_judge", side_effect=responses) as m_anthropic:. Three combinations therefore go unexercised at samples>1: (a) the Bedrock arm of _invoke_tool_channel (src/coder_eval/criteria/llm_judge.py:190) — the route the nightly uses via --backend bedrock, and the one whose invoker raises JudgeInfrastructureError for every failure mode (src/coder_eval/evaluation/judge_bedrock.py:104/110/112/114), which is the escalate-vs-degrade branch in _grade_with_sampling; (b) capture_transcript=False with samples>1 — the not criterion.capture_transcript early return in the newly extracted _build_transcript (llm_judge.py:331) is only reached from the single-sample test at line 716; (c) usage-summing fidelity — test_judge_multi_sample_token_usage_summed (line 1288) uses usage = {"input_tokens": 100, "output_tokens": 10}, omitting cache_creation_input_tokens / cache_read_input_tokens, which are exactly the buckets N repeated identical prompts would populate (calls 2..N read the cache) and which drive cost via TokenUsage.__add__; and no multi-sample test asserts result.token_usage is None when no sample reports usage (the if not present: return None branch of _sum_usage), the None-vs-zero distinction the single-sample test at line 1024 protects. I confirmed the Bedrock arm is functionally fine at samples>1 with a probe test (BEDROCK CALLS 3 SCORE 0.6 ERR None), so this is coverage debt rather than a latent bug — add one Bedrock multi-sample test (patching invoke_bedrock_judge with BedrockRoute(bearer_token="t", region="us-east-1"), mirroring line 927), and extend the usage test with cache buckets plus a token_usage is None case.
  5. [Axis 5] Judge sampling is asymmetric: samples exists only on LLMJudgeCriterion, with no recorded rationale and no doc/error pointer for agent_judge (src/coder_eval/models/criteria.py:767) — samples: int = Field(default=1, ge=1, le=9, ...) is added at models/criteria.py:767 to LLMJudgeCriterion only. AgentJudgeCriterion (models/criteria.py:816) is the other judge criterion, deliberately mirrors llm_judge's authoring surface (its own comment at :851 reads # Prompt & context — mirrors LLMJudgeCriterion for author consistency), shares the same JudgeVerdict / JudgeCriterionResult / format_details machinery, and has the same judge-variance problem — yet agent_judge: {samples: 3} now hard-fails on model_config = ConfigDict(extra="forbid") with a bare unknown-key error. Per-criterion placement is the right call (sampling is meaningless for deterministic file checks, so BaseSuccessCriterion would be wrong, and N× agent-judge runs are genuinely expensive), so the gap here is only that the asymmetry is unrecorded: add one line to the AgentJudgeCriterion docstring and/or the docs/TASK_DEFINITION_GUIDE.md agent_judge section saying sampling is llm_judge-only and why (cost), so the next author does not read it as an oversight — or hoist samples into a small shared judge mixin if agent_judge sampling is intended later.
  6. [Axis 7] Field name samples collides with the existing dataset-row "sample" vocabulary in the same YAML/CLI surface (src/coder_eval/models/criteria.py:767) — The new knob is samples: int = Field(default=1, ge=1, le=9, ...) (line 767), but "sample" is already taken in this config surface for something unrelated — dataset row selection: Dataset.sample_per_stratum (src/coder_eval/models/tasks.py:219), Dataset.stratify_field (:230), Dataset.sample_seed (:234), and the CLI flags --sample / --sample-per-stratum (src/coder_eval/cli/run_command.py:239,248). A single task YAML can legitimately carry dataset: {sample_per_stratum: 5} alongside success_criteria: [{type: llm_judge, samples: 3}], where the two "sample" keys mean rows and judge invocations respectively. The docs paragraph disambiguates against replicates (docs/TASK_DEFINITION_GUIDE.md:840) but not against sample_*. Since the project is greenfield ("No worries about backward compatibility" per CLAUDE.md), rename to judge_samples (or vote_samples) so the key is self-disambiguating at the point of use, or at minimum extend the field description to contrast it with dataset.sample_per_stratum the way the guide already contrasts it with replicates. n/a
  7. [Axis 7] le=9 upper bound on samples has no stated rationale in the field description or the guide (src/coder_eval/models/criteria.py:770) — Line 770 is a bare le=9, and neither the 12-line field description (lines 771-782) nor the guide row (docs/TASK_DEFINITION_GUIDE.md:837, which only restates the range as "(1–9)") says why 9. The error itself is fine — verified against the PR worktree: LLMJudgeCriterion(description='x', prompt='p', samples=10) raises 1 validation error for LLMJudgeCriterion / samples / Input should be less than or equal to 9 [type=less_than_equal, input_value=10], which names the field and the bound. The gap is only the missing "why": add one clause to the description (e.g. "capped at 9 — the marginal variance reduction past ~5 samples does not justify the linear API cost, and judge spend is outside run_limits budget caps"), so a future reader knows whether the cap is a cost guard that can be raised or a semantic limit. n/a

What's Missing

Downstream consumers:

  • 🟡 Downstream consumers 🟠 — The per-sample scores land only in the free-text details string (llm_judge.py:307-308, sample_scores: [0.800, 0.600]); JudgeCriterionResult (models/results.py:167) gained no typed field (e.g. sample_scores: list[float]), so no consumer can read judge spread — not reports_html._render_judge_card, not reports_stats, not reports_junit, not the cross-repo evalboard, and not the criterion's own aggregate(). The variance number the feature exists to measure is unreadable by anything except a human eyeballing a prose blob, while every other structured judge signal on that model (findings, token_usage, transcript) is a typed field. (trigger: src/coder_eval/criteria/llm_judge.py)
  • 🔵 Downstream consumers 🟡 — Suite-aggregate semantics changed meaning with no doc or guard: BaseCriterion.aggregate() still emits count/mean/median/std/min/max over per-row scores, but those scores are now medians-of-N, so a suite_thresholds: {std: …} gate and the cross-variant tables in reports_experiment.py measure a different quantity in a samples: 3 run than in a samples: 1 run — the two are no longer comparable, and nothing records that. No intra-row sample-spread metric was added to the aggregate either. (trigger: src/coder_eval/criteria/llm_judge.py)

Daily/nightly:

  • 🟡 Daily/nightly pipeline impact 🟠 — The PR never states the nightly blast radius. samples defaults to 1 so the nightly is byte-for-behavior unchanged today, but the nightly is exactly the --backend bedrock judge route that is untested at samples>1 and whose invoker wraps its own retry loop inside every sample (so the serial latency multiplier is worst on the production route), and no tasks/*.yaml adopts the field (3 task files mention llm_judge, none set samples:), so the feature ships unexercised in production. The missing artifact is a one-line "nightly unchanged until a suite opts in; opting in multiplies judge wall-clock inside task_timeout and multiplies judge spend" statement. (trigger: src/coder_eval/models/criteria.py) (restates: Axis 8: samples runs N judge calls serially inside the task_timeout watchdog)
  • 🟡 Daily/nightly pipeline impact 🟠 — Cross-repo contract not updated: docs/REPORT_SCHEMA.md:120-123 documents the judge result variant as adding findings, token_usage ("kept distinct from the agent total") and transcript_path, but token_usage now aggregates N calls and details grows an extra sample_scores: line in task.json. The external coder-eval-uipath / eval-runner + evalboard consume that record shape, so a field's meaning changed under them with no schema-doc edit — REPORT_SCHEMA.md is not in the diff at all. (trigger: src/coder_eval/criteria/llm_judge.py)
  • 🔵 Daily/nightly pipeline impact 🟡 — Request-rate amplification on the shared Bedrock judge account is unstated: run_batch already runs rows in parallel, so samples: N multiplies concurrent judge calls N-fold, and a throttled sample is swallowed into infra_errors / unexpected_errors with nothing logged — the nightly would quietly start scoring medians over fewer samples, with only a prose count note in details to show it. (trigger: src/coder_eval/criteria/llm_judge.py) (restates: Axis 6: Sample-loop exception handling discards diagnostics)

Parallel paths:

  • 🟡 Parallel paths 🟠 — The sibling model doc is now false and only 1 of N verdicts survives: models/results.py:131-133 still asserts "llm_judge is one-shot so its transcript only carries raw_verdict", yet _build_transcript(criterion, rep_raw, …) (llm_judge.py:315) persists only the representative sample's raw verdict to judge-<idx>.yaml — the N−1 other verdicts, including the outlier readings the median exists to damp, are discarded. Nothing on JudgeTranscript records that it is one draw of N, so post-hoc "why did the judge disagree with itself?" auditing (the transcript's stated purpose) is impossible for exactly the runs that need it. (trigger: src/coder_eval/criteria/llm_judge.py)
  • 🔵 Parallel paths 🟡 — Ownership of the judge details format split in two: evaluation/judge_context.py::format_details is the shared single formatter for both judges (and the shape reports_html._extract_rationale parses by line prefix), but the new sample_scores: line is appended outside it at llm_judge.py:307-308. The shared formatter gained no sample_scores parameter, so any future sampler (agent_judge) will re-invent the line ad hoc and consumers parsing details line-by-line now have an undeclared line to handle. (trigger: src/coder_eval/criteria/llm_judge.py)
  • 🔵 Parallel paths 🟡 — samples is unreachable from every override layer: -D/--set accepts only the agent / run_limits / sandbox roots (config_merge.ALLOWED_OVERRIDE_ROOTS) and ExperimentVariant (models/experiment.py:25-66) has no success_criteria override, so the calibration A/B this field exists for — the same suite at samples: 1 vs samples: 3 — cannot be expressed, and adoption means hand-editing every task YAML. A pre-existing platform limit, but it collides head-on with the field's stated purpose and the PR doesn't acknowledge it. (trigger: src/coder_eval/models/criteria.py)
  • 🔵 Parallel paths 🟡 — AgentJudgeCriterion (models/criteria.py:816), which deliberately mirrors llm_judge's authoring surface and shares the same JudgeVerdict / JudgeCriterionResult / format_details machinery, did not get samples and carries no note explaining why, so agent_judge: {samples: 3} fails with a bare extra_forbidden error. (trigger: src/coder_eval/models/criteria.py) (restates: Axis 5: Judge sampling is asymmetric — samples exists only on LLMJudgeCriterion)

Tests:

  • 🟡 Tests 🟠 — All 16 new multi-sample tests patch invoke_anthropic_judge on a DirectRoute with default toggles, leaving unexercised at samples>1: the Bedrock arm (llm_judge.py:187-198 — the nightly's route, and the only invoker that raises JudgeInfrastructureError for every failure mode, i.e. the escalate-vs-degrade branch), capture_transcript=False, the cache-token buckets that N repeated identical prompts actually populate, and the _sum_usagetoken_usage is None branch. (trigger: tests/test_llm_judge_criterion.py) (restates: Axis 3: samples>1 tested only on the Direct/Anthropic arm with default toggles)
  • 🔵 Tests 🟡 — The all-samples-parse-fail branch (llm_judge.py:286-294) is asserted only on score/error (test_judge_multi_sample_all_no_verdict_keeps_failure_path, …_all_fail_reports_first_sample_diagnostic); its two other outputs — the summed token_usage and the transcript built from the first sample — have no assertion, so a regression that drops judge cost or the audit transcript on the total-failure path lands green. (trigger: tests/test_llm_judge_criterion.py)
  • 🔵 Tests 🟡 — No test carries a multi-sample result through a report renderer: nothing pins that the appended sample_scores: line leaves reports_html._extract_rationale (which matches on the rationale: line prefix) and the reports_junit failure-body truncation intact, even though this PR is the first change to alter the judge details line set since those parsers were written. (trigger: tests/test_llm_judge_criterion.py)
  • 🔵 Tests 🔵 — Every new test runs at the default temperature: 0.0 while mocking divergent per-sample scores — a combination the real transport is unlikely to produce. Nothing (validator, warning, field-description clause, or doc line) records that samples > 1 buys real variance reduction mainly when the judge is sampled at a non-zero temperature, so an author can pay up to 9x judge cost for nine near-identical draws with no signal from the harness. (trigger: tests/test_llm_judge_criterion.py)

Display & mapping dicts:

  • 🔵 Display / mapping 🟡 — No new enum / Literal / status value was added, so the reports*.py icon and label dicts need no new entries. What is missing is any rendering that says "this score is a median over N samples": reports_html._render_judge_card shows one score pill plus the representative sample's rationale, so an HTML reader cannot distinguish a single-call verdict from a 9-sample median, and the markdown failure-reason line (reports.py:823) shows the median with no sample context — the only trace anywhere is raw details text inside the criteria-table disclosure. (trigger: src/coder_eval/criteria/llm_judge.py)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE026 — broad except must not store the exception object as its only handling. New rule /Users/religa/src/coder_eval/tests/lint/rules/ce026_no_exception_swallowed_into_collection.py (BaseRule, visit_ExceptHandler), wired into ALL_RULES in /Users/religa/src/coder_eval/tests/lint/runner.py (CE026 is the only free number below CE027; also add "CE026" to the [tool.ruff.lint] external list in pyproject.toml, which currently stops at CE018, so a # noqa: CE026 isn't reported as unused). Pattern forbidden: a broad handler (except Exception as e / bare / tuple containing Exception) in which every use of the bound name is an argument to a collection store (.append / .add / .extend / .setdefault / .update) and the body contains no raise, no logger.* call, and no interpolation of the name into an f-string / str() / repr(). This is exactly the gap that let the defect through the existing CE005: CE005 treats any reference to the bound name as "handled", so except Exception as exc: unexpected_errors.append(exc) passes — I confirmed check_file() returns [] on the PR file. I prototyped the refined predicate over src/ and tests/: exactly one violation repo-wide, src/coder_eval/criteria/llm_judge.py:271. Deliberately narrow: the four superficially similar sites stay clean without noqa — orchestration/batch.py:157 (passes exc to _create_error_task_result, not a collection store), cli/report_command.py:119 and criteria/json_check.py:177 (interpolate {e} into user-visible text), orchestration/experiment.py:706 (narrow exception tuple). Prevents: A6-high (grouped A7-medium / A8-high): except Exception as exc: unexpected_errors.append(exc) at llm_judge.py:271 swallows a first-party TypeError/ValidationError with no log record and no exception identity anywhere on the result. Also covers future accumulate-and-continue loops, which is the shape the finding itself asked for.
  • [ce-lint] CE032 — no *-splat construction of a NamedTuple / dataclass / model from a call. Forbid Foo(*bar()) where Foo is a Name whose identifier (leading underscores stripped) is CapWords, i.e. a type constructor. Field identity then travels by position between two independently declared signatures, which pyright cannot guard once two fields share a type — precisely the _SampleOutcome(*_invoke_tool_channel(...)) case where fields 2 and 3 are str | None / str. Fix direction the rule pushes authors toward: annotate the producer -> _SampleOutcome and return _SampleOutcome(...) by keyword (a NamedTuple still unpacks positionally, so the existing destructuring at llm_judge.py:133 keeps working). Measured over all of src/coder_eval: exactly one hit, src/coder_eval/criteria/llm_judge.py:260 — zero pre-existing violations to baseline. Same file/runner wiring as CE026. Prevents: A1/A2-medium: _SampleOutcome (llm_judge.py:220) redeclares _invoke_tool_channel's 4-field return shape (declared a third time in its docstring at :178) and is populated by an order-dependent positional splat.
  • [ce-lint] CE033 — do not re-raise only the first of an accumulated exception list. Forbid raise <name>[0] where <name> is a local list the enclosing function appends exception objects to; require raise ExceptionGroup(...) or logging the remainder before re-raising the representative. Measured over src/: exactly two hits, both introduced by this PR — src/coder_eval/criteria/llm_judge.py:280 (raise infra_errors[0]) and :282 (raise unexpected_errors[0]) — so no baseline. Detectability is high (ast.Raise whose exc is a Subscript with Constant(0)), and the accumulator-provenance check keeps it from firing on a legitimate raise errors[0] over a single-element container. Prevents: The two secondary losses inside A6-high: infra errors are raised in precedence over unexpected_errors, so a genuine code bug is masked whenever any sample also hit a transient transport failure, and the remaining N−1 exception messages are discarded outright.
  • [ce-lint] CE035 — adjacent same-annotated parameters must be keyword-only (scoped to the criterion/judge surface). For functions in src/coder_eval/criteria/** and src/coder_eval/evaluation/** (skip dunders) with ≥3 positional parameters where two adjacent ones carry identical annotations, require *,. _build_transcript(criterion, raw_verdict_text, user_msg, scrub_key) (llm_judge.py:331) has two adjacent plain-str params, is called from 4 sites, and a swap type-checks cleanly (pyright reports 0 diagnostics on the file) while silently persisting the prompt as JudgeTranscript.raw_verdict — pyright cannot reach this, only an arity/keyword rule can. Scope matters: repo-wide the pattern has 35 pre-existing sites (unviable as a hard gate), but inside these two packages it is 7 — criteria/run_command.py:41,74,108,203, criteria/llm_judge.py:331, criteria/command_executed.py:70, evaluation/judge_context.py:363 — all module-private or single-consumer helpers, so the one-time *, conversion is mechanical, and the convention already holds for _invoke_tool_channel and _grade_with_sampling in the same module. Prevents: A2-low: _build_transcript broke the module's keyword-only convention with two adjacent str params whose swap pyright cannot detect.
  • [ce-lint] CE034 — model docstrings must not defer to a field description. A doc-surface check (wire as a @pytest.mark.lint class next to /Users/religa/src/coder_eval/tests/lint/doc_schema_parity.py, following the CE027–CE031 whole-tree pattern rather than BaseRule): fail when a class docstring under src/coder_eval/models/ contains see the field description / see the field docs / see the field's description. A docstring that defers to a Field(description=...) is by construction restating semantics that already have an SSOT — the exact violation of CLAUDE.md's "field descriptions defined once in Pydantic models". Verified over src/ and docs/: exactly one hit, src/coder_eval/models/criteria.py:662, so zero baseline. Prevents: A1-low: median-aggregation semantics restated in four places (class docstring criteria.py:661-663, field description :771-782, _grade_with_sampling docstring, TASK_DEFINITION_GUIDE) — the self-deferring class-docstring copy is the one a static check can kill.
  • [ce-lint] CE036 (optional, low priority) — a numeric upper bound must be justified in its field description. For the CE030-tracked user-authored config models, require that Field(..., le=N) / lt=N restate the bound value in description, so "why this number" is authored where the number lives. Cost is measured and small but non-zero: 10 fields across models/criteria.py + models/tasks.py carry an le/lt bound and 9 do not mention it (criteria.py:112, 677, 758, 767, 860; tasks.py:84, 112, 267, 282), so this needs a one-time pass over 9 short descriptions before the gate can go green. Worth adopting only if bound rationale should be a standing invariant; otherwise this finding stays a review catch. Prevents: A7-low: bare le=9, at models/criteria.py:770 with no rationale in either the 12-line field description or the guide row, leaving the next reader unable to tell a cost guard from a semantic limit.
  • [ce-lint] CE037 (optional, low priority) — judge-criterion field-parity allowlist. LLMJudgeCriterion and AgentJudgeCriterion deliberately mirror each other's authoring surface (the comment at criteria.py:851 says so). Measured today: 13 shared fields, 4 llm-only (model, temperature, max_tokens, samples), 3 agent-only (agent, max_turns, turn_timeout). A check requiring every asymmetric field to appear in an explicit _JUDGE_FIELD_ASYMMETRY = {field: reason} map turns "oversight or decision?" into a compile-time answer with a 7-entry baseline. Because both models are extra="forbid", an unrecorded asymmetry surfaces to users only as a bare extra_forbidden error on agent_judge: {samples: 3}. Prevents: A5-low + grouped A7-low: samples added to only one of the two sibling judge criteria, with no recorded rationale and no doc/error pointer for agent_judge.

Harness improvements (not statically reachable):

  • Cyclomatic-complexity ratchet in make lint, radon-based with a frozen baseline. Add a whole-tree lint test (CE027–CE031 style) that runs radon.complexity.cc_visit over src/coder_eval and fails when a function is newly above CC 10 or exceeds its recorded baseline value, with the baseline checked in as a JSON/TOML map. radon is already a runtime dependency and already wrapped by src/coder_eval/scoring/complexity.py, so this adds no tooling. Why not static: Ruff's mccabe implementation scores _grade_with_sampling at 8, not 15 (verified: silent at --select C901 --config lint.mccabe.max-complexity=10; it only trips at 7), so no non-disruptive C901 threshold catches this function — radon's counting does (CC 15, rank C). But a flat radon threshold is not adoptable either: 129 functions in src/ already exceed CC 10 (98 C / 23 D / 6 E / 2 F), topping out at orchestration/experiment.py:789 aggregate_results at 48. Hence a baseline ratchet rather than a gate. I also tested and reject the more direct check — flagging duplicate result-construction shapes within a criterion module — as unviable noise: 9 of 14 criterion modules already construct the same *CriterionResult keyword shape more than once (reference_comparison.py 6x, run_command.py 4x, uipath_eval.py 4x). Prevents: A1-medium: _grade_with_sampling (llm_judge.py:229, CC 15) duplicates _check_impl's result-construction tail into a second judge path that will drift, plus the two error-accumulator lists only ever read at index 0.
  • Route-matrix fixture for the judge test suite. Add a shared parametrized judge_route fixture to /Users/religa/src/coder_eval/tests/test_llm_judge_criterion.py (or conftest.py) yielding (route, patch_target) for DirectRoute() -> coder_eval.criteria.llm_judge.invoke_anthropic_judge and BedrockRoute(bearer_token=..., region=...) -> invoke_bedrock_judge, then move the behavioral judge tests onto it so every present and future judge test runs on both arms by construction. Once the fixture exists a small lint test can sustain it: any test patching invoke_*_judge directly must consume judge_route or carry an explicit route-specific marker. While there, extend test_judge_multi_sample_token_usage_summed with cache_creation_input_tokens / cache_read_input_tokens (the buckets N identical prompts actually populate) and add a token_usage is None case for _sum_usage's no-usage branch. Why not static: This is a combination gap, not a branch gap: both arms of the match route: dispatch and the capture_transcript=False early return are already executed by single-sample tests, so branch coverage (already on — branch = true in [tool.coverage.run]) and any per-file or diff-coverage floor report them as covered. Which feature x route pairs get exercised is a property of test parametrization at runtime; no AST or coverage check can see that samples>1 was never run on the Bedrock arm. Prevents: A3-low: samples>1 exercised only on the Direct/Anthropic arm with default toggles, leaving untested the Bedrock route (the one the nightly uses via --backend bedrock, and the one whose invoker raises JudgeInfrastructureError for every failure mode — i.e. the escalate-vs-degrade branch), capture_transcript=False, cache-bucket summing, and the token_usage is None case.
  • Close the judge cost/latency blind spot around samples. Three pieces: (1) fan the N samples out concurrently instead of the serial for _ in range(criterion.samples): at llm_judge.py:257 — they are independent and check_all already runs off the event loop via asyncio.to_thread — removing the wall-clock multiplier entirely; (2) add a regression test with a fake judge that sleeps, asserting samples=N wall-clock stays ~ one sample (the assertion that keeps it concurrent); (3) give judge spend a gate it currently lacks — a run_limits.max_judge_usd cap or a separate reported judge-budget line — since orchestrator.py:848-852 documents that judge usage is intentionally excluded from the max_usd aggregate. Also document the trade at docs/TASK_DEFINITION_GUIDE.md:840: N samples multiply judge spend, max_usd does not see it, and in simulation mode with check_criteria: every_turn the multiplier compounds to samples x turns against a single task_timeout. Why not static: The failure is an interaction between accumulated wall-clock and a watchdog (ThreadedWatchdog(timeout_seconds=task_timeout) at orchestrator.py:468 wrapping the check_all at :1409, default task_timeout: 600 in experiments/default.yaml:29) and between per-call USD and a cap living in a different aggregate — both runtime state no AST can evaluate. I also tested the one plausibly static form — flagging a known network invoker called inside a loop in criteria/ — and it detects nothing here: the loop body calls the local _invoke_tool_channel, so the invoker is one level of indirection away and detection would need a call graph. Prevents: A8-medium: samples runs up to 9 serial judge calls (each with a 120 s transport ceiling, more on Bedrock's retry loop) inside the task_timeout watchdog, able to flip a row to FinalStatus.TIMEOUT for identical agent output, while multiplying judge spend that run_limits.max_usd cannot gate.
  • Recorded boundary — two findings and one check shape static analysis cannot reach; leave to review, add no rule. (a) The redundant float(median(scores)) at llm_judge.py:297 is redundant only because scores is list[float]; statistics.median returns the element type, so float(median(xs)) over list[int] is a real conversion. An AST-only CE rule would therefore be wrong, and there is no type-aware setting to flip — verified ruff 0.15.x has no FURB123 (--select FURB123 errors out), RUF046 is int-only, and pyright's reportUnnecessaryCast covers only typing.cast. (b) The samples vs dataset.sample_per_stratum vocabulary collision is not mechanically separable from legitimate reuse: a stem-collision check across the YAML-authored config models finds 17 colliding stems today (max_* alone has 11 members, expected_* 6), so the rule would be pure noise. Both stay in the reviewer's judgment column, and this bullet is the deliberate record of that. Why not static: (a) needs inferred element types at the call site — pyright's domain, with no corresponding diagnostic available; (b) needs semantic judgment about whether two names in the same YAML surface denote the same concept, with a measured noise floor of 17 pre-existing collisions proving a mechanical version unusable. Prevents: A2-low (redundant float() cast) and A7-low (samples collides with the dataset-row sample_* vocabulary) — recorded as review-only rather than silently omitted from the lint proposals.

Top 5 Priority Actions

  1. Run the judge samples concurrently instead of serially at src/coder_eval/criteria/llm_judge.py:257 (they are independent and check_all is already off the event loop via asyncio.to_thread), which removes the up-to-9x wall-clock multiplier that today can consume the shared task_timeout: 600 budget (experiments/default.yaml:29) and flip a row to FinalStatus.TIMEOUT for identical agent output — worse in simulation check_criteria: every_turn mode where it becomes samples x turns.
  2. Stop discarding diagnostics in the sample loop at src/coder_eval/criteria/llm_judge.py:271-272: log each swallowed exception at WARNING with exc_info=True and put type(exc).__name__ into the degraded note, so a systematic first-party regression (e.g. a TypeError/ValidationError in verdict parsing that trips 1-in-3 responses) can no longer ship as a green criterion with a quietly shifted median the way criteria/base.py:79-96 would have caught at samples: 1.
  3. Collapse the two parallel judge paths into one by deleting the if criterion.samples > 1: dispatch at src/coder_eval/criteria/llm_judge.py:122-129 and always running range(criterion.samples) (N=1 is a one-element median and _sum_usage of one usage is that usage), gating only the sample_scores and degraded-note renderings on samples > 1 — this removes two near-verbatim JudgeCriterionResult construction blocks (145-166 vs 286-317) that will otherwise drift apart and drops _grade_with_sampling below its current radon C(15).
  4. Add the missing samples>1 coverage in tests/test_llm_judge_criterion.py (the new block at :1084-1131 is Direct/Anthropic-only): one Bedrock multi-sample test — the nightly's --backend bedrock route, and the arm whose invoker raises JudgeInfrastructureError for every failure mode, i.e. the escalate-vs-degrade branch — plus cache-bucket fields and a token_usage is None case in test_judge_multi_sample_token_usage_summed (:1288), since repeated identical prompts are exactly what populates the cache-read buckets that drive cost.
  5. Close the authoring-surface gaps around the new knob: document the latency/task_timeout and ungated-judge-spend trade at docs/TASK_DEFINITION_GUIDE.md:840, record why samples is llm_judge-only (so agent_judge: {samples: 3} stops failing with a bare extra_forbidden, src/coder_eval/models/criteria.py:816) and why the bound is le=9 (:770), disambiguate the name against the existing dataset sample_*/--sample vocabulary, and while in the file make _build_transcript (llm_judge.py:331) keyword-only and have _invoke_tool_channel return _SampleOutcome by keyword instead of the positional *-splat at :259-268.

Stats: 0 🔴 · 1 🟠 · 3 🟡 · 7 🔵 across 8 axes reviewed.

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