[GBX-Max1220]

feat(judge): validated named metrics and persistence (script judge, slice 1)

Refs #317. First bounded implementation slice, scoped per #317 (comment) — named metrics only, with report integration and cross-run comparison left for later PRs. Marked draft because this is slice 1 of the agreed plan and awaits maintainer confirmation.

What this adds

A per-judge named-metric registry, a script-judge result channel that feeds it, and the derived state that reaches the persisted artifacts. Per the maintainer decision, NOT_RUN is dropped (a never-started and a stopped-mid-flight member are both INTERRUPTED, separated by execution_stage) and no group_execution_state is introduced.

Configuration — judge.metrics[] with id, required, min_score, evidence_required, validated in internal/config (identifier shape, duplicate ids, thresholds inside [0,1]). The registry is the coverage denominator: fixed before execution, never inferred from a returned payload. Unknown ids in output are diagnostics and never create slots; a duplicated id invalidates its planned slot once.

Input channel (script judge only) — the runner assigns one private, invocation-bound result path per attempt and injects EVAL_METRICS_RESULT_PATH, EVAL_METRICS_INVOCATION_ID and EVAL_METRICS_TEMPLATE. The path is Lstat-ed and must be a regular, non-symlink file; its size is checked before it is read; the payload must echo the invocation id. Rejected as a whole envelope: unparseable JSON, non-object top level, trailing content, duplicate JSON keys at any level, unknown top-level or entry fields, wrong schema_version, unbound/stale invocation, oversized file. A nonzero exit never collects scores even if a valid file exists — the file stays an on-disk diagnostic. stdout remains evidence; there is no stdout fallback.

State — four fields, one question each:

Field	Values
execution_stage	NOT_INVOKED / STARTED / COMPLETED
completion_state	INTERRUPTED / FAILED / INCOMPLETE / COMPLETED
termination_cause	none / cancellation / deadline_expiry / expect_gate / process_exit / execution_error / contract_error / judge_declined (+ deadline_source / cancel_source)
score_availability	AVAILABLE / MISSING / ERROR / SKIP / NOT_PRODUCED

metric_quality_state is present whenever the envelope exists and is derived from slots only, never from completion_state: thresholds configured and all thresholded values available → MET/NOT_MET; thresholds configured but a thresholded value unavailable (including cancellation, gate and error cases) → UNKNOWN; valid scores with no thresholds configured → NOT_CONFIGURED; no valid scores → NOT_ASSESSED. A judge-declared SKIP keeps its own reason and is never reachable for a slot the execution layer never reached — that slot is NOT_PRODUCED, so an interrupted run cannot masquerade as a judge declining a metric. Scores are never fabricated as zero.

Group completion is derived, not assigned — folded over member completions by FAILED > INCOMPLETE > INTERRUPTED > COMPLETED, so failed + pending-cancelled keeps FAILED while the cancellation and the unfinished members stay visible on their own members, and completed + pending-cancelled reads INTERRUPTED. Members with no execution metadata contribute nothing, and an empty set leaves the field unset so the artifact omits it rather than inventing a state.

Timeouts do not discard a trustworthy result — a child that writes its bound result and then outlives the deadline keeps both scores AVAILABLE (an explicit 0 included) and the existing script-timeout verdict mapping is preserved unchanged: grouped ERROR, single-judge FAIL. Cancellation arriving while a member is running is never treated as acceptance, so its untouched slots read NOT_PRODUCED rather than an omission from a completed judge.

Whole-envelope vs single-entry rejection — an entry-level defect (out-of-range, non-finite, wrong type, evidence requirement unmet) poisons only that slot; independently valid siblings stay AVAILABLE and are treated identically by the collector, the stored envelope and any later consumer.

Compatibility, deliberately untouched — PASS/FAIL/SKIP/ERROR; the #309 assertion and member denominators with zero-credit skipped members (skipped: true retained in summary.total and counted in summary.failed); the all_required one-assertion-per-member aggregation; grading.json; the JUnit mapping; the existing single-vs-grouped script-timeout asymmetry. No JSONL evaluation-event field is added here — that stays behind its own schema and consumer review. cancel_source reports os_signal only when an interrupt was actually observed and unknown otherwise; it is never inferred from error text.

Persistence — the envelope rides on judge.Result.Metrics (report.json → grading.metrics) and on each grouped judge.Outcome (evaluation.json → judge_results[].metrics, judge_results[].execution), with aggregation.completion_state / aggregation.termination_cause alongside the unchanged aggregation.status. Nothing lives only inside judge.Result: a member with no result at all still carries its execution facts and its registry-derived slots.

Verification status — stated precisely

Verified by real execution on this machine (Windows, go1.26.4, base 6fec8e9):

• a real script child writes its invocation-bound result and exits 0 → both scores (including 0.0) reach the envelope and the derived state, through executeCase for the single-judge path and the grouped path
• real child-process timeout — the child writes its bound result, then outlives a 1s deadline and is killed: scores stay AVAILABLE, quality=NOT_CONFIGURED, and the verdict mapping stays ERROR (grouped) / FAIL (single). This is TestScriptJudgeRealSubprocessTimeoutCollectsResult.
• a separately written set of tests uses an already-expired context plus a pre-placed file to exercise the collector's deadline branch deterministically; that path is not a subprocess-kill test and is not described as one
• cancellation with the context already cancelled → member INTERRUPTED / NOT_PRODUCED / NOT_ASSESSED, group INTERRUPTED + cancellation, member assertion denominator unchanged
• whole-envelope vs per-entry rejection, duplicate and unknown ids, non-regular path, oversized file, unassigned channel, and the artifact round-trip through the real WriteEvaluation writer

Not verified on this machine:

• the full end-to-end "accepted member, then a later cancellation" case (TestExecuteCaseCancellationKeepsAcceptedResult) is POSIX-only and skipped here: on Windows whether the first member finishes before the timer fires depends on process-startup timing, so the ordering is not deterministic. The branch therefore does not claim this ordering as proven end-to-end; the decision itself is covered at unit level by recordMemberContextError with a genuinely cancelled context and deterministically by TestExecuteCaseCancellationInterruptsRunningMember. Maintainer or CI confirmation on Linux is welcome.
• sandboxed-runtime collection and any non-none environment behaviour beyond the explicit metrics_channel_unsupported result

Test status: go build ./..., go vet, gofmt clean. internal/judge, internal/config, internal/report, internal/cli pass. internal/evaluator shows 4 failures and internal/runtime fails to build. I ran the identical full-suite command (./internal/... ./pkg/..., -count=1) on base commit 6fec8e9 in this same Windows environment and on this branch: both give 18 passing packages, the same 9 failing tests and the same 4 failing packages, including the internal/runtime build failure. That comparison is environment-scoped: it shows this branch adds no new failure here, and it is not a claim of zero regressions on Linux/macOS, which CI will establish.

Out of scope for this PR

• agent_judge numeric metric output (its strict result contract is separate work)
• metrics collection for sandboxed runtimes — only the local none environment is assigned a path; anything else reports metrics_channel_unsupported instead of pretending a stale file was found
• report/HTML/Markdown rendering of metrics, benchmark statistics, per-metric comparisons and the model-identity advisory consumer; no unconsumed compatibility helper is shipped here, it returns together with its consumer
• any change to the JSONL evaluation-event schema
• normalising the existing single-vs-grouped script-timeout verdict asymmetry
• finer cancel-source attribution beyond an observed process interrupt

Co-authored-by / AI assistance note: this slice was drafted with AI assistance against the contract in #317 and reviewed commit by commit; no test was weakened or skipped to make it pass.

[CLAassistant]

All committers have signed the CLA.

[zpzjzj]

Thanks for the first implementation slice. I reviewed HEAD 72b449d and reproduced several issues in the scoring and persistence paths. Please address these before merging; CI-reported lint/build/test failures are excluded from this review.

1. Apply the opt-in metric policy to verdicts.
   An exit-zero script currently remains PASS regardless of its metric result. With min_score=0.8 and a score of 0, quality becomes NOT_MET but the case still passes. Missing required metrics and malformed payloads also leave the verdict PASS. Apply threshold FAIL and contract/evaluation ERROR semantics when metrics are explicitly configured, while preserving legacy behavior for configurations without metrics.

2. Complete single-judge execution metadata and planned-slot persistence.
   The new execution metadata is constructed only in the grouped path. A single-judge gate failure loses the planned metric slots, and a nil Result has no independent execution record. Single-judge timeout results also lack structured timeout/source metadata alongside retained scores. Please carry this information through the actual evaluator and persisted case record, including early returns.

3. Finalize group state on short-circuit paths.
   recordSkippedJudges creates interrupted member outcomes, but gate failures return without calling foldGroupState. The resulting aggregation has no completion or termination cause. Apply consistent finalization to gate failures and other early-return paths that materialize skipped members.

4. Respect optional metrics when deriving completion.
   completionForContract treats every unavailable metric as an unmet requirement. A valid result with only optional metrics, no thresholds, and an empty metrics array currently becomes INCOMPLETE. The agreed result is COMPLETED, coverage 0, quality NOT_ASSESSED. Completion derivation needs access to the configured required flags and thresholds.

5. Preserve the configured Judge identity.
   Collected envelopes hardcode judge_id to "script". A member configured as scorer-a therefore persists the wrong producer identity. Pass the stable configured ID through collection and keep it consistent with the enclosing outcome.

6. Attribute the deadline that actually expired.
   memberStop labels a Judge timeout as a case timeout whenever the parent already carries a case deadline source. For example, with a 300-second case budget and a 1-second Judge budget, the Judge expires while the parent remains active, but the recorded source is the case setting. Prioritize the expired member deadline when the parent has not expired.

Please add regression tests through the real evaluator paths, asserting verdicts and persisted metadata alongside envelope values. Existing tests that require PASS for every exit-zero metrics script, or INCOMPLETE for an optional omission, should be updated to the agreed contract.

The bounded scope remains appropriate: Agent-judge numerical output, sandbox collection, report rendering, comparisons, and JSONL schema extensions can remain in follow-up PRs.

[JHWang-1997]

Reviewed commit 72b449d356930945fb573da4ffafa23bd48ba8b0. Requesting changes before merge.

I independently reproduced the six issues in the existing review, and they remain present at this head. Please address those findings: applying the opt-in metric policy to verdicts, preserving single-judge execution metadata and planned slots, finalizing grouped state on early returns, respecting optional metrics when deriving completion, preserving the configured judge identity, and attributing the deadline that actually expired.

The verdict issue is the highest priority: an exit-zero script with min_score: 0.8 and score 0 still produces a case-level PASS, despite metric_quality_state=NOT_MET. Missing required metrics and an invalid score of 2 also leave the case passing. Configurations that explicitly opt into these constraints need the agreed failure/error semantics, while configurations without metrics should retain their existing behavior.

I also found an additional deterministic test-fixture defect, described in the inline comment: the fast and slow judges in TestExecuteCaseCancellationKeepsAcceptedResult reference the same script file. The second write overwrites the first, so the test never reaches the accepted-result state it is intended to protect.

Validation on macOS in an isolated snapshot:

• go test -race ./internal/judge ./internal/config ./internal/evaluator ./internal/report: judge, config, and report passed; evaluator failed at TestExecuteCaseCancellationKeepsAcceptedResult.
• Temporary targeted reproducers confirmed the verdict, gate-state, optional-completion, identity, and deadline-attribution findings.

These requests stay within the current slice. Agent-judge numeric output, sandbox metrics collection, report rendering, comparisons, and JSONL schema changes can remain follow-up work.

[JHWang-1997]

[P2] Give the fast and slow judges distinct script paths

Both metricsScript calls use the same scratch directory, and the helper always writes score.sh on POSIX, so creating slow overwrites fast before either member runs. Consequently, the first member also sleeps and is cancelled, causing TestExecuteCaseCancellationKeepsAcceptedResult to fail instead of exercising retention of an already accepted result. I reproduced this on macOS with go test -race ./internal/evaluator -run '^TestExecuteCaseCancellationKeepsAcceptedResult$' -count=1: the first member was interrupted and its metrics were NOT_PRODUCED. Give the two scripts distinct directories or filenames, then rerun this test while preserving its assertions and platform coverage.

[GBX-Max1220]

Thank you both for the detailed reviews and for reproducing these issues. I appreciate the concrete examples and the extra check on the cancellation test fixture.

I’ll work through the reported issues, including the verdict handling, early-return paths, group state, metric identity, and timeout attribution. I’ll also fix the fixture and add regression tests that exercise the evaluator paths.

I’ll update the PR when I have changes ready for another review.

[GBX-Max1220]

Thanks for the detailed review. I pushed three commits to the PR branch at 32b98f2.

1. The opt-in metric policy now affects verdicts: a threshold miss fails, while an invalid or unavailable required/thresholded result errors; configurations without a metric registry retain the legacy verdict. A real executeCase table covers the single-judge path. The grouped path uses the same finalizer, but a dedicated grouped threshold-failure end-to-end case is still missing; that coverage remains a limitation.
2. Single-judge early returns now retain execution metadata and planned slots independently of judge.Result; a runner-level test reads them back from evaluation.json. The real child-process timeout test also confirms a bound score survives the deadline.
3. Short-circuit paths that materialize skipped members now finalize the group completion and cause.
4. Optional-only metrics no longer make an otherwise completed judge INCOMPLETE; coverage and quality remain separately reported.
5. Metric envelopes use the configured judge ID, with a fallback only when it is unset; this is checked on single, grouped, and persisted paths.
6. Deadline attribution now prefers the member judge deadline when it expires while the parent context remains active; single- and grouped-path tests cover this.

The cancellation fixture now gives the fast and slow judges distinct script paths. The accepted-result-then-cancellation end-to-end test passes on Linux; it remains skipped on Windows because process-startup timing makes that ordering nondeterministic there.

Validation on Linux passed: the real pre-commit hooks for all three commits, make verify, make test (including go test -race ./...), and make e2e (88 passed, 40 skipped for unavailable optional agent binaries/API credentials). GitHub Actions for the new head are waiting for maintainer approval, so there is not yet a CI result. The PR remains a draft.

[zpzjzj]

Thanks for the fixes. I re-reviewed HEAD 32b98f2 across all 25 changed files. The previously reported normal-path issues are addressed, and the cancellation fixture is corrected. Locally, go build ./... and race tests for judge, config, evaluator, report, CLI, and runner all passed.

Additional real-subprocess tests through executeCase reproduced three remaining issues. These are separate from CI-reported lint/build/test failures:

1. [P1] A threshold miss can downgrade a timeout ERROR to FAIL.
   In a grouped script judge, write an invocation-bound score of 0.2, configure min_score: 0.8, and then sleep past the Judge deadline. The script layer correctly returns ERROR with the valid score retained, but finalizeJudgeMetrics calls applyMetricFailure because quality is NOT_MET and overwrites the verdict to FAIL. The resulting case has no top-level error, so retryReasonForResult no longer recognizes it as a retryable timeout. Preserve the execution ERROR and timeout provenance while reporting NOT_MET and the valid score separately; numerical policy must not lower an existing execution-error verdict.
   Location: metrics_finalize.go:43–44.

2. [P2] Nonzero script exits are reclassified as missing-required-score contract errors.
   With a required metric configured, write a valid result file and then exit 3. The collector correctly declines to accept the file, but caseMetricsEnvelope treats the missing envelope as output from a completed scoring pass because the Judge started and the context is still active. It materializes MISSING, and the finalizer changes the original FAIL into ERROR with cause contract_error. This reproduces on both single and grouped paths. Preserve the nonzero-exit FAIL/process_exit behavior, mark unproduced slots NOT_PRODUCED, and keep the raw file diagnostic-only. Execution outcome must take precedence over required-score validation; distinguish an unsuccessful process from an exit-zero process that omitted required output.
   Location: metrics_finalize.go:49–61.

3. [P2] Metric-policy failures change the compatibility assertion denominator.
   appendMetricFailure adds a synthetic “named metric contract” boolean assertion and increments Summary.Total/Failed. An exit-zero single script with a below-threshold score therefore changes from one passed script assertion to passed=1, total=2, pass_rate=0.5. This also propagates into Anthropic grading and benchmark projections. The agreed contract keeps numerical scores and their decisions separate from existing assertion/member pass-rate arithmetic. A configured threshold may change the verdict, but should not introduce a new compatibility assertion or denominator entry. Store the metric decision and its evidence in the additive metric/execution metadata instead.
   Location: metrics_finalize.go:118–125.

Please add evaluator-level regressions for timeout plus a below-threshold valid score, nonzero exit after writing a valid metrics file, and unchanged compatibility assertion counts under metric-policy failure. The first two need both execution and scoring outcomes asserted together; testing the collector or quality derivation alone misses the finalizer interaction.

The first-slice scope remains appropriate. These three corrections should stay within scoring/persistence; the deferred Agent-judge output, sandbox collection, rendering, comparisons, and JSONL schema work need not be pulled into this PR.

[GBX-Max1220]

Thanks for the fixes. I re-reviewed HEAD 32b98f2 across all 25 changed files. The previously reported normal-path issues are addressed, and the cancellation fixture is corrected. Locally, go build ./... and race tests for judge, config, evaluator, report, CLI, and runner all passed.

Additional real-subprocess tests through executeCase reproduced three remaining issues. These are separate from CI-reported lint/build/test failures:

1. [P1] A threshold miss can downgrade a timeout ERROR to FAIL.
   In a grouped script judge, write an invocation-bound score of 0.2, configure min_score: 0.8, and then sleep past the Judge deadline. The script layer correctly returns ERROR with the valid score retained, but finalizeJudgeMetrics calls applyMetricFailure because quality is NOT_MET and overwrites the verdict to FAIL. The resulting case has no top-level error, so retryReasonForResult no longer recognizes it as a retryable timeout. Preserve the execution ERROR and timeout provenance while reporting NOT_MET and the valid score separately; numerical policy must not lower an existing execution-error verdict.
   Location: metrics_finalize.go:43–44.
2. [P2] Nonzero script exits are reclassified as missing-required-score contract errors.
   With a required metric configured, write a valid result file and then exit 3. The collector correctly declines to accept the file, but caseMetricsEnvelope treats the missing envelope as output from a completed scoring pass because the Judge started and the context is still active. It materializes MISSING, and the finalizer changes the original FAIL into ERROR with cause contract_error. This reproduces on both single and grouped paths. Preserve the nonzero-exit FAIL/process_exit behavior, mark unproduced slots NOT_PRODUCED, and keep the raw file diagnostic-only. Execution outcome must take precedence over required-score validation; distinguish an unsuccessful process from an exit-zero process that omitted required output.
   Location: metrics_finalize.go:49–61.
3. [P2] Metric-policy failures change the compatibility assertion denominator.
   appendMetricFailure adds a synthetic “named metric contract” boolean assertion and increments Summary.Total/Failed. An exit-zero single script with a below-threshold score therefore changes from one passed script assertion to passed=1, total=2, pass_rate=0.5. This also propagates into Anthropic grading and benchmark projections. The agreed contract keeps numerical scores and their decisions separate from existing assertion/member pass-rate arithmetic. A configured threshold may change the verdict, but should not introduce a new compatibility assertion or denominator entry. Store the metric decision and its evidence in the additive metric/execution metadata instead.
   Location: metrics_finalize.go:118–125.

Please add evaluator-level regressions for timeout plus a below-threshold valid score, nonzero exit after writing a valid metrics file, and unchanged compatibility assertion counts under metric-policy failure. The first two need both execution and scoring outcomes asserted together; testing the collector or quality derivation alone misses the finalizer interaction.

The first-slice scope remains appropriate. These three corrections should stay within scoring/persistence; the deferred Agent-judge output, sandbox collection, rendering, comparisons, and JSONL schema work need not be pulled into this PR.

Thanks for the three reproductions — they exposed real issues. I pushed fixes and regression tests in ba9635e and 0dfde3a.

1. A threshold miss no longer downgrades an existing timeout ERROR. The timeout cause, deadline source, retry classification, and collected score are preserved.

2. A nonzero script exit remains a process failure. Its metrics file is not accepted, and the planned slots are reported as NOT_PRODUCED.

3. Metric decisions no longer add assertions or change the existing summary denominator. One consequence is that a threshold miss can produce a FAIL verdict while the assertion pass rate remains 1.0; the metric decision is reported separately.

The new tests exercise these cases through the evaluator with real script processes. I also checked that they fail when the production fixes are reverted.

On Linux, the repository checks and race tests passed locally. The GitHub workflows for the new head are still awaiting approval, so I don't have CI results yet.

The PR remains a draft, and the previously listed follow-up work is still outside this slice.