feat(evaluation): preserve criteria after agent failures

[tmatup]

Summary

• evaluate available criteria after a terminal agent failure while the sandbox and drained turn records are still readable
• record that evidence in post_failure_criteria_results, with evaluation_status distinguishing evaluated from not_evaluated
• warn at plan and once at runtime when task_timeout > turn_timeout, without rejecting or mutating the resolved limits

Design

ERROR remains the terminal status and its canonical weighted_score remains 0.0. Post-failure results are deliberately separate from success_criteria_results, so they do not affect scoring, gating, or suite aggregation. Existing task JSON remains readable because both the sibling result list and evaluation_status have backward-compatible defaults.

Turn timeouts and agent crashes run the existing SuccessChecker before teardown. Agent-dependent criteria are marked not evaluated only when no turn record survived. Task-timeout exhaustion records a full not-evaluated vector because grading cannot extend the existing task-timeout envelope. Budget failures reuse already-collected canonical criteria and only run the diagnostic pass if no complete result vector exists. Judge-infrastructure and checker-misuse errors retain their existing escalation behavior. Diagnostic judge costs and transcript siblings remain accounted and reloadable.

The timeout warning states the precise relationship: a larger task_timeout cannot extend the agent's single iteration; the agent budget is turn_timeout. task_timeout still governs the surrounding task work and grading.

Validation

• make verify: 4,176 passed, 6 environment-specific skips, 91.61% coverage
• focused error, serialization, warning, cost, and transcript tests: 176 passed
• custom lint considered; this behavior depends on runtime unwind and post-merge values rather than a stable syntax pattern, so no rule was added

This is not a request to rescore any historical run.

Fixes #114

🤖 Generated with Codex
Co-Authored-By: Codex

[github-actions]

Claude finished @tmatup's task in 1m 33s —— View job

---

Code Review in Progress

Todo List:

• Read .github/code_review.md for review guidelines
• Read CLAUDE.md for project conventions
• Get full PR diff with git diff origin/main...HEAD
• Read each changed file in full for context
• Perform cross-file consistency checks
• Analyze design-level decisions and architecture
• Check backward compatibility
• Review test coverage
• Post comprehensive review feedback

Working on reviewing PR #119...

[tmatup]

@claude Please rerun the complete PR review. The opened-event run ended with is_error=true before posting its final review. Follow .github/code_review.md, cover every severity, and post the final findings (including What's Missing) so all threads can be triaged.

[github-actions]

Claude finished @tmatup's task in 1m 38s —— View job

---

PR Review in Progress

Task List:

• Read .github/code_review.md for review standards
• Read CLAUDE.md for project conventions
• Get full diff with git diff origin/main...HEAD
• Review each changed file comprehensively
• Check for correctness issues
• Check for design and architecture issues
• Perform "What's Missing" analysis
• Post final review findings

Starting comprehensive PR review...

[tmatup]

@claude Constrained review retry: the prior two runs both ended is_error=true after exactly four turns. Do not post a progress checklist. Use at most three analysis turns for the guidelines, core diff, and tests, then use the fourth turn to post the complete final review via the PR-comment tool. Include every severity and What's Missing; if there are no findings, say so explicitly. Posting the final review takes priority over further file reads.

[github-actions]

Claude finished @tmatup's task in 1m 12s —— View job

---

I'll analyze this and get back to you.

[tmatup]

@claude Please review the updated PR head after main was merged. Follow .github/code_review.md and post the final findings, including What's Missing.

[github-actions]

Claude finished @tmatup's task in 1m 32s —— View job

---

PR Review in Progress

Reviewing: feat(evaluation): preserve criteria after agent failures

Todo List:

• Read .github/code_review.md for review guidelines
• Analyze full diff (origin/main...HEAD)
• Review core implementation changes
• Review test coverage
• Post final review with all severities and What's Missing

[bai-uipath]

Approve — the problem is real, well diagnosed, and the approach makes sense. Everything below is a suggestion, none of it blocking.

Suggested simplification: one criteria list, not two

The parallel list re-encodes the conflation the issue is about. A criterion checked after a crash is not a different kind of object from one checked before: same checker, same criteria, same sandbox, same bytes on disk. "How the run ended" is final_status and "what the artifacts show" is the criteria results; those are already orthogonal fields, and forking the schema couples them again. The tax is visible in this diff already (spill, load, cost rollup, dump exclude all learn about two lists), and it recurs for every future feature that touches criterion results.

Make the weighted score consult the status instead. The only thing the fork actually protects is the unconditional score computation in finalize, which derives everything from the criteria list and never looks at whether the agent crashed. Gate it there, average over evaluated entries only, and return None rather than a fabricated 0.0 when nothing ran. The gate stays final_status == SUCCESS, so a crashed run that passes every criterion is still not a pass.

Moving the nightly numbers is fine. ERROR rows getting a real coverage-aware score is the point of the change, not a regression to route around.

evaluation_status then does real work. Three values rather than two (evaluated, evaluated_post_failure, not_evaluated) keeps the provenance distinction without a second list. The full-length placeholder vector becomes necessary rather than noise, since it is what holds the positional alignment the aggregates depend on.

Surface it

Nothing renders the new evidence. It lands in task.json and no report, HTML, JUnit, or evalboard panel reads it, so answering "did the artifact pass when the agent crashed?" still means grepping task.json by hand. One list makes most of this free (the HTML criteria section already renders it); evalboard needs the status carried on the DTO and a badge, the same way gating was handled for weight-0 criteria.

Minor, fix if you agree

• The diagnostic pass runs judges. LLM and agent judges are not requires_agent, so every crashed row fires the full judge set, including after a cost-budget breach. The issue asked for deterministic artifact-only checks; restricting to non-judge types removes the cost exposure.
• Unguarded re-grade in simulation mode. With check_criteria: every_turn or both the criteria are already scored each turn, so a mid-dialog crash pays for a complete extra pass. The "already have a full vector" check exists but only on the budget branch; apply it to all three terminal errors.
• The recovery path can rewrite the terminal record. A judge-infrastructure failure during diagnostics replaces the original error message, and a budget breach becomes plain ERROR; a watchdog fire during grading turns a crash into TIMEOUT. Diagnostics should never overwrite the row's cause of death.
• The timeout warning is inverted. It fires whenever task_timeout > turn_timeout, which is every task in the repo and the default experiment, and stays silent on turn_timeout >= task_timeout, where the turn budget genuinely never binds.
• Nits: unreachable TaskTimeoutError handler in the new wrapper (the loop never raises it), one-shot warning flag guarding a setup path already called once, duplicated cancel-to-timeout block and reason strings, REPORT_SCHEMA not updated for the new fields, and the ASD-STE-100 line in CLAUDE.md belongs in its own PR.

[uipreliga]

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

Scope: pr:119 (13 files) axis:1,2,3,4,5,6,7,8 · branch fix/114-grade-after-agent-error · 3d3774b · 2026-08-16T08:48Z · workflow variant

Change class: complex — adds a nested post-failure diagnostic grading path with new exception control flow in the orchestrator, a new persisted CriterionResult field, and a new EvaluationResult list that changes task.json schema and judge-transcript spill naming

The architecture, security posture, and type discipline are excellent (Architecture 10.0, Security 9.9, Type Safety 9.4) and the post-failure-evidence feature is a genuinely valuable addition, but its error-handling seams are the real risk — three separate paths in _run_evaluation_with_failure_evidence can rewrite a run's final_status (ERROR→TIMEOUT, COST_BUDGET_EXCEEDED→ERROR) or silently drop the evidence vector for byte-identical agent output, an inverted validate_run_limits warning fires on 44 of 46 shipped tasks while staying silent on the genuinely broken config, and the new persisted surfaces reach no renderer or doc; fix the four orchestrator/run-limits issues before merge and this is a strong 9+ change.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	8.4 / 10	0	0	3	1	Unreachable except TaskTimeoutError branch in _run_evaluation_with_failure_evidence, duplicating a message string
2. Type Safety	9.4 / 10	0	0	1	1	judge_cost_usd reaches token_usage via string getattr, erasing CriterionResultUnion to Any on the line the PR widened to both result lists (models/results.py:979; token_usage declared at :203, extra="allow" at :69)
3. Test Health	8.3 / 10	0	1	1	2	The new post-failure evaluation funnel's branches are untested: the BudgetExceededError short-circuit, the all-agent-dependent path, and the recovery-failure handlers all have zero coverage
4. Security	9.9 / 10	0	0	0	1	Basename allowlist in load_judge_transcripts does not reject the literal "..", contradicting the SECURITY rationale this PR rewrote
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	7.9 / 10	0	2	0	1	A judge/checker failure during diagnostic post-failure grading overwrites the terminal error (crash/turn-timeout/budget) and leaves post_failure_criteria_results empty
7. API Surface & Maintainability	9.5 / 10	0	0	1	0	New persisted post-failure surfaces (post_failure_criteria_results, evaluation_status, post-failure-judge-.yaml) reach no reader, renderer, report row or documented run-directory contract
8. Evaluation Harness Quality	8 / 10	0	2	0	0	Post-failure grading runs under the still-armed task-timeout watchdog, so a cancel during grading rewrites final_status to TIMEOUT and discards the original terminal error

Overall Score: 8.9 / 10 · Weakest Axis: Error Handling & Resilience at 7.9 / 10
Totals: 🔴 0 · 🟠 5 · 🟡 6 · 🔵 6 across 8 axes.

Blockers

1. [Axis 3] The new post-failure evaluation funnel's branches are untested: the BudgetExceededError short-circuit, the all-agent-dependent path, and the recovery-failure handlers all have zero coverage (src/coder_eval/orchestrator.py:662-667) — The new guard is:

661:        except (AgentCrashError, TurnTimeoutError, BudgetExceededError) as terminal_error:
662:            if (
663:                isinstance(terminal_error, BudgetExceededError)
664:                and self.result is not None
665:                and len(self.result.success_criteria_results) == len(self.task.success_criteria)
666:            ):
667:                raise

Coverage at PR HEAD (verified by re-running pytest ... --cov=coder_eval.orchestrator --cov-report=term-missing in the PR worktree) reports line 667 as missed — no test in the suite ever takes the short-circuit. Yet it is the dominant production branch: the single-shot loop runs criteria before the budget gate (orchestrator.py:1781-1782 — # Budget gate runs AFTER criteria so partial-credit visibility is preserved. / self._check_run_limits(iteration=iteration)), and the dialog loop does the same at orchestrator.py:2119-2124 before re-raising. So on a real TOKEN_BUDGET_EXCEEDED / COST_BUDGET_EXCEEDED run len(success_criteria_results) == len(task.success_criteria) and the code takes line 667. The only test that drives this arm, tests/test_run_limits_orchestrator.py::test_run_arm_maps_budget_to_status, injects the error from a mocked _evaluation_loop with an empty success_criteria_results, so it exercises only the else path (asserted at tests/test_run_limits_orchestrator.py:284-285). Inverting the comparison or dropping the isinstance would make every budget-exceeded run re-grade all criteria — including llm_judge / agent_judge, i.e. real extra spend on a run that just blew its budget — and no test would fail. Add a test that raises BudgetExceededError from _evaluation_loop after populating result.success_criteria_results with one result per task.success_criteria, and assert result.post_failure_criteria_results == [] and that success_checker.check_all_async was never awaited a second time.
2. [Axis 6] A judge/checker failure during diagnostic post-failure grading overwrites the terminal error (crash/turn-timeout/budget) and leaves post_failure_criteria_results empty (src/coder_eval/orchestrator.py:681) — orchestrator.py:681-682 is except (JudgeInfrastructureError, CheckerMisuseError): / raise. That raise re-raises the RECOVERY error, so the trailing raise at line 692 (which would re-raise terminal_error) is never reached and _record_post_failure_not_evaluated is never called. The run's real cause is discarded: run()'s except Exception as e: arm sets self.result.error_message = str(e) from the judge error, and post_failure_criteria_results stays [] — indistinguishable from 'the feature never ran'. The PR's own test asserts the defect: tests/test_timeout_orchestrator.py drives side_effect=TurnTimeoutError(1200, ...) and then asserts result.error_message == "judge unavailable". This contradicts the method's own docstring intent ('preserving the original terminal error', line 688) . Fix: record the not_evaluated vector with a reason naming the escalating error, then fall through to the trailing raise so terminal_error propagates — e.g. except _ESCALATING_EXCEPTIONS as esc: self._record_post_failure_not_evaluated(f"post-failure grading escalated ({type(esc).__name__}: {esc})") and drop the bare raise. A diagnostic step must never become the reported cause of failure. Also consider importing the tuple from criteria/base.py::_ESCALATING_EXCEPTIONS (currently a second hand-maintained copy of the same pair).
3. [Axis 6] New validate_run_limits warning fires on the shipped/correct task_timeout > turn_timeout configuration (44 of 46 tasks, including experiments/default.yaml) and is silent on the genuinely degenerate inverse (src/coder_eval/orchestration/run_limits.py:28) — run_limits.py:28-34 warns whenever task_timeout > turn_timeout. experiments/default.yaml:29,31 ships task_timeout: 600 / turn_timeout: 300, so EVERY task resolved from the defaults trips it. Verified by running the function: validate_run_limits(TaskDefinition(..., run_limits=RunLimits(task_timeout=600, turn_timeout=300))) returns ("run_limits.task_timeout (600s) exceeds run_limits.turn_timeout (300s). A larger task_timeout cannot extend the agent's single iteration; the agent budget is turn_timeout.",). plan_command.py:140-143 prints it as a yellow ⚠ per task × per variant and orchestrator.py:1141 logs it per task run, so a 100-row suite emits 100 warnings on a correct config. The advice is also misleading: task_timeout is documented as 'Max seconds for the entire evaluation loop (all iterations)' (models/limits.py:52) and must cover setup, pre-run commands, check_all_async judge calls, post-run commands — and now this PR's own post-failure grading — so headroom over turn_timeout is required, not pointless. Meanwhile the genuinely broken inverse, task_timeout < turn_timeout (the task watchdog kills before turn_timeout can ever fire, making turn_timeout dead config and forcing a TIMEOUT classification instead of the partial-preserving turn-timeout path), returns () at line 28-29 and is never reported. Fix: invert the comparison to warn on task_timeout < turn_timeout with a message naming the unreachable turn_timeout, and drop the current warning (or gate it strictly on non-simulation single-shot tasks whose task_timeout is within a small margin of turn_timeout).
4. [Axis 8] Post-failure grading runs under the still-armed task-timeout watchdog, so a cancel during grading rewrites final_status to TIMEOUT and discards the original terminal error (src/coder_eval/orchestrator.py:669) — _run_evaluation_with_failure_evidence is invoked from inside the watchdog scope (with ThreadedWatchdog(...) as wd: at orchestrator.py:499-508), and post-failure grading runs there too:

668:            try:
669:                await self._evaluate_post_failure_criteria()
670:            except asyncio.CancelledError:
671:                if watchdog.fired:
...
675:                    raise TaskTimeoutError(

The watchdog is a live threading.Timer for the whole with body (agents/watchdog.py:105-111) and cancels the current asyncio task on fire. Before this PR the terminal error propagated out of the with immediately, so an AgentCrashError/TurnTimeoutError was ALWAYS FinalStatus.ERROR. Now, if the remaining task_timeout budget expires while the diagnostic checkers run, the original error is discarded and TaskTimeoutError is raised instead -> FinalStatus.TIMEOUT. models/enums.py:37,42 puts those in different reporting buckets (ERROR -> "error", TIMEOUT -> "failed"), so tasks_error / tasks_failed / error_share change for identical agent output. It is genuinely run-to-run nondeterministic: a crash late in the budget plus a judge/run_command criterion whose latency varies by tens of seconds decides the bucket. The content of post_failure_criteria_results is wall-clock-dependent for the same reason (full graded vector vs. the all-not_evaluated vector written at line 672). Fix: run post-failure grading OUTSIDE the task-timeout watchdog under its own short, independent deadline (e.g. asyncio.wait_for with a small fixed budget), and never let that deadline rewrite the terminal error/final_status.
5. [Axis 8] Post-failure grading re-executes the full criteria suite — paid judges and sandbox-mutating run_command checks — with no opt-out, no USD accounting, and a short-circuit that covers only BudgetExceededError (src/coder_eval/orchestrator.py:661) — ```
661: except (AgentCrashError, TurnTimeoutError, BudgetExceededError) as terminal_error:
662: if (
663: isinstance(terminal_error, BudgetExceededError)
664: and self.result is not None
665: and len(self.result.success_criteria_results) == len(self.task.success_criteria)
666: ):
667: raise

Three consequences, all new:
(a) COST. `_evaluate_post_failure_criteria` calls `check_all_async` over the whole criteria list (orchestrator.py:742-747), so every `llm_judge` (paid API call) and `agent_judge` (spawns a Claude Code sub-agent) is now billed on a crashed run that previously cost nothing extra. `_check_run_limits` prices only turn `token_usage` (orchestrator.py:999-1016), so this spend is invisible to `run_limits.max_usd`; it lands only in `judge_cost_usd` (models/results.py:978) and therefore in the row's `total_cost_usd`. There is no opt-out flag, unlike the analogous `run_limits.stop_early: false` kill switch.
(b) The `already-graded` short-circuit is applied ONLY to `BudgetExceededError`. In simulation mode with `check_criteria: every_turn`/`both`, `_run_dialog_criteria_check` has already populated `success_criteria_results` (orchestrator.py:1838, set at 2108) when an `AgentCrashError` on a later turn arrives — so the entire judge suite is paid for a second time even though the canonical vector is complete. Extend the guard at 662-666 to all three error types.
(c) SANDBOX MUTATION. `run_command` criteria execute arbitrary shell in the LIVE sandbox (`criteria/run_command.py:68`: `exit_code, stdout, stderr = sandbox.run_command(criterion.command, timeout=criterion.timeout)`). Post-failure grading runs before the `finally` block that captures the workspace into `run_dir/artifacts` (orchestrator.py:2444-2450), so anything those commands write is preserved and will be seen by a later `coder-eval evaluate <task> artifacts/` re-grade.

## Non-blocking, but please consider before merge

1. **[Axis 1] Unreachable `except TaskTimeoutError` branch in _run_evaluation_with_failure_evidence, duplicating a message string** (`src/coder_eval/orchestrator.py:656`) — ```python
        except TaskTimeoutError:
            self._record_post_failure_not_evaluated(
                "the task_timeout budget was exhausted before post-failure grading could run"
            )
            raise

git grep -n TaskTimeoutError pr-119 -- src/ shows exactly three raise sites: orchestrator.py:515 (in run(), after the with ThreadedWatchdog block exits — outside this function), orchestrator.py:650 and orchestrator.py:675. The latter two are raised from inside except clauses of this same try statement, and Python never routes an exception raised in an except handler to a sibling handler of that statement. Nothing in _evaluation_loop's call graph raises TaskTimeoutError. The branch is therefore unreachable, no test covers it (tests/test_timeout_orchestrator.py reaches the timeout vector via the asyncio.CancelledError branch at line 645), and it tells the next reader that _evaluation_loop can surface a task timeout — which it cannot. Delete it, or if it is deliberately defensive for out-of-tree agents that import TaskTimeoutError, say so in a comment and add the test that exercises it.
2. [Axis 1] Orchestrator run-limits warning helper: one-shot flag plus wrapper method guarding a single call site that already runs once per run, and named for one specific message while iterating a generic tuple (src/coder_eval/orchestrator.py:413) — ```python
# One-shot flag: a resolved task may be inspected more than once during
# setup, but its ineffective timeout relationship should be logged once.
self._run_limits_warning_emitted: bool = False

The premise is false. `_warn_on_ineffective_task_timeout` (line 1050) has exactly one call site, `_setup()` line 1141, and `_setup()` has exactly one call site, `run()` line 477 (`git grep -n '_setup()'` returns one hit). So the guard protects against a re-entry that cannot happen, and the only thing that exercises it is a test written to call the method twice by hand (`tests/test_timeout_orchestrator.py::test_runtime_timeout_warning_is_emitted_once`). Contrast the neighbouring `_expected_turns_warning_emitted` at line 409, whose flag is genuinely needed because `_check_expected_turns` is called from two per-turn sites (lines 1779 and 2142). Drop the flag and the wrapper and inline `for message in validate_run_limits(self.task): logger.warning(...)` in `_setup()`, or fix the comment to state the real reason if one exists.
3. **[Axis 1] _evaluate_post_failure_criteria adds a fourth spelling of the load_reference three-tuple unpack + check_all_async block** (`src/coder_eval/orchestrator.py:737`) — `_evaluate_post_failure_criteria` re-spells the block at lines 737-748:
```python
            reference_code, reference_dir, self._reference_code = load_reference(
                task=self.task,
                task_file=self.task_file,
                cached_reference=self._reference_code,
            )
            checked = await self.success_checker.check_all_async(
                runnable,
                reference_code=reference_code,
                reference_dir=reference_dir,
                turn_records=self.result.iterations,
            )

The same pair now appears at lines 1647/1652 (evaluate-only), 1705/1710 (single-shot), 1826/1831 (_run_dialog_criteria_check) and here — four sites. _run_dialog_criteria_check's own docstring records the precedent: "The block lifted verbatim from the three identical sites (per-turn, budget-gate fallback, end-of-dialog)", i.e. the codebase already decided this block gets one home. The new copy differs only in passing a runnable subset and skipping _accumulate_judge_usage / calculate_weighted_score. Parameterize the existing helper (criteria subset + whether to record/score) and call it, so the reference-loading and turn-record wiring stay single-sourced.
4. [Axis 2] judge_cost_usd reaches token_usage via string getattr, erasing CriterionResultUnion to Any on the line the PR widened to both result lists (models/results.py:979; token_usage declared at :203, extra="allow" at :69) (src/coder_eval/models/results.py:979) — PR HEAD line 978-980 reads:

    criterion_results = result.success_criteria_results + result.post_failure_criteria_results
    usages = [u for cr in criterion_results if (u := getattr(cr, "token_usage", None)) is not None]
    return sum_costs(*(u.total_cost_usd for u in usages))

Both lists are declared list[CriterionResultUnion] — a properly discriminated union in which token_usage: TokenUsage | None is declared on JudgeCriterionResult (models/results.py:194). I confirmed with pyright (1.1.408, the repo's pinned version, run on a probe module placed inside src/coder_eval/ so the include filter picks it up) that the string-keyed access throws the type away:

information: Type of "u" is "Any | None"          # getattr(cr, "token_usage", None)
information: Type of "cr.token_usage" is "TokenUsage | None"   # after isinstance(cr, JudgeCriterionResult)

So u.total_cost_usd is an unchecked Any flowing straight into sum_costs(...), whose contract is float | None. Consequences with no pyright signal: renaming/moving JudgeCriterionResult.token_usage makes judge_cost_usd silently return None for every run (judge spend vanishes from the task row); and because CriterionResult sets extra="allow" (results.py:66), any result_kind="basic" record that carries a token_usage key deserializes it as a raw dict in __pydantic_extra__, so u.total_cost_usd raises AttributeError at report time rather than being rejected.

The inconsistency is inside this PR: the same change narrows correctly two files over — evaluation/judge_persistence.py:147 uses if not isinstance(cr, JudgeCriterionResult): continue for exactly this problem. Fix: usages = [cr.token_usage for cr in criterion_results if isinstance(cr, JudgeCriterionResult) and cr.token_usage is not None].

Calibration note for the verifier: the getattr spelling predates this PR (origin/main had the identical comprehension over success_criteria_results alone); line 978-979 is the PR's own edit, and it doubled the input set flowing through the untyped access — including the new post-failure judge spend the PR's docstring change explicitly claims to account for — rather than narrowing while the file was open.
5. [Axis 3] No test populates both criterion-result lists at once, leaving the judge-transcript filename-collision fix (judge-<idx> vs post-failure-judge-<idx>) unasserted (tests/test_judge_persistence.py:136-158) — The filename change from judge-<idx>.yaml to <prefix>-<idx>.yaml exists to stop a canonical judge at index 0 and a post-failure judge at index 0 from both writing judge-0.yaml into the same directory (the second silently overwriting the first). The new test sets up only one list:

137:    judge = _make_judge_result(transcript=_make_transcript())
138:    result = _make_evaluation_result(criteria=[])
140:    result.post_failure_criteria_results = [judge]
142:    assert spill_judge_transcripts(result, tmp_path) == 1
143:    assert judge.transcript_path == "post-failure-judge-0.yaml"

criteria=[] means the collision case is never constructed; test_spill_preserves_index_for_multiple_judges likewise only fills the canonical list (a repo-wide git grep post_failure_criteria_results pr-119 -- tests/ returns 16 hits, none with both lists non-empty). The same single-list fixture shape leaves EvaluationResult.post_failure_criteria_results's documented promise — "they do not affect weighted_score, task gating, or suite aggregation" (src/coder_eval/models/results.py:546-549) — unexercised: no test computes calculate_weighted_score / all_criteria_passed / a suite rollup with a non-empty post_failure_criteria_results beside a non-empty canonical list. Add (a) a spill test with a judge at index 0 in both lists, asserting two distinct files exist and both round-trip through load_judge_transcripts; and (b) a test with canonical results scoring 0.0 and post-failure results scoring 1.0 that calls result.calculate_weighted_score(task.success_criteria) and asserts the score stays 0.0 and all_criteria_passed stays False.
6. [Axis 7] New persisted post-failure surfaces (post_failure_criteria_results, evaluation_status, post-failure-judge-.yaml) reach no reader, renderer, report row or documented run-directory contract (src/coder_eval/models/results.py:543) — post_failure_criteria_results: list[CriterionResultUnion] (results.py:543) and evaluation_status: Literal["evaluated", "not_evaluated"] (results.py:85) are new fields on the cross-repo task.json contract, and spill_judge_transcripts now writes a new artifact filename family — result_groups = (("judge", result.success_criteria_results), ("post-failure-judge", result.post_failure_criteria_results)) (judge_persistence.py:141-144) producing post-failure-judge-<idx>.yaml siblings. None of that reaches the documented surfaces: docs/REPORT_SCHEMA.md:113-146 ("task.json — EvaluationResult … The authoritative per-replicate record") still lists only success_criteria_results in its Results table and its CriterionResult base-field list (criterion_type, description, score, details, error, pass_threshold, gating) omits evaluation_status; docs/REPORT_SCHEMA.md:31 and :163 still say transcripts spill to judge-0.yaml / judge-N.yaml only, as do docs/TASK_DEFINITION_GUIDE.md:1104 and :1179 ("Persist a JudgeTranscript … to a sibling judge-<idx>.yaml"). A grep of evaluation_status|post_failure_criteria_results across src/, docs/, evalboard/, plugins/ and .claude/ returns hits only in the three files this PR touches — no renderer consumes either field: reports_html.py:1496-1497 renders _render_criteria(result.success_criteria_results or [], ...) and _render_judge_section(result.success_criteria_results or []), reports_junit.py:201 reads data.get("success_criteria_results"), reports.py:870/901/937 and reports_experiment.py:107 likewise. So the evidence this PR exists to preserve — including any post-failure-judge-<idx>.yaml transcript it pays a judge call for (judge_cost_usd now bills them, results.py:978) — is invisible in every generated report and undescribed for the external eval-runner consumer. Update docs/REPORT_SCHEMA.md (both the EvaluationResult table and the two judge-N.yaml mentions) and docs/TASK_DEFINITION_GUIDE.md:1104/1179, and either render the list in reports_html.py's per-task section or state in the field description why it is deliberately write-only.

Nits

1. [Axis 1] _run_evaluation_with_failure_evidence at CC 14 duplicates its 5-line re-raise block and its reason literal (src/coder_eval/orchestrator.py:643) — radon (re-run at HEAD 3d3774b) reports Orchestrator._run_evaluation_with_failure_evidence - C (14) at 635:4 and Orchestrator._evaluate_post_failure_criteria - C (11) at 714:4. Inside the C(14) function two blocks are copy-pasted:

                raise TaskTimeoutError(
                    task_timeout or 0,
                    task_id=self.task.task_id,
                    elapsed_seconds=time.time() - start_time,
                ) from None

appears identically at lines 650-654 and 675-679 (and a near-twin at 515-519 in run()), and the literal "the task_timeout budget was exhausted before post-failure grading could run" appears at lines 648 and 658. Extract a _task_timeout_error(task_timeout, start_time) factory and hoist the two reason strings to module constants; that removes both copies and drops the branch count without changing behaviour.
2. [Axis 2] JudgeCriterionResult.transcript_path field description still documents the pre-rename judge-0.yaml scheme (and calls the sibling a JSON file) (src/coder_eval/models/results.py:229) — PR HEAD lines 225-233:

    transcript_path: str | None = Field(
        default=None,
        description=(
            "Filename of the sibling JSON file holding this result's full transcript "
            "(e.g. ``judge-0.yaml``), relative to the directory containing ``task.json``. "
            ...

The PR renamed the spill scheme from the single literal f"judge-{idx}.yaml" to f"{prefix}-{idx}.yaml" with prefix in ("judge", "post-failure-judge") (evaluation/judge_persistence.py:141-155), and updated every prose surface in that module plus orchestrator.py:776/794 — but the Pydantic field description that this very PR makes ambiguous was left behind. A reader of the model (the schema is the documented source of truth per CLAUDE.md's "Single Source of Truth" principle) is told the only shape is judge-<idx>.yaml, while post-failure judges now write post-failure-judge-0.yaml. The "sibling JSON file" wording is separately wrong — the spill has been YAML since the format change — and the PR's own docstring edits fixed exactly that wording in judge_persistence.py's module docstring.

Fix: change to "Filename of the sibling YAML file holding this result's full transcript (e.g. ``judge-0.yaml`` for a canonical result, ``post-failure-judge-0.yaml`` for a post-failure diagnostic), relative to the directory containing ``task.json``."

Ripple (out of the in-scope file list, but the same rename): src/coder_eval/models/criteria.py:1263 and :1440, docs/TASK_DEFINITION_GUIDE.md:1104/:1179, docs/REPORT_SCHEMA.md:31, and the generated plugins/coder-eval/reference/criteria.md:62/:215 all still say judge-<idx>.yaml (the plugin reference is generated from the criteria model descriptions, so fixing criteria.py + make plugin-reference covers two of those).
3. [Axis 3] task.json transcript-exclusion for the new list is only pinned by a hand-copied dict in a test; a typo in the production key is silent (src/coder_eval/orchestrator.py:935-938) — Production writes:

935:                exclude={
936:                    "success_criteria_results": {"__all__": {"transcript"}},
937:                    "post_failure_criteria_results": {"__all__": {"transcript"}},
938:                },

Pydantic silently ignores an unknown key in exclude (verified: A().model_dump_json(exclude={'nonexistent': {'__all__': {'y'}}}) returns {"x":1} with no error). The only assertion that the new key works is a copy of the same dict inside the test, tests/test_judge_persistence.py:145-151, which cannot detect drift from the production literal — and tests/test_timeout_orchestrator.py::test_turn_timeout_records_post_failure_evidence_without_rescoring reads back task.json with only non-judge criteria, so the post-failure exclusion is never exercised end-to-end. Impact is size only (20-100 KB of inline transcript per row), hence Low. Fix cheaply by hoisting the dict to a module-level constant in evaluation/judge_persistence.py (e.g. TRANSCRIPT_EXCLUDE) that both the orchestrator and the test import, or by adding an orchestrator-level test whose post-failure criterion is a JudgeCriterionResult with a transcript and asserting "raw_verdict" not in (run_dir / "task.json").read_text().
4. [Axis 3] Tautological assertion: the cost test's "without affecting score" guard asserts a value the test itself just set (tests/test_cost_accounting_paths.py:209) — The test is named test_post_failure_judge_cost_rolls_up_without_affecting_score, but the score half asserts nothing:

194:        result.weighted_score = 0.0
...
205:        row = eval_result_to_task_dict(result)
...
209:        assert row["weighted_score"] == 0.0

eval_result_to_task_dict copies the field verbatim ("weighted_score": result.weighted_score, src/coder_eval/reports_experiment.py:139), so line 209 is guaranteed true regardless of post_failure_criteria_results and would keep passing even if the post-failure list did leak into scoring. The cost assertions (lines 207-208) are genuine and worth keeping. Either drop line 209 and rename the test to test_post_failure_judge_cost_rolls_up, or make it real: set result.success_criteria_results to a scoring criterion, add a post-failure result with score=1.0, call result.calculate_weighted_score(criteria), and assert the score reflects only the canonical list.
5. [Axis 4] Basename allowlist in load_judge_transcripts does not reject the literal "..", contradicting the SECURITY rationale this PR rewrote (src/coder_eval/evaluation/judge_persistence.py:210) — The PR rewrote the SECURITY block at lines 198-209 to claim the allowlist is what refuses tampered paths: # ``transcript_path: '/etc/passwd'`` or ``../../secrets`` is refused at the / # door rather than relying on ``is_relative_to`` to catch it after a join. The guard it points at is line 210: if PurePosixPath(path).name != path or PureWindowsPath(path).name != path:. On the project's own interpreter (pyproject.toml:7 requires-python = ">=3.13") I measured PurePosixPath('..').name == '..' and PureWindowsPath('..').name == '..' (python 3.13.11), so a tampered transcript_path: '..' passes the door check; the reserved-device check at line 219 also passes it ('..'.split('.',1)[0].upper() == ''). It is stopped only by the containment check at line 232, if not resolved_sibling.is_relative_to(resolved_root): — precisely the fallback the comment says it is not relying on. No file is actually read today (the parent dir is not contained, and is_file() at line 240 would fail on a directory anyway), so this is a defense-in-depth/rationale defect rather than a live traversal; it becomes a real one only if a future refactor trims line 232 on the strength of this comment. Fix: make the door check explicit about the dot segments, e.g. if path in {'.', '..'} or PurePosixPath(path).name != path or PureWindowsPath(path).name != path:, and add a transcript_path = '..' case to the traversal tests in tests/test_judge_persistence.py (which today cover ../../etc/passwd and subdir/judge-0.yaml but not the bare dot segment, and cover only success_criteria_results, not the new post_failure_criteria_results list this PR routes through the same loop at line 187). CVSS:3.1/AV:L/AC:H/PR:H/UI:R/S:U/C:L/I:N/A:N
6. [Axis 6] Degraded-recovery reason recorded in task.json names only the exception class, not what failed (src/coder_eval/orchestrator.py:685) — orchestrator.py:684-686 records f"post-failure grading could not complete ({type(recovery_error).__name__})" on every criterion, so a reader of task.json sees Not evaluated after terminal agent failure: post-failure grading could not complete (FileNotFoundError). with no path, no criterion and no message — while the actionable detail goes only to the log via exc_info=True at line 690. Include the exception message (truncated), e.g. f"post-failure grading could not complete ({type(recovery_error).__name__}: {str(recovery_error)[:200]})". This branch is also completely untested (coverage: 683-687 unexercised); a test that makes check_all_async raise a plain RuntimeError and asserts the recorded reason plus that the ORIGINAL terminal error still propagates would pin both this and finding #1's contract.

What's Missing

Parallel paths:

• 🟡 validate_run_limits is wired into only two of the three seams its sibling validate_early_stop runs at — cli/plan_command.py:140 and Orchestrator._setup (orchestrator.py:1141) — but NOT orchestration/experiment.py:674 (resolve_all_tasks), the run path's own resolution seam and the only one that has applied layer-5 -D overrides. Consequences: on coder-eval run the warning never reaches the console (only the per-task log, after spend has started), and plan evaluates layers 1-4 only, so -D run_limits.turn_timeout=900 silences it in the run but not in the preview. Wire it at resolve_all_tasks for the same pre-spend visibility — after fixing the comparison direction, since wiring the current rule into a third site multiplies the false warning. (trigger: src/coder_eval/orchestration/run_limits.py)
• 🔵 The judge-<idx>.yaml → <prefix>-<idx>.yaml rename updated evaluation/judge_persistence.py but none of the other places that declare the scheme: models/results.py:229, models/criteria.py:1255 and :1440, and therefore the CE033-generated plugins/coder-eval/reference/criteria.md:62/:215 (fix the model descriptions then make plugin-reference), plus docs/TASK_DEFINITION_GUIDE.md:1102/:1177. All still say judge-<idx>.yaml is the only shape a transcript sibling can take. (trigger: src/coder_eval/evaluation/judge_persistence.py) (restates: Axis 2: JudgeCriterionResult.transcript_path field description still documents the pre-rename judge-0.yaml scheme)

Tests:

• 🟡 No test drives the new post-failure grading path via AgentCrashError, the most common of the three terminal errors the wrapper catches (orchestrator.py:661). Only TurnTimeoutError exercises the grading branch (tests/test_timeout_orchestrator.py:339) and BudgetExceededError only exercises the else-path; the crash arm — the one whose sandbox state differs most (agent process gone, possibly zero turns) — is unasserted end to end. (trigger: tests/test_timeout_orchestrator.py)
• 🟡 The position-reconstruction splice in _evaluate_post_failure_criteria (orchestrator.py:755-766: unavailable_positions + next(checked_iter)) is only tested with the agent-dependent criterion LAST, so a reversed or off-by-one splice would still pass. Add a case where an agent-dependent criterion sits FIRST and between two artifact-only ones and assert each recovered result lands on its own criterion — this is exactly the positional list[i] ⟷ criteria[i] coupling the codebase treats as a known hazard (reports.py:888, calculate_weighted_score's zip(strict=True)). (trigger: src/coder_eval/orchestrator.py)
• 🟡 Every post-failure test injects a MagicMock sandbox and a MagicMock success_checker, so the feature's central premise — that grading runs against the STILL-LIVE sandbox, before _cleanup captures artifacts — is never verified. No test asserts the ordering (_evaluate_post_failure_criteria before _cleanup/capture_to) or that a real SuccessChecker can read a real sandbox file on the crash path. (trigger: tests/test_timeout_orchestrator.py)
• 🔵 The len(checked) != len(runnable) reconciliation guard (orchestrator.py:748-751) has no test, and because it raises inside the recovery try it is swallowed by the generic except Exception as recovery_error handler and silently downgraded to an all-not_evaluated vector — so the invariant it was written to protect can never surface as an error. Test it, or move the check outside the recovery handler. (trigger: src/coder_eval/orchestrator.py)
• 🔵 _not_evaluated_result (orchestrator.py:695) always builds a base CriterionResult, so a placeholder for a classification criterion (skill_triggered, classification_match) loses its ClassificationCriterionResult subtype and a judge criterion's placeholder loses result_kind="judge". No test in tests/test_criterion_result_round_trip.py covers a placeholder standing in for a subclassed criterion, and any future aggregation of the post-failure list (overlay_classification_metrics, spill_judge_transcripts) would silently skip those rows. (trigger: tests/test_criterion_result_round_trip.py)

Downstream consumers:

• 🟡 reports_experiment._cost_complete keys case 2 on final_status is FinalStatus.TIMEOUT; the new watchdog-during-grading window can turn a crash into a TIMEOUT, so cost_complete flips to False and RunSummary.tasks_cost_incomplete / the run.json row's cost_complete flag change for identical agent output. Neither the flag's docstring rationale ("a TIMEOUT row always lost an in-flight turn") nor its consumers were revisited. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 8: Post-failure grading runs under the still-armed task-timeout watchdog)
• 🟡 Widening judge_cost_usd (models/results.py:978) changes a cost formula whose consumers were not revisited: total_cost_usd (results.py:1022) → the run.json row's judge_cost_usd/total_cost_usd (reports_experiment.py:123/180) → suite cost tables and the evalboard cost views. Crashed rows that previously cost only the agent now carry judge spend with no row, badge or note attributing it, and run_limits.max_usd (orchestrator.py:999-1016) prices only turn token_usage so it cannot bound it. (trigger: src/coder_eval/models/results.py) (restates: Axis 8: Post-failure grading re-executes the full criteria suite — paid judges and sandbox-mutating run_command checks)
• 🟡 reports_junit.py:201 builds the CI failure body from success_criteria_results only, so the JUnit XML — the surface a coder-eval GitHub-Action user actually reads after a crashed task — still shows a bare status line while the evidence the PR pays a judge to collect sits unused in post_failure_criteria_results. Same for reports_html._render_criteria/_render_judge_section (1496-1497), and cli/report_command.py:125 now LOADS post-failure transcripts via load_judge_transcripts and then drops them on the floor. (trigger: src/coder_eval/evaluation/judge_persistence.py) (restates: Axis 7: New persisted post-failure surfaces reach no reader, renderer, report row or documented run-directory contract)
• 🟡 docs/TASK_DEFINITION_GUIDE.md:238-244 ships task_timeout: 600 / turn_timeout: 300 as the canonical run_limits example — precisely the shape the new warning fires on — and neither that section nor docs/CI_GATE.md mentions the warning at all, so a user who copies the documented example gets a yellow ⚠ from coder-eval plan with no documented meaning. CE030 cannot catch it: the PR adds behaviour to RunLimits, not a field. (trigger: src/coder_eval/orchestration/run_limits.py) (restates: Axis 6: New validate_run_limits warning fires on the shipped/correct task_timeout > turn_timeout configuration)

Display & mapping dicts:

• 🟡 CriterionResult.evaluation_status gets no entry in any rendering surface, while its immediate neighbour gating — added the same way and whose own field description mandates that "every display surface must render it as informational rather than failed" — is mirrored in all four: reports.py:940, reports_html.py:582/:600, reports_junit.py:220 (the [INFO] branch), and the cross-repo evalboard/lib/runs.ts:126/:2028. A not_evaluated placeholder is score 0.0 + gating=True, so the moment any surface renders these results it will display an ungraded criterion as a hard failure, and the evalboard's typed mapping will drop the field entirely. (trigger: src/coder_eval/models/results.py) (restates: Axis 7: New persisted post-failure surfaces reach no reader, renderer, report row or documented run-directory contract)

Daily/nightly:

• 🟠 The PR changes the production run path and the cross-repo task.json contract but states no blast radius for the nightly suite: every crashed / turn-timed-out / budget-exceeded row now performs an extra full criteria pass (incl. paid llm_judge and sub-agent-spawning agent_judge), each crashed row's tail grows inside task_timeout, and rows can migrate from the error bucket to the failed bucket (FinalStatus.ERROR → TIMEOUT, models/enums.py:37/42), moving tasks_error / tasks_failed / error_share for identical agent output. On a failure-heavy night that is a cost delta and a metric discontinuity with no stated estimate, no opt-out flag, and no note in the PR. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 8: Post-failure grading re-executes the full criteria suite — paid judges and sandbox-mutating run_command checks)
• 🟡 Post-failure grading runs run_command criteria in the live sandbox BEFORE _cleanup captures artifacts/ (orchestrator.py:2444-2453), so on the nightly path those side effects are archived to the run blob and are then visible to any later coder-eval evaluate <task> artifacts/ re-grade and to the evalboard's artifact viewer — the archived workspace of a crashed run is no longer the agent's output alone. The PR does not say this, and no test asserts what artifacts/ contains after a post-failure pass. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 8: Post-failure grading re-executes the full criteria suite — paid judges and sandbox-mutating run_command checks)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] Extend CE030's DOCUMENTED_MODELS registry to the persisted run-record contract. In tests/lint/doc_schema_parity.py, add (EvaluationResult, "docs/REPORT_SCHEMA.md") and (CriterionResult, "docs/REPORT_SCHEMA.md") to DOCUMENTED_MODELS (import CriterionResult from coder_eval.models.results). No new wiring needed — tests/test_custom_lint.py::TestCE030DocSchemaParity already iterates the registry. MEASURED COST: exactly zero pre-existing violations. I ran CE030's own inline-code match against docs/REPORT_SCHEMA.md: of EvaluationResult's 33 fields the only undocumented one is post_failure_criteria_results, and of CriterionResult's 9 the only undocumented one is evaluation_status — i.e. the registry extension is free today and fails exactly on this PR's two new fields. This is the highest-leverage item in the list: task.json is a cross-repo contract (evalboard, coder-eval evaluate, CI artifacts) and CE030 already exists for precisely this class; it simply was never pointed at the output side. Prevents: A7-medium (new persisted post_failure_criteria_results / evaluation_status reach no documented surface — the PR touches zero files under docs/); also the doc half of A2-low.
• [ce-lint] Extend CE031's CONSUMED_MODELS to CriterionResult. In tests/lint/dead_config_fields.py add CriterionResult to CONSUMED_MODELS and one EXEMPT entry: "CriterionResult": {"result_kind": "pydantic discriminator — consumed by the CriterionResultUnion tag, never by attribute read"}. MEASURED: running dead_config_fields(CriterionResult, consumed_attr_names(Path('src')), {}) at PR HEAD returns exactly ['result_kind', 'evaluation_status'], and EvaluationResult returns []. So the extension costs one exemption and fires on precisely the field this review found is written (orchestrator.py:701, _not_evaluated_result) and read by nothing anywhere in the tree. Note this widens CE031's stated charter from "config a user sets" to "a field somebody has to consume"; the docstring must be amended to say the registry now covers one output model, and why (a persisted verdict field nobody reads is the same defect from the other end). Prevents: A7-medium (evaluation_status is set on every criterion result and consulted by no renderer, no report, no consumer).
• [ce-lint] New CE044 — a resolution-time advisory must be silent on the shipped task tree. New @pytest.mark.lint class TestCE044ShippedTreeAdvisorySilence in tests/test_custom_lint.py (doc/YAML-surface rule, not a BaseRule — it loads YAML and runs the 5-layer merge, exactly CE034's shape). Parametrize over sorted((ROOT/'tasks').rglob('*.yaml')) excluding metadata.yaml and tasks/samples, resolve each through load_experiment(ROOT/'experiments/default.yaml') + resolve_task_for_variant(...), then assert validate_run_limits(resolved) == (). Statement of the pattern it forbids: a non-blocking warning predicate may not fire on the repository's own shipped, correct configuration. MEASURED at PR HEAD: 44 of 46 shipped tasks trip the new warning (the two silent ones — tasks/smoke_task_timeout.yaml 30/300 and tasks/token_check.yaml 120/300 — are exactly the degenerate task_timeout < turn_timeout shape the rule should have flagged), so the sensor is inverted with respect to its own tree. Register the predicate list as a module constant (ADVISORY_PREDICATES = [validate_run_limits]) so a future second advisory is covered by construction. Prevents: A6-high / A1-high / A5-high / A7-high / A8-high (the merged five-axis finding: validate_run_limits warns on the shipped experiments/default.yaml 600/300 config for 44/46 tasks, once per task × variant, and is silent on the genuinely broken inverse).
• [ce-lint] New CE045 — no except handler for a project-defined exception that only a sibling handler of the same try raises. A BaseRule in tests/lint/rules/ce045_unreachable_sibling_handler.py, wired into ALL_RULES in tests/lint/runner.py (AST-shape check over one file at a time, same class as CE037/CE040/CE041/CE042). For each ast.Try: collect direct raise types in the body and in the handlers; flag any handler catching type T where T is raised in a sibling handler and nowhere in the body. Restrict T to exceptions defined under src/coder_eval/errors/ — a builtin (ValueError, RuntimeError) can arrive from any called function, a project exception's raise sites are enumerable. MEASURED: with the builtin restriction the rule fires on exactly ONE site in src/ — orchestrator.py:656 (except TaskTimeoutError, whose only in-scope raise sites are the sibling handler at 650 and the nested handler at 675). Without the restriction it fires on 3 (adding orchestrator.py:2385 and models/judge.py:49, both builtins raised by called code), which is why the narrowing is load-bearing. Docstring must state the boundary: it sees only direct raise statements, so a project exception raised by a helper called from the body is a false positive resolvable with # noqa: CE045 plus a reason. Prevents: A1-medium (unreachable except TaskTimeoutError at orchestrator.py:656, which also carries a false attribution message byte-identical to the watchdog branch's) and, by deleting the branch, half of A1-low (the duplicated reason literal at 648/658).
• [ce-lint] New CE046 — no string-literal getattr for a name that is a declared model field. A BaseRule in tests/lint/rules/ce046_no_model_field_getattr.py + ALL_RULES. Forbids getattr(x, "<literal>", ...) where <literal> is a field name on any BaseModel reachable from coder_eval.models (runtime introspection over model_fields, CE038's technique, not a hardcoded name list), scoped to src/coder_eval/models/ and src/coder_eval/evaluation/. The required spelling is isinstance(cr, JudgeCriterionResult) and cr.token_usage — the in-tree precedent is evaluation/judge_persistence.py:147. MEASURED adoption cost in that scope: 6 sites (models/results.py:276,979, models/tasks.py:598,616, evaluation/judge_persistence.py:190,196) — all of them the same union-probing shape, i.e. real conversions, not noqa fodder. Widening to reports*.py adds 11 more (reports.py:903-904, reports_html.py:283,436,438,526-529), several of which are duck-typed SDK objects; keep them out of the initial scope and say so in the docstring. RECORDED BOUNDARY, measured not assumed: pyright cannot reach this. I ran the repo-pinned pyright with typeCheckingMode: "strict" over src/coder_eval/models/results.py — 0 errors, because getattr's typeshed return is a declared Any (not an inferred Unknown), so neither reportUnknownMemberType nor any standard-mode setting fires on u.total_cost_usd. There is no ruff/pyright flag for this; a CE rule is the only static route. Prevents: A2-medium (judge_cost_usd at models/results.py:979 — the line this PR widened to both criterion lists — reads token_usage through getattr, erasing CriterionResultUnion to Any; a rename silently returns None for every run and an extra="allow" basic record raises AttributeError at report time).
• [ce-lint] New CE047 — the escalating-exception tuple may be spelled in exactly one place. A BaseRule in tests/lint/rules/ce047_escalating_exceptions_single_declaration.py + ALL_RULES, on the CE037/CE040/CE042 precedent ("one declaration of a rule whose second copy agrees on ordinary input and diverges exactly where it matters"). Matches any tuple literal — in an except (...) clause or an assignment — containing both JudgeInfrastructureError and CheckerMisuseError outside src/coder_eval/criteria/base.py. Requires promoting criteria/base.py:71::_ESCALATING_EXCEPTIONS to a public ESCALATING_EXCEPTIONS (it is private today, so sharing it is part of the fix) and rewriting orchestrator.py:681 to except ESCALATING_EXCEPTIONS:. The divergence this prevents is concrete: a third escalating error added to criteria/base.py would be captured-and-scored by the decorator but silently swallowed-and-re-raised by the orchestrator's hand-copied pair, or vice versa. Prevents: The second-copy half of A6-high / A8-medium (orchestrator.py:681 re-spells (JudgeInfrastructureError, CheckerMisuseError), already declared at criteria/base.py:71).
• [ce-lint] New CE048 — a reader of success_criteria_results must decide about its sibling list. A whole-tree @pytest.mark.lint class (CE031's shape: scan every .py under src/ for the attribute name). Any module that reads success_criteria_results must also read post_failure_criteria_results, or appear in an EXEMPT map that stores the REASON (CE038's convention), plus a companion test that fails when an exemption names a module that no longer reads either field. MEASURED seeding cost: 9 modules read the canonical list without the sibling today (models/results.py, orchestrator.py, reports.py, reports_html.py, reports_junit.py, reports_experiment.py, orchestration/experiment.py, evaluation/judge_persistence.py — already compliant — and cli/evaluate_command.py), so adoption is a one-time pass writing 8 one-line reasons. That is the point: each reason is the decision this PR never made anywhere. A cheaper variant if 8 exemptions is too much: scope the rule to the render/serialize surfaces (reports*.py), which is 4 modules and is where the invisible-evidence defect actually lands. Prevents: A7-medium (the evidence this PR pays judge calls for is rendered by nothing: reports_html.py:1496-1497, reports_junit.py:201, reports.py:870/901/937, reports_experiment.py:107 all read the canonical list only).
• [ce-lint] New CE049 — a pydantic exclude= spec must be validated against the model's fields. Two halves. (1) Hoist orchestrator.py:935-938's literal into a module constant in evaluation/judge_persistence.py (e.g. TASK_JSON_TRANSCRIPT_EXCLUDE) that the orchestrator and the tests both import, so the test can no longer assert against a hand-copied twin. (2) A @pytest.mark.lint test using runtime introspection (CE038's technique, since the question is about resolved field types): every top-level key of that constant must be a field of EvaluationResult, and every nested key a field of CriterionResult. Pydantic silently ignores an unknown exclude key — verified: A().model_dump_json(exclude={'nonexistent': {'__all__': {'y'}}}) returns {"x":1} with no error and no log — so a rename or typo degrades to "transcripts inlined into every task.json" with nothing raising. An AST-only rule cannot do this (it cannot resolve which model is being dumped), hence the constant-plus-introspection shape. Prevents: A3-low (the new post_failure_criteria_results exclusion key is pinned only by a copy of the same dict inside tests/test_judge_persistence.py:145-151, which cannot detect drift from the production literal).
• [ce-lint] New CE050 — one declaration of the judge-transcript filename scheme, with a derived doc surface. Replace the inline tuple at evaluation/judge_persistence.py:141-144 with a module constant JUDGE_TRANSCRIPT_PREFIXES: tuple[str, ...] = ("judge", "post-failure-judge") and a judge_transcript_name(prefix, idx) helper (exact path_utils.replicate_subdir_name / CE042 precedent). Then a @pytest.mark.lint derived-surface sensor (CE033/CE026 shape) asserting every surface that names the scheme mentions every prefix: models/results.py::JudgeCriterionResult.transcript_path's description (line 229), models/criteria.py:1263/:1440, docs/REPORT_SCHEMA.md:31/:176, docs/TASK_DEFINITION_GUIDE.md:1102/:1177, and the generated plugins/coder-eval/reference/criteria.md. The surface list lives beside the constant so a new prefix forces the doc decision rather than silently existing. Fold in the YAML/JSON wording fix while there — the field description still calls the sibling a "JSON file". Prevents: A2-low (the field description and five doc surfaces still document only judge-<idx>.yaml after this PR added the post-failure-judge-<idx>.yaml family).
• [ruff] Enable C901 (mccabe) in [tool.ruff.lint] select, with [tool.ruff.lint.mccabe] max-complexity = 12. The repo already gates function SIZE (PLR0915 max-statements=80, PLR0912 max-branches=25) under a stated "existing offenders carry a visible # noqa debt marker" regime; branch-complexity is the missing third axis and is what the new funnel blew past. MEASURED cost at HEAD: ruff check --select C901 --config lint.mccabe.max-complexity=12 src/ = 28 offenders (16 at 13-14, 13 at 15). The reviewed function Orchestrator._run_evaluation_with_failure_evidence is CC 14, so a threshold of 12 or 13 is required to catch it; 12 is the conventional default and costs a one-time 28-marker sweep (the tree currently carries exactly ONE # noqa: PLR09 marker, so the debt is genuinely new, not amnesty for an existing mess). If 28 markers is judged too much churn, land at 15 first (13 markers) and ratchet down — but record that 15 does NOT catch this PR's function. Prevents: A1-low (_run_evaluation_with_failure_evidence at CC 14 with two copy-pasted 5-line re-raise blocks and a duplicated reason literal; _evaluate_post_failure_criteria at CC 11 sits just under).
• [bandit-codeql] Add a CodeQL py/path-injection configuration with EvaluationResult deserialization as a custom taint source. bandit cannot see this shape (no subprocess/eval/assert marker; the sink is an ordinary Path join), and it is not currently covered. The concrete wiring: a .github/codeql/coder-eval-python.qll extension declaring EvaluationResult.model_validate_json / model_validate results as remote-ish sources — task.json travels across trust boundaries (CI artifacts, shared eval bundles), which is what the SECURITY block at judge_persistence.py:198-209 itself asserts — and Path.__truediv__ → read_text/is_file as sinks, with is_relative_to recognised as the barrier. That configuration reports the flow that today is sanitized ONLY by the post-join containment check at line 232, which is exactly the reviewer's point: the basename allowlist at line 210 that the comment credits does not stop the flow. Cheaper complement if CodeQL is not wanted: fold path in {".", ".."} into the door check and add a CE-style assertion that any "basename allowlist" predicate rejects both dot segments (PurePosixPath('..').name == '..' on py3.13, verified). Prevents: A4-low (transcript_path: '..' passes the basename allowlist and the reserved-device check, contradicting the SECURITY rationale this PR rewrote; a future refactor trimming line 232 on the strength of that comment turns it into a live traversal).

Harness improvements (not statically reachable):

• Patch-coverage gate in make verify and .github/workflows/pr-checks.yml. Add diff-cover (or pytest-cov + a changed-files coverage assertion) against the merge base, e.g. uv run diff-cover coverage.xml --compare-branch=origin/main --fail-under=90, alongside the existing global --cov-fail-under=80. MOTIVATION IS MEASURED, NOT THEORETICAL: the full suite at PR HEAD (4396 passed) reports orchestrator.py at 89.78% — comfortably over the global gate — while the ENTIRE new funnel is uncovered: missing lines 655, 657-660, 667, 671-680, 683-687, i.e. the budget short-circuit, the TaskTimeoutError re-record, the non-watchdog CancelledError re-raise, the cancel-during-recovery arm and the generic recovery-failure arm. A global percentage gate structurally cannot see a fully-uncovered new block in a large well-covered file; a patch gate can. Why not static: Coverage is a property of executing the test suite against the code; no AST or type analysis can tell whether a branch is reached at runtime. Prevents: A3-high (BudgetExceededError short-circuit and every recovery handler uncovered), A6-low (683-687 unexercised).
• Mutation spot-check restricted to the diff, as a manual make mutate-diff target run on error-handling changes (and optionally a non-blocking PR job). Wire mutmut/cosmic-ray (or a 20-line harness that flips guard conditions in changed functions) over functions touched by the diff, reporting survivors. MEASURED JUSTIFICATION FROM THIS REVIEW'S OWN VERIFY PASS: dropping isinstance(terminal_error, BudgetExceededError) from the guard at orchestrator.py:662-666 leaves the full suite GREEN (4396 passed, 0 failed) — a surviving mutant on a guard whose failure mode is re-running paid judges on a run that just blew its budget. Inverting == to != in the same guard is caught (4 failures), so the suite is partially pinning the block and a coverage number alone would not have distinguished the two. The same technique kills tautological assertions: tests/test_cost_accounting_paths.py:209 asserts a weighted_score the test itself set, and no mutation of production code can ever fail it. Why not static: Requires building N mutated trees and running the suite against each; the question is whether the tests DISCRIMINATE, which no static analysis can answer. Prevents: A3-high (the isinstance-drop mutant survives), A3-low (tautological weighted_score assertion in the cost test).
• A terminal-error classification determinism test fixture. Add to tests/test_timeout_orchestrator.py a fixture that drives a real ThreadedWatchdog with a small task_timeout and an injected slow SuccessChecker.check_all_async (e.g. 2× the remaining budget), asserting that a run whose _evaluation_loop raised AgentCrashError lands FinalStatus.ERROR regardless of how long post-failure grading takes — and, symmetrically, that its post_failure_criteria_results content does not depend on wall clock. The bucket split is load-bearing for the harness's own metrics: models/enums.py:37,42 put ERROR -> "error" and TIMEOUT -> "failed" in different report buckets, so tasks_error / tasks_failed / error_share currently move for identical agent output depending on judge latency. Pair with the fix (grade outside the task-timeout watchdog under its own fixed deadline that never rewrites the terminal error). Why not static: The defect is a race between a live threading.Timer and the duration of criteria execution — it needs real threads and a wall clock; nothing in the AST distinguishes "await inside a watchdog scope" that is safe from one that is not. Prevents: A8-high (post-failure grading runs under the still-armed watchdog, so a cancel during grading rewrites final_status to TIMEOUT and discards the original terminal error), A6-high (the BudgetExceededError→ERROR status regression on the escalation path).
• A spend-accounting assertion fixture for the crash path: count criterion invocations per run. Add a test-only SuccessChecker spy (or an EvaluationResult-level counter) asserting that (a) no criterion is graded twice in a single run, and (b) no llm_judge / agent_judge criterion is invoked at all on a terminal-error path when success_criteria_results is already full-length. Today the dialog path with check_criteria: every_turn re-grades the entire judge suite after an AgentCrashError on turn N>1 even though the canonical vector is complete (the short-circuit at orchestrator.py:662-666 covers only BudgetExceededError), and this spend is invisible to run_limits.max_usd (_check_run_limits at 999-1016 prices only turn token_usage) while still landing in the row's total_cost_usd via judge_cost_usd. Ship the opt-out alongside it — a run_limits-level kill switch on the stop_early: false precedent — and have the fixture assert the switch actually suppresses the grading call. Why not static: "How many paid API calls did this run make, and were any of them redundant" is a runtime accounting property; a static rule cannot know that success_criteria_results is already full-length when the handler is entered. Prevents: A8-high (unbounded, unaccounted, un-opt-out-able judge spend on crashed runs; the already-graded short-circuit covering only one of three error types).
• A golden run-directory contract fixture rendered through every surface. Build one fixture run dir in which BOTH criterion lists are non-empty with a JudgeCriterionResult at index 0 in each (plus a transcript), then: assert the spill writes two distinct files (judge-0.yaml, post-failure-judge-0.yaml) and both round-trip through load_judge_transcripts; render it through reports.py, reports_html.py, reports_junit.py and reports_experiment.py and snapshot the output; and add the same fixture to the evalboard's runs.ts parsing tests. A repo-wide grep shows 14 test-side occurrences of post_failure_criteria_results across 5 files and NONE of them populates both lists at once — tests/test_judge_persistence.py:137-143 uses criteria=[], and tests/test_timeout_orchestrator.py:387 explicitly asserts the canonical list is empty — so the exact collision the f"{prefix}-{idx}.yaml" rename exists to prevent is never constructed. The snapshot half is what makes "no renderer consumes the new field" visible as a diff instead of an omission. Why not static: Needs the full serialize→spill→reload→render pipeline (and the TypeScript consumer) executed end to end; a static rule can check that a field is documented or read somewhere, not that the rendered artifact actually contains it. Prevents: A3-medium (filename-collision fix unasserted; score non-interference unexercised), A7-medium (new fields and the new artifact family reach no rendered surface).
• One shared traversal-input corpus for transcript_path, applied to both criterion lists. Replace the two ad-hoc traversal cases in tests/test_judge_persistence.py with a single parametrized constant covering "..", ".", "../../etc/passwd", "subdir/judge-0.yaml", "/etc/passwd", "C:\\Windows\\win.ini", "CON", an empty string and a symlinked sibling — and run it over success_criteria_results AND post_failure_criteria_results, since this PR routes the new list through the same loop at judge_persistence.py:187. Today the bare dot segments are untested and the new list is untested for traversal at all. Why not static: A path-traversal corpus asserts what a hardening predicate REJECTS at runtime; a static rule can flag a missing dot-segment check (see the bandit-codeql item) but cannot demonstrate that the composed door-check + containment-check pair actually refuses each shape. Prevents: A4-low (basename allowlist accepts the literal "..", and the new list's traversal behaviour is unasserted).

Top 5 Priority Actions

1. Move post-failure grading out of the still-armed task-timeout watchdog scope (src/coder_eval/orchestrator.py:669-679, watchdog with at :500-505) and give it its own short independent deadline, because today a slow judge or run_command during grading converts a deterministic AgentCrashError/TurnTimeoutError (FinalStatus.ERROR, bucket "error") into TaskTimeoutError (FinalStatus.TIMEOUT, bucket "failed"), so tasks_error/tasks_failed/error_share flip run-to-run on identical agent output.
2. Stop the diagnostic step from becoming the reported cause of failure at src/coder_eval/orchestrator.py:681-682, where the bare raise on JudgeInfrastructureError/CheckerMisuseError discards terminal_error and leaves post_failure_criteria_results empty — reproduced to downgrade a real USD-budget breach from FinalStatus.COST_BUDGET_EXCEEDED to FinalStatus.ERROR with error_message "judge unavailable" — so record the not-evaluated vector naming the escalating error and re-raise while preserving the terminal classification.
3. Bound and control the re-grade at src/coder_eval/orchestrator.py:661-667: extend the already-graded short-circuit to AgentCrashError/TurnTimeoutError (simulation with check_criteria: every_turn re-bills the entire judge suite on a complete vector), add an opt-out plus max_usd accounting for post-failure judge spend that today is invisible to _check_run_limits (:999-1016) but lands in total_cost_usd, and exclude sandbox-mutating run_command criteria whose writes are captured into run_dir/artifacts and will be seen by a later coder-eval evaluate re-grade (criteria/run_command.py:68).
4. Invert the new cross-field check at src/coder_eval/orchestration/run_limits.py:28 to warn on task_timeout < turn_timeout (the shape that makes turn_timeout dead config and forfeits the very post-failure path this PR adds) rather than on task_timeout > turn_timeout, which is the correct shipped configuration from experiments/default.yaml:29,31 and fires on 44 of 46 tasks per plan/run, updating tests/test_run_limits_models.py::TestRunLimitsCrossFieldWarnings which pins the current direction.
5. Close the dead-output and coverage loop: render or explicitly document post_failure_criteria_results / evaluation_status / post-failure-judge-<idx>.yaml (src/coder_eval/models/results.py:543 and :85; docs/REPORT_SCHEMA.md:129/:31/:176, docs/TASK_DEFINITION_GUIDE.md:1102/:1177 all untouched by this PR), narrow the getattr(cr, "token_usage") in judge_cost_usd to an isinstance(cr, JudgeCriterionResult) check (src/coder_eval/models/results.py:979), and add the two missing tests — a BudgetExceededError raised with a full-length canonical vector (kills the surviving isinstance-drop mutant on line 667) and a spill with a judge at index 0 in both lists (pins the filename-collision fix).

---

Stats: 0 🔴 · 5 🟠 · 6 🟡 · 6 🔵 across 8 axes reviewed.