Skip to content

fix: recover grading failures without replaying agents - #248

Merged
cosminacho merged 12 commits into
mainfrom
codex/grading-error-recovery
Nov 5, 2026
Merged

cosminacho merged 12 commits into
mainfrom
codex/grading-error-recovery

Conversation

@cosminacho

@cosminacho cosminacho commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

A grader failure can hide an execution limit or replace a previously measured result during detached grading. This change records execution limits before grading, keeps execution failures as the primary outcome, and distinguishes completed grading failures from interrupted execution.

Grading failures use one typed boundary. Results retain error_phase and error_history; a failed detached grade preserves the immediate prior verdict, answers, usage and judge artifacts, then appends one incident with one authoritative write. When evaluating an external work directory, recovery does not write run records into it. Completed runs whose grading failed remain eligible for grading without replaying the agent. The original weighted-score behavior is preserved.

The inherited OAuthlib minimum and lock now require 4.0.1 for CVE-2026-49272.

Validation:

  • 408 affected execution/grading tests and 741 architecture checks passed locally.
  • Independent controls covered settled budgets at checker entry, single-write failure handling, real detached-grading watchdogs, and container failure versus successful repair. Container transport was mocked; no live Docker claim.
  • Changed-file Ruff/format checks, documentation prose checks and production type checks passed.

The targeted dependency update preserves all other locked packages. A hash-checked OAuthlib 4.0.1 installation passed a focused package audit; full automatic CI remains the publication check.

Cosmin Maria and others added 3 commits November 4, 2026 19:19
osv-scanner fails the Quality Gate on pyasn1 0.6.4 (GHSA-2qm8-x737-gmw5,
GHSA-9m75-vc3g-4x39, GHSA-fm9c-5rcf-2q5p, GHSA-mm57-4v3p-rchx), on main as
well. 0.6.5 fixes all four. Lockfile only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit ffa1713)
@cosminacho
cosminacho marked this pull request as ready for review November 5, 2026 09:56
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Claude finished @cosminacho's task in 1m 11s —— View job


Code Review in Progress

Todo List:

  • Read .review standards in .github/code_review.md
  • Read CLAUDE.md for project conventions
  • Get and analyze the diff
  • Read each changed file with full surrounding code
  • Perform cross-file consistency analysis
    from code-review.md
    design-level scrutuerituny
    Working on comprehensive PR review...

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fix: recover grading failures without replaying agents (#248)

PR #248 by @cosminacho · codex/grading-error-recovery → main · OPEN · reviewed against 1ab2acd

Scope: pr:248 · branch codex/grading-error-recovery (PR #248 head; local ref pr-248) · 1ab2acd · 2026-11-05T11:35Z · workflow variant

Change class: complex — changes grading/resume control flow, FinalStatus preservation and the persisted task.json result schema

PR 248 has a sound goal and its type, security and error-handling quality is good (9.4 to 9.9), but it must not merge as is: a grading crash is now repairable before the execution-phase work (budget checks, remaining dialog turns) has finished, so --resume or evaluate can turn a TOKEN_BUDGET_EXCEEDED row into SUCCESS for the same agent output, and a failed regrade of a row with an execution failure (a fact --resume must not overwrite) writes an unmeasured weighted_score of 0.0 that --resume never repairs.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 6.8 / 10 0 2 2 2 Grading-origin ERROR predicate inlined at ~7 sites with no named property; overlapping public execution-fact predicates
2. Type Safety 9.4 / 10 0 0 1 1 New persisted public model GradingErrorInfo adds a dict[str, Any] escape hatch, although a typed ErrorContext already exists
3. Test Health 6.3 / 10 0 2 3 2 Grading failure under evaluate --format harbor writes task.json into the user's WORK_DIR (restore_pre_grade_record lost its run-dir guard)
4. Security 9.9 / 10 0 0 0 1 New CLI error prints put grading_error.message into Rich markup without escape(); a message containing '[/...' raises MarkupError before the prior verdict is restored
5. Architecture & Design 9.4 / 10 0 0 1 1 One grading incident is stored twice (error_message/error_details/error_log_tail and grading_error), and the two copies do not agree
6. Error Handling & Resilience 9.9 / 10 0 0 0 1 Failed-regrade paths in run_command ignore the restore result and print inaccurate operator messages
7. API Surface & Maintainability 9.8 / 10 0 0 0 2 New public persisted model GradingErrorInfo has no field descriptions
8. Evaluation Harness Quality 5.5 / 10 1 1 1 0 Making grading ERROR repairable skips execution-phase work the crash cut short, so --resume/evaluate can turn a budget stop or an unfinished dialog into SUCCESS

Overall Score: 8.4 / 10 · Weakest Axis: Evaluation Harness Quality at 5.5 / 10
Totals: 🔴 1 · 🟠 5 · 🟡 8 · 🔵 10 across 8 axes.

Blockers

  1. [Axis 1] Grading-origin ERROR predicate inlined at ~7 sites with no named property; overlapping public execution-fact predicates (src/coder_eval/orchestrator.py:682) — The same two-term test X.final_status is FinalStatus.ERROR and X.error_phase == "grading" is written by hand at 7 places: orchestrator.py:682 (elif prior.final_status is FinalStatus.ERROR and prior.error_phase == "grading":), orchestrator.py:691 (the same test on the next if, which repeats the elif above it), models/results.py:731 (inside has_execution_failure), orchestration/batch.py:414 (if result.final_status is FinalStatus.ERROR and result.error_phase == "grading":), cli/run_command.py:913 and :920, and cli/evaluate_command.py:527. This matches the Axis-1 High anchor (logic duplicated across 4+ sites). It is also the classifier that decides whether a row can be repaired, so if one copy drifts, one command treats a row as recoverable and another treats it as an execution fact. Fix: add one EvaluationResult.is_grading_error property. Make has_execution_failure return self.final_status.is_execution_fact and not self.is_grading_error, and call the property at all six other sites. In _seed_from_prior_result, compute it once into a local variable rather than evaluating it twice in a row (lines 682 and 691).
  2. [Axis 1] Unreachable grading-phase branches in the evaluate CLI and the legacy/backup-read restore paths (src/coder_eval/cli/evaluate_command.py:541) — Members of this finding: (1) evaluate_command.py:541-543, if result.error_phase == "grading": restore_pre_grade_record(...); console.print("[yellow]⚠[/] Grading errored; the prior verdict is preserved."). This code runs only after line 524 (if result.grading_error is not None: ... raise typer.Exit(1)) has not matched. The orchestrator never writes error_phase="grading" without also setting grading_error (orchestrator.py:934-941 and :950), so this branch cannot run. Its comment ("Restore explicit grader faults only...") makes it look like a required path. (2) run_command.py:919-921, the clause or (result.final_status is FinalStatus.ERROR and result.error_phase == "grading"), is redundant for the same reason. (3) regrade.py:590/594/596-597, the prior_result is None path in restore_pre_grade_record, which reads task.execute.json (backup.read_text(...), then EvaluationResult.model_validate_json(text)). All three production callers (run_command.py:926, evaluate_command.py:532 and :542) pass a non-None prior_result. Only tests reach this path (test_detached_grading_boundaries.py:456/467/476, test_regrade.py:1260). A reader still sees a function named restore_pre_grade_record with an Optional prior_result and concludes that crash recovery falls back to the pre-grade backup. That is now false. This is close to the Axis-1 Critical anchor (dead code on a live path that misleads readers). I filed it as High because the branches cannot change behavior today. Fix: delete (1) and (2). Make prior_result and grading_result required in restore_pre_grade_record, delete the backup-read branch, and rename the function (for example restore_prior_result) so the name matches its new contract. Then update or delete the tests that pin the removed path.
  3. [Axis 3] Grading failure under evaluate --format harbor writes task.json into the user's WORK_DIR (restore_pre_grade_record lost its run-dir guard) (src/coder_eval/cli/evaluate_command.py:532) — Line 532 is restore_pre_grade_record(target.target, prior_result=prior, grading_result=result) and line 542 is the same call. Neither has an EvaluateMode.RUN_DIR guard. The success path at line 595 has one (if prior is not None and target.mode is EvaluateMode.RUN_DIR:), and its comment at lines 591-593 says the harbor synthetic prior 'is not a run directory'. On main, restore_pre_grade_record(target.target) returned False when no task.execute.json backup existed. With this PR, regrade.py:590 skips the backup check whenever prior_result is passed (prior_result is None and (...)), so the function now writes. I confirmed this with a probe in a scratch copy: I patched SuccessChecker.check_all_async to raise, then ran evaluate task.yaml workdir --format harbor --trajectory t.json. It exited 1, printed 'Grading errored; prior verdict restored with grading provenance.', and left WORKDIR FILES ['task.json', 'test.txt']. The planted task.json makes the next evaluate of that directory try the RUN_DIR detection (evaluate_command.py:141-153), and the graded workspace now holds a file the agent did not write. No test exercises this branch: coverage at PR HEAD shows 526->534 and 528->530 partial, and tests/test_evaluate_format_harbor.py has no grading-failure case. Fix: guard both restore calls (532, 542) with target.mode is EvaluateMode.RUN_DIR. Add a harbor test that makes the checker raise and asserts not (work_dir / 'task.json').exists() and exit 1. Cross-axis: 6 (error handling), 8 (harness).
  4. [Axis 3] EvaluationResult.has_execution_failure has no direct test, and a mutant that ignores the status in its grading exemption survives the suite. The changed input never occurs in practice (src/coder_eval/models/results.py:728) — Every caller now uses this property instead of FinalStatus.is_execution_fact (orchestrator.py:467/632/942/947, evaluate_command.py:522/602, run_command.py:912). It decides whether a grading pass may overwrite a recorded status. The body is return self.final_status.is_execution_fact and not (self.final_status is FinalStatus.ERROR and self.error_phase == "grading"). I replaced it with self.final_status.is_execution_fact and self.error_phase != "grading", which lets a TIMEOUT / COST_BUDGET_EXCEEDED / TOKEN_BUDGET_EXCEEDED / BUILD_FAILED row tagged error_phase: grading be regraded into SUCCESS. All 361 targeted tests still passed. The CLI tests cover only ERROR/grading, ERROR/execution, ERROR/None and TIMEOUT/execution. tests/test_seed_from_prior_result.py:286 still pins only the context-free enum. Fix: add a parametrized table test over every FinalStatus x {"execution", "grading", None}. It must assert that only (ERROR, "grading") returns False among the execution-fact statuses, that TIMEOUT/BUILD_FAILED/budget with phase "grading" stay True, and that NOT_GRADED/SUCCESS/FAILURE/MAX_TURNS_EXHAUSTED stay False. This is the test that trips the gate.
  5. [Axis 8] Making grading ERROR repairable skips execution-phase work the crash cut short, so --resume/evaluate can turn a budget stop or an unfinished dialog into SUCCESS (src/coder_eval/orchestration/batch.py:414) — The new _owes_a_grade arm if result.final_status is FinalStatus.ERROR and result.error_phase == "grading": return True (batch.py:414-415) sends every fresh-run grading crash to the detached re-grade. That re-grade uses the evaluate-only path (agent None, orchestrator.py:2190-2221), which runs the criteria once and never calls _check_run_limits. But on a fresh run a criteria crash happens BEFORE execution-phase work that decides the status: (a) the post-criteria budget check self._check_run_limits(iteration=iteration) at orchestrator.py:2314 is skipped; (b) in dialog mode, criteria_results = await self._run_dialog_criteria_check(judge_usage_accum) (orchestrator.py:2628, when check_every_turn) comes before the budget gate at 2636 and before the remaining simulated turns, so a crash cuts the dialog short; (c) the budget-gate fallback check at 2642 sits inside except BudgetExceededError:, so a crash there drops the BudgetExceededError. I reproduced (a) on the PR head with Orchestrator._check_run_limits patched to raise BudgetExceededError. A clean run gives TOKEN_BUDGET_EXCEEDED. The same run with one transient JudgeInfrastructureError from check_all_async gives ERROR/error_phase=grading, and the next run --resume gives SUCCESS with weighted_score 1.0. So the same agent output gets a different final_status. On main this row stayed ERROR, so the status was never laundered into a pass. Under the Axis 8 scoring-correctness rule this blocks the merge. Fix: mark a fresh-run crash as repairable (error_phase="grading") only when all execution-phase work has finished. Option 1: run _check_run_limits before the criteria, or again in the detached re-grade over the seeded iterations. Option 2: when grading fails before the post-turn work, do not set error_phase "grading" on a simulation task (or on any task whose remaining loop work did not run), because _reject_simulation_under_execute exists for exactly this 'trajectory run would not have produced' reason. Add a test that patches the run-limit check to trip, crashes the grader once, resumes, and asserts the budget status survives. Also add the same test for a check_criteria: every_turn dialog. Nightly impact: the change is in the graded run path and the --resume partition. Any eval-runner job that resumes a partial nightly would re-grade these rows and publish a different status. n/a
  6. [Axis 8] A failed re-grade of an execution-fact row writes weighted_score 0.0, and --resume then never re-grades it (src/coder_eval/cli/run_command.py:911) — In _grade_resumed_tasks, the new first branch if result.grading_error is not None and (prior.has_execution_failure or ...) (run_command.py:911-918) only prints a message. It keeps the row that _finalize_result already wrote. In that row _record_error kept the prior status (self.result.final_status = self.prior_result.final_status; return, orchestrator.py:942-946), and then _finalize_weighted_score ran calculate_weighted_score over an empty results list, which writes self.weighted_score = 0.0 (results.py:742-744). I reproduced it on the PR head: execute, then edit the row to TIMEOUT/execution, then run --resume with a transient judge error. The row on disk is TIMEOUT execution 0.0 [] with grading_error='transient', and _owes_a_grade(...) returns False, because of if result.weighted_score is not None or result.success_criteria_results: return False (batch.py:416). So a transient grader fault leaves the row with a 0.0 score that was never measured. That row now counts in tasks_measured ('Rows carrying a criteria verdict (weighted_score is not None)'), and no later --resume repairs it. On main the ERROR result took restore_pre_grade_record(rt.run_dir), so the row stayed re-gradeable. evaluate <run_dir> has the same fault: the inherited arm (evaluate_command.py:527-529) calls _write_back(target.target, result, prepared_run_dir), which writes the 0.0 and refreshes run.json. Fix: when grading fails, keep weighted_score = None and keep success_criteria_results empty (or restore the prior record with the grading provenance added) for execution-fact priors too. A row with an open grading_error must not count as measured. Add a run --resume case to test_failed_regrade_preserves_execution_fact_and_records_the_grader_incident, which today tests evaluate only and does not check weighted_score. n/a

Non-blocking, but please consider before merge

  1. [Axis 1] _record_error is C(14) and its interacting booleans are hard to follow; one branch records a reference tamper as a grading_error (src/coder_eval/orchestrator.py:930) — _record_error(self, error, *, component, grading: bool = False) (radon C(14), in the hot orchestrator module) combines three overlapping booleans: the grading parameter (used only by the weighted-score call at line 1027), checker_failure = self._publish_grading_failures(error) (a call that mutates state and also returns a value), and is_grading = not isinstance(error, ReferenceTamperedError) and (checker_failure or self.prior_result is not None). Their combinations then pass through three early-exit branches. The cost to readability is visible at lines 942-944: if self.prior_result is not None and self.prior_result.has_execution_failure: if not is_grading: self.result.grading_error = self._grading_error_info(error, component). When a prior result exists, is_grading is False only for ReferenceTamperedError. So this branch records an answer-key tamper as a grading incident. That contradicts the notes ("an agent changing its answer key is an execution failure") and contradicts the NOT_GRADED-prior path, which a test pins to grading_error is None (test_execute_evaluate_loop.py:1042). _record_limit_error (905-909) also calls back into _record_error under its own condition, so a reader must trace both functions to know how one timeout is classified. Fix: classify once, up front, into an explicit outcome (for example phase = self._classify(error) returning execution / grading / reference), then branch once on that value. Remove the grading= flag parameter by having the weighted-score site raise or record through the same classifier.
  2. [Axis 1] _report_and_exit rises to D(30); the failed-regrade disposition is duplicated in run_command (src/coder_eval/cli/evaluate_command.py:524) — radon at the PR head: _report_and_exit is D(30), up from D(22) on the base (+8 in this PR). _grade_resumed_tasks (run_command.py:836) is C(14). Each function now makes the same three-way decision after a failed regrade: keep the prior execution fact and write back the grading incident, restore the prior verdict with provenance, or write back a new execution error. They do it with different branch structures: evaluate_command.py:524-546 (if result.grading_error is not None: ... if inherited or (prior.final_status is FinalStatus.ERROR and prior.error_phase == "grading"): ... else: restore_pre_grade_record(...)) and run_command.py:911-934 (if result.grading_error is not None and (prior.has_execution_failure or (...)): ... elif result.grading_error is not None or (...): restore_pre_grade_record(...); prior.grading_error = ...). The function is outside the hot modules and above CC 20, and the same logic appears at 2 sites, so the Axis-1 Medium anchor applies. Fix: move the decision into one function in orchestration/regrade.py, for example settle_failed_regrade(run_dir, prior, result) -> Outcome, that returns the row to publish. Each CLI then only prints and sets the exit code.
  3. [Axis 2] New persisted public model GradingErrorInfo adds a dict[str, Any] escape hatch, although a typed ErrorContext already exists (src/coder_eval/models/results.py:535) — details: dict[str, Any] | None = None (results.py:535) is a new field on a model that coder_eval.models.__all__ exports and that task.json persists for downstream consumers. Its only producer is _grading_error_info (orchestrator.py:864-876), which stores create_error_context(...). That helper builds a typed ErrorContext (errors/categories.py:85) and then flattens it to dict[str, Any]. Its docstring says the flattening exists only "for backward compatibility with EvaluationResult.error_details", and CLAUDE.md says the project is greenfield with no back-compat burden. errors/categories.py imports only enum and pydantic, so models/ can import it without a cycle. Type the field as ErrorContext | None and build it directly. At the one dict-sourced site (details=prior.error_details, orchestrator.py:686) use ErrorContext.model_validate(...), or record None for a legacy row. Also, ErrorContext already has error_message and timestamp, so message and recorded_at repeat them. Choose one source for each.
  4. [Axis 3] Resume arm that preserves execution facts on a failed regrade is untested. The evaluate twin has a test; the resume twin has none (run_command.py:911-918). (src/coder_eval/cli/run_command.py:911) — Lines 911-918 handle a resumed regrade that fails over an execution-fact or grading-ERROR prior: if result.grading_error is not None and (prior.has_execution_failure or (prior.final_status is FinalStatus.ERROR and prior.error_phase == "grading")):. Line 915 is uncovered at PR HEAD. A mutant that replaced the condition with if False and (...) survived all 361 tests. With that mutant, a TIMEOUT row whose resume regrade fails goes to the elif arm instead, and line 931 rewrites its message in run.json to Grading errored during --resume: .... The evaluate-side twin is pinned by tests/test_execute_evaluate_loop.py:844 (test_failed_regrade_preserves_execution_fact_and_records_the_grader_incident), which only runs evaluate. Fix: parametrize that test (or add a sibling) with command in ("evaluate", "resume"), as test_repaired_grading_error_reaches_a_verdict_without_replaying_execution already does. Assert the run.json row and task.json keep error_message == "original execution incident" and the original status, and carry the new grading_error. Also add a resume case for a repeated failure over an already-grading-ERROR row.
  5. [Axis 3] Resume fold-back of grading provenance into the run.json row is unasserted (src/coder_eval/cli/run_command.py:932) — Lines 932-933 are prior.grading_error = result.grading_error and prior.error_history = result.error_history. They copy the new incident onto the in-memory row that becomes the run.json entry. A mutant that deleted both lines survived all 361 tests. The only test of this arm, tests/test_detached_grading_boundaries.py:480 with phase="grading", asserts only folded.final_status and "Grading errored during --resume" in folded.error_message. It asserts nothing on folded.grading_error or folded.error_history, or on the on-disk copy. run.json is the downstream consumer contract (docs/REPORT_SCHEMA.md now documents grading_error/error_history), so this field can drop with no failing test. Fix: in that test, assert folded.grading_error.message == "mock failure". Also assert that the on-disk task.json grading_error equals the in-memory one, so run.json and task.json cannot disagree.
  6. [Axis 3] New log-tail routing (keep the prior error_log_tail; attach the tail to grading_error) is untested; both mutants survive (src/coder_eval/orchestrator.py:630) — Lines 629-633 are tail = log_tail.get_text() or None / if self.result.grading_error is not None: self.result.grading_error.log_tail = tail / if self.prior_result is None or not self.prior_result.has_execution_failure: self.result.error_log_tail = tail. I ran two mutants. (a) M2 always overwrites error_log_tail, so a failed regrade replaces the original TIMEOUT/BUILD_FAILED tail, which carries the Docker build-log tail. (b) M3 drops the grading_error.log_tail assignment. Both survived all 361 tests. orchestration.md says 'the run keeps its original status, message, structured context and log tail', and REPORT_SCHEMA.md lists log_tail as a grading_error field. test_failed_regrade_preserves_execution_fact_and_records_the_grader_incident (tests/test_execute_evaluate_loop.py:862-867) never seeds error_log_tail in the prior, and no new test reads grading_error.log_tail. Fix: seed error_log_tail="original tail" in that prior. Assert failed["error_log_tail"] == "original tail" and failed["grading_error"]["log_tail"] is a non-empty string.
  7. [Axis 5] One grading incident is stored twice (error_message/error_details/error_log_tail and grading_error), and the two copies do not agree (src/coder_eval/orchestrator.py:951) — Fresh-run path: a criterion raises, _check_criteria records GradingErrorInfo with component "orchestrator.grading" (line 890), and the outer handler calls self._record_error(e, component=failed_component) with failed_component = f"orchestrator.iteration_{self.result.iteration_count}" (lines 593-595). _publish_grading_failures then sets self.result.grading_error = selected[1] (line 900), and lines 949-958 write the same exception again into the terminal fields: self.result.final_status = FinalStatus.ERROR, self.result.error_phase = "grading" if is_grading else "execution", self.result.error_message = str(error), self.result.error_details = create_error_context(... component=component ...). The log tail is also written twice (lines 631 and 633). So one event has two records, and they disagree on which component failed. The model has no validator that ties error_phase, grading_error and final_status together (results.py:625-636). Because of this, _seed_from_prior_result needs special cases. It archives prior.grading_error AND, at lines 682-690, keeps an elif that builds a GradingErrorInfo from prior.error_message for a grading-ERROR row that has no grading_error. The new writers cannot produce that shape, and no legacy row can have error_phase == "grading" because the field is new in this PR, so the branch is dead. Fix: choose ONE authoritative record. Either the terminal error fields are the incident and grading_error is set only when an execution fact outranks it, or the reverse. Add a model validator that enforces this invariant. Delete the unreachable synthesis branch.
  8. [Axis 8] Grading-provenance fields are task.json-only; run.json rows and reports cannot show the grading incident or tell a grading ERROR from an execution ERROR (docs/REPORT_SCHEMA.md:163) — REPORT_SCHEMA.md:163-170 adds error_phase, grading_error and error_history to the task record, and resume/evaluate behaviour now depends on them. But eval_result_to_task_dict (src/coder_eval/run_record.py, the run.json row serializer) still writes only status, error_message and error_category, and no renderer in reports/ reads grading_error. The downstream eval-runner/dashboard reads run.json as its contract. It therefore cannot tell a repairable grading ERROR from a permanent execution ERROR. It also cannot see that a SUCCESS/FAILURE/NOT_GRADED row was restored with an open grading_error, because restore_pre_grade_record(..., prior_result=prior, grading_result=result) writes the old verdict plus provenance to task.json only. Cross-run error-rate comparisons will count grader outages as agent errors. Fix: add error_phase and a has_grading_error boolean (or grading_error.message) to the run.json row, and document them in REPORT_SCHEMA's run.json section. Optionally show a grading-incident marker in the markdown and HTML reports. The schema change is additive (EvaluationResult does not forbid extra keys), so older consumers and container images under version skew still load. No image rebuild is needed. n/a

Nits

  1. [Axis 1] restore_pre_grade_record round-trips JSON for no reason, and its log and console messages are out of date (src/coder_eval/orchestration/regrade.py:593) — Lines 593-600 serialize the prior result (text = restored.model_dump_json(indent=2) if restored is not None else backup.read_text(...)), then, if restored is None, parse the text back (restored = EvaluationResult.model_validate_json(text)), set two fields, and serialize again (text = restored.model_dump_json(indent=2)). Build the model once, change it, and dump it once. The success log at line 607 still says "Restored the ungraded record at %s after a grading failure.", but the function now restores the immediate prior verdict, which can be SUCCESS or FAILURE. run_command.py:928-929 has the same problem: it prints "keeping the ungraded row so it stays re-gradeable." and interpolates result.error_message, while the sibling branch at :916 uses result.grading_error.message. Use grading_error.message in both branches and reword both messages to say "prior record".
  2. [Axis 1] New tests repeat the check_all_async patch block 9 times and the evaluate-or-resume argv block twice (tests/test_execute_evaluate_loop.py:777) — The tests this PR adds write patch("coder_eval.evaluation.checker.SuccessChecker.check_all_async", new=AsyncMock(side_effect=...)) at lines 777, 809, 815, 870, 897, 951, 969, 981 and 1033. They also build the evaluate-or-resume argv block (["evaluate", str(task_dir)] if command == "evaluate" else ["run", str(AGENTLESS_TASK), "--run-dir", str(run_dir), "--resume"]) twice, at lines 790 and 1029. Replace these with two small helpers, _checker_raises(error) and _regrade_args(command, run_dir, task_dir).
  3. [Axis 2] restore_pre_grade_record keeps a backup-only optional signature that no production caller uses, and the unit tests pin it (src/coder_eval/orchestration/regrade.py:580) — The signature at regrade.py:580-581 is prior_result: EvaluationResult | None = None, grading_result: EvaluationResult | None = None. All three call sites pass both arguments (evaluate_command.py:532, evaluate_command.py:542, run_command.py:926). So the prior_result is None branches (read the backup, EvaluationResult.model_validate_json(text), and the ValueError catch they need) cannot be reached from production code, and pyright cannot show that. Make prior_result: EvaluationResult required. Pass only the two values used (grading_error: GradingErrorInfo | None, error_history: list[GradingErrorInfo]) and not a whole second EvaluationResult. Then delete the backup-read fallback, or keep it only if a caller still needs it.
  4. [Axis 3] error_history accumulation branches are uncovered; an incident can be dropped from history with no failing test (src/coder_eval/orchestrator.py:898) — Lines 898-899 are if self.result.grading_error is not None: self.result.error_history.append(self.result.grading_error). Also uncovered at PR HEAD: 939-941 (the same archival in _record_error), 944 (if not is_grading: self.result.grading_error = self._grading_error_info(error, component), which covers a ReferenceTamperedError during a regrade of an execution-fact prior) and 966 (the ReferenceTamperedError re-raise in _start_grading). Mutants that deleted 898-899 (M6) or 943-944 (M10) both survived. Fix: add an orchestrator-level test with two grading failures in one pass, for example a checker failure followed by a calculate_weighted_score ValueError. Assert that both incidents appear across grading_error + error_history. Add a regrade test that raises ReferenceTamperedError over a TIMEOUT prior and asserts the status stays TIMEOUT and grading_error is set.
  5. [Axis 3] Test name contradicts two of its three parametrizations (tests/test_detached_grading_boundaries.py:480) — async def test_the_resume_error_arm_restores_and_folds_back_ungraded( is now parametrized @pytest.mark.parametrize("phase", ["grading", "execution", None]). For "execution" and None it asserts that the row is NOT restored (expected = FinalStatus.NOT_GRADED if phase == "grading" else FinalStatus.ERROR). Split it into two tests: one that restores the ungraded row for a grading-origin ERROR, and one that keeps an execution-origin or unclassified ERROR.
  6. [Axis 4] New CLI error prints put grading_error.message into Rich markup without escape(); a message containing '[/...' raises MarkupError before the prior verdict is restored (src/coder_eval/cli/evaluate_command.py:525) — Line 525 console.print(f"\n[red]✗ Evaluation error: {result.grading_error.message}[/red]") runs before restore_pre_grade_record(...) at line 532. Line 916 in run_command.py does the same: f"[yellow]⚠[/] Grading {rt.task.task_id} errored ({result.grading_error.message}); ". The message is str(exception) from a criterion, from the before_grading hook, or from a timeout. A criterion error can include text from the graded workspace, which the agent controls. I checked with the repo's Rich: Console().print(f'[red]x Evaluation error: bad value [/tmp/x] in file[/red]') raises MarkupError: closing tag '[/tmp/x] ... doesn't match any open tag'. When that happens, evaluate crashes with a traceback and the prior SUCCESS/FAILURE/NOT_GRADED record is not restored, so the ERROR row stays on disk. In run --resume the error propagates out of _grade_resumed_tasks and stops grading of the rows that follow. Balanced markup such as [link=...] can also inject terminal hyperlinks. Sibling handlers in this file already use escape(str(e)) (lines 407, 431, 496). Fix: wrap the message in escape(...) at both new sites, and also at the existing result.error_message prints at evaluate_command.py:537 and run_command.py:928. CVSS:3.1/AV:L/AC:H/PR:N/UI:R/S:U/C:N/I:L/A:L
  7. [Axis 5] restore_pre_grade_record becomes a fourth task.json serializer that does not use the shared TASK_JSON_TRANSCRIPT_EXCLUDE (src/coder_eval/orchestration/regrade.py:594) — On main, restore copied the backup bytes verbatim. Now it re-serializes: text = restored.model_dump_json(indent=2) if restored is not None else backup.read_text(encoding="utf-8") (line 594) and text = restored.model_dump_json(indent=2) (line 600). Each of the other three task.json writers passes exclude=TASK_JSON_TRANSCRIPT_EXCLUDE: Orchestrator._finalize_result (orchestrator.py:1195), evaluate_command._write_back (line 640) and docker_runner (line 869). Because the restored row was loaded from a transcript-stripped task.json, today the only drift is extra "transcript": null keys. But the path now writes a different on-disk shape from its siblings. If a future caller passes an in-memory result, inline judge transcripts will be written into task.json. Fix: send every task.json write through one helper (for example write_task_json(path, result) next to the exclusion constant) and call it here.
  8. [Axis 6] Failed-regrade paths in run_command ignore the restore result and print inaccurate operator messages (src/coder_eval/cli/run_command.py:926) — This change widens restore_pre_grade_record to catch (OSError, ValueError) and return False. Neither caller reads the result. run_command.py:926 calls restore_pre_grade_record(rt.run_dir, prior_result=prior, grading_result=result), prints 'keeping the ungraded row', and sets result = prior. evaluate_command.py:532 and 542 print 'prior verdict restored'/'preserved'. When the write fails or the target is a symlink, the ERROR-grading row stays on disk, but run.json is rebuilt from the in-memory prior. That gives exactly the run.json vs task.json mismatch the old docstring warned about. The row can still be re-graded through _owes_a_grade, so the failure is not severe. Fix: branch on the return value. On False, print a warning that names the file left as ERROR, and fold the on-disk result back in instead of the prior.
  9. [Axis 7] New public persisted model GradingErrorInfo has no field descriptions (src/coder_eval/models/results.py:531) — class GradingErrorInfo(BaseModel) declares message: str, details: dict[str, Any] | None = None, log_tail: str | None = None and recorded_at: datetime with no Field(description=...). The model is exported from coder_eval.models and serialized into task.json. Its sibling EarlyStopInfo (line 475) describes every field. The CLAUDE.md DRY principle says field descriptions are defined once in the Pydantic models. As a result, the field meanings exist only in docs/REPORT_SCHEMA.md:165-167, and the model's JSON schema has none. Fix: add descriptions to the four fields. State that recorded_at is naive local time, the same as started_at.
  10. [Axis 7] _owes_a_grade docstring now contradicts its body (src/coder_eval/orchestration/batch.py:414) — The docstring says "EVIDENCE, not label ... A row with a criteria vector or a score HAS been graded, whatever its status." The new first check if result.final_status is FinalStatus.ERROR and result.error_phase == "grading": return True is a label test, and it returns True even when a score or a criteria vector exists. The docstring states the contract callers rely on, so update it to name this exception.

What's Missing

Parallel paths:

  • 🟡 The new _owes_a_grade arm sends a grading-origin ERROR to grade-in-place, but the resume contract on every other surface still says the opposite. Not updated: docs/USER_GUIDE.md § Resuming a run (table row "Any other status, including FAILURE / ERROR -> already complete" and "Resume never retries failures. FAILURE and ERROR count as complete"), the partition_for_resume docstring ("FAILURE and ERROR rows that DO carry a verdict stay complete under both commands"), the test_partition_still_never_retries_failures docstring in tests/test_resume.py, and the task.execute.json line in .claude/shared/run-layout.md / plugins/coder-eval/reference/run-layout.md ("over a NOT_GRADED row"). A user who reads the guide expects an ERROR row to stay final, but a later run --resume now changes it. (trigger: src/coder_eval/orchestration/batch.py) _(restates: Axis 7: owes_a_grade docstring now contradicts its body)
  • 🟡 The provenance contract (grading_error + error_history) applies only to the in-process grading path. The driver: docker detached-grade path is not updated. _grade_in_container turns a container failure into RegradeError. On resume (run_command.py:895-908), that arm only appends "Grading failed during --resume" to error_message and records no GradingErrorInfo and no history. In evaluate (evaluate_command.py:491), it exits 1 and records nothing. A grading-ERROR docker row that fails again in its grading container therefore loses the audit trail the PR promises, and its run.json message changes. No test drives the RegradeError arm over an error_phase: grading prior. (trigger: src/coder_eval/cli/run_command.py)

Tests:

  • 🟡 The unit tests for partition_for_resume in tests/test_resume.py were not extended for the new _owes_a_grade arm. Missing: a direct case where ERROR + error_phase="grading" with graded=True (score/criteria present) goes to to_grade under grade=True and stays in prior_results under grade=False, plus companion cases that show ERROR/"execution", ERROR/None and TIMEOUT/"grading" stay complete. Today only the CLI integration tests pin the new routing, and test_partition_still_never_retries_failures keeps passing only because its fixture rows set no phase. (trigger: src/coder_eval/orchestration/batch.py)

Downstream consumers:

  • 🟡 The PR adds an execution-vs-grading classification (error_phase, has_execution_failure), but the consumers that compute rates and scores from status were not updated. orchestration/experiment.py::_measured_scores still scores every category == "error" row as 0.0, so a grader outage counts as an agent miss in A/B means and the nightly headline. The run-level tasks_error / error-share rates and the suite rollup rows_error (reports/markdown.py:951) do the same. The _pick_worst_status docstring still says a failed resume grade folds back "still ungraded", which is no longer true: the restored prior can now be SUCCESS/FAILURE with an open grading_error. (trigger: src/coder_eval/models/results.py) (restates: Axis 8: Grading-provenance fields are task.json-only; run.json rows and reports cannot show the grading incident or tell a grading ERROR from an execution ERROR)

Display & mapping dicts:

  • 🔵 No new enum value was added, but the new grading_error / error_history fields are not shown anywhere. reports/html.py::_render_error_details and the markdown failure sampling (reports/markdown.py:1058/1144) read only error_message / error_details / error_log_tail. A SUCCESS/FAILURE/NOT_GRADED row restored with an open grading_error shows no error section. An ERROR row does not say whether it is a repairable grading incident. Archived incidents in error_history are never shown. The evalboard task shape (evalboard/lib/runs.ts) does not read these fields either. (trigger: src/coder_eval/models/results.py) (restates: Axis 8: Grading-provenance fields are task.json-only; run.json rows and reports cannot show the grading incident or tell a grading ERROR from an execution ERROR)

Nightly pipeline:

  • 🟡 The PR description says no saved records change, but it does not state the effect on the nightly pipeline. (a) task.json gains error_phase / grading_error / error_history, which the external coder-eval-uipath / eval-runner consumers read. (b) For driver: docker, the orchestrator that runs inside the container writes error_phase, so an image built before this change writes untagged ERROR rows. The host then treats these as legacy execution facts that cannot be repaired, until the nightly image is rebuilt. (c) Any nightly job that resumes a partial run now re-grades fresh grading-ERROR rows and publishes a different final_status and pass rate than the original run.json. The PR must state whether an image rebuild is required and what a resumed nightly will report. (trigger: src/coder_eval/orchestrator.py)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE069 (next free number after CE068; CE067 has no rule file, so check .claude/notes/lint-rules.md before you reuse it) no_inline_grading_error_predicate. The rule forbids an ast.Compare whose left side is an attribute ending in .error_phase and whose right side is a string literal, anywhere under src/coder_eval/ except models/results.py. It also forbids .final_status.is_execution_fact outside models/results.py and models/enums.py. Callers must use the new EvaluationResult.is_grading_error / has_execution_failure properties. Put the rule in tests/lint/rules/ce069_no_inline_grading_error_predicate.py and register it in tests/lint/runner.py. Add a module docstring that names the blind spot: a getattr or a rebound local escapes the rule. Prevents: The 7 hand-written copies of final_status is ERROR and error_phase == "grading" (orchestrator.py:682/:691, results.py:731, batch.py:414, run_command.py:913/:920, evaluate_command.py:527). The variant at evaluate_command.py:541 that already drifted (error_phase == "grading" with no ERROR check). The overlapping is_execution_fact/has_execution_failure pair (run_command.py:912-913). Finding 1, and part of finding 2's dead branch at evaluate_command.py:541.
  • [pyright] Make the illegal error states unrepresentable, so that pyright proves the dead branches dead. Replace the three independent fields error_phase: Literal[...] | None, error_message/error_details and grading_error on EvaluationResult with one discriminated field terminal_error: ExecutionError | GradingIncident | None (discriminator phase). Keep grading_error only as an "outranked by an execution fact" slot. Then if result.grading_error is None and <phase is grading> cannot be expressed, and pyright standard mode reports the narrowed-to-Never arm. You can also turn on reportUnnecessaryComparison for cli/ and orchestration/regrade.py. Prevents: Finding 2 (the unreachable grading-phase branch at evaluate_command.py:541-543 and the redundant clause at run_command.py:919-921). Finding A5 (one grading incident stored twice with components that disagree, orchestrator.py:949-958 vs :890/:900). The dead synthesis elif at orchestrator.py:682-690, which builds a GradingErrorInfo from error_message for a shape that no writer produces.
  • [ce-lint] CE070 rich_markup_must_escape_dynamic_text, scoped to src/coder_eval/cli/ and any module that imports rich. It fires on console.print(f"...") and on Console.print/.log/.status with a JoinedStr argument when a FormattedValue interpolates an expression whose trailing name matches message|error|error_message|reason|detail|stderr|output|^e$ or is a str(<exc>) call, and that expression is not wrapped in escape(...) from rich.markup or a markup=False kwarg. Use # noqa: CE070 for text that is known to be static. Prevents: Finding A4: grading_error.message (agent-influenced criterion text) put into Rich markup at evaluate_command.py:525 and run_command.py:916, and the existing unescaped error_message prints at evaluate_command.py:537 and run_command.py:928. A message that contains [/... raises MarkupError before restore_pre_grade_record runs, and it can stop grading of later rows on --resume. Sibling handlers in the same file already use escape() (lines 407/431/496), so the pattern is known and only remembered by hand. No ruff or bandit rule covers Rich markup.
  • [ce-lint] CE071 task_json_single_writer. First add one write_task_json(path, result) helper next to TASK_JSON_TRANSCRIPT_EXCLUDE in evaluation/judge_persistence.py, which always passes exclude=TASK_JSON_TRANSCRIPT_EXCLUDE. Then forbid any write_text_atomic(...) / Path.write_text(...) whose payload is an EvaluationResult.model_dump_json(...) call outside that helper. The AST shape is a model_dump_json call that is a direct argument of a write call, or that is assigned to a local which a write call then uses. This sits beside CE053, which already owns the TASK_JSON filename literal. Prevents: Finding A5-low: restore_pre_grade_record became a fourth task.json serializer (regrade.py:594/:600) and dropped TASK_JSON_TRANSCRIPT_EXCLUDE. The three siblings at orchestrator.py:1195, evaluate_command.py:640 and docker_runner.py:869 each pass the exclusion by hand. It also removes the needless JSON round-trip in finding A1-low (regrade.py:593-600).
  • [ce-lint] Extend CE049 (no_score_or_zero) from score or 0.0 to assignments. Fire on <target>.weighted_score = <numeric literal> and on <score-named attr> = ... if ... else 0.0 in src/. The intent is the same as CE049: an unmeasured row is None, never 0.0. Use # noqa: CE049 for the one legitimate case, which is 0.0 when a task really has zero criteria and has been graded. Prevents: Finding A8-high. calculate_weighted_score writes self.weighted_score = 0.0 over an empty results list (results.py:742-744) after a failed regrade of an execution-fact prior. _owes_a_grade (batch.py:416) then treats the row as measured, so --resume never repairs it, and tasks_measured counts a score that was never measured.
  • [ce-lint] CE072 run_json_projection_parity, a whole-tree @pytest.mark.lint class in tests/test_custom_lint.py that follows the CE030 doc_schema_parity pattern. Every EvaluationResult field must either be emitted by run_record.eval_result_to_task_dict or be listed in a RUN_JSON_EXEMPT: dict[str, str] (field -> reason) in tests/lint/. It must also be named in docs/REPORT_SCHEMA.md's run.json task_results[] section when it is emitted. The rule also fails when a field that orchestration/batch.py (_owes_a_grade) reads is exempt, because resume behaviour must not depend on a field that run.json consumers cannot see. Prevents: Finding A8-medium: error_phase, grading_error and error_history are documented only for task.json (REPORT_SCHEMA.md:163-170), resume branches on them, and run.json (the downstream contract) cannot tell a repairable grading ERROR from an execution ERROR or show a restored verdict with an open grading_error.
  • [ce-lint] CE073 no_untyped_dict_on_persisted_model. In src/coder_eval/models/, a field annotation of dict[str, Any] (or dict[str, Any] | None) on a BaseModel class is forbidden unless the class.field pair is in an allowlist that holds today's offenders (for example EvaluationResult.error_details). The allowlist can only get smaller. In the same rule, also require Field(description=...) on every field of a class exported from coder_eval.models.__all__, with the same frozen allowlist. This applies the CLAUDE.md DRY rule that "field descriptions are defined once in the Pydantic models". Prevents: Finding A2-medium: GradingErrorInfo.details: dict[str, Any] | None (results.py:535) when the typed ErrorContext already exists. Finding A7-low: GradingErrorInfo's four fields have no descriptions (results.py:531), while the sibling EarlyStopInfo describes every field.
  • [ce-lint] CE074 no_always_passed_optional_param, a whole-tree rule in the same family as CE037 (dead private helper). It flags a function in src/ that has a parameter annotated X | None = None when every call site in src/ passes that parameter by keyword with an expression that is not the literal None. A test-only call does not count. This means the None-branch is reachable only from tests. Use # noqa: CE074 for a public SPI entry point. Its blind spots: **kwargs forwarding and dynamic calls. Prevents: Finding 2(3) and finding A2-low: restore_pre_grade_record(..., prior_result: EvaluationResult | None = None, grading_result: ... | None = None) (regrade.py:580-581). All three production callers pass both arguments, so the backup-read fallback (regrade.py:590-597) is live only in tests and misleads readers about crash recovery.
  • [ci-gate] Add a complexity ratchet: make complexity-ratchet, wired into make verify. It runs radon cc -j on the files changed against origin/main. It fails when a function that is already rank C or worse (CC >= 11) increases its CC, or when a new function exceeds CC 15 in the hot modules (orchestrator.py, orchestration/, cli/). Ruff C901 alone is impractical here, because about 200 functions are already rank C or worse and about 47 are D or worse. That many noqa markers would dilute it, so a diff-relative ratchet follows the "cap below the current value" rule without a flag day. Prevents: Finding 1-medium: _record_error is C(14) with three interacting booleans (orchestrator.py:930). Finding A1/A7-medium: _report_and_exit rose from D(22) to D(30) and _grade_resumed_tasks from B(8) to C(14), with the failed-regrade disposition duplicated in two shapes (evaluate_command.py:524-546, run_command.py:911-934).

Harness improvements (not statically reachable):

  • Add a crash-then-resume equivalence matrix test. For each interruption point in the graded run path, inject a single JudgeInfrastructureError at that point, then run run --resume. Assert that the final_status and weighted_score match an uninterrupted run of the same agent output. The interruption points are: a criteria crash before the post-criteria _check_run_limits (orchestrator.py:2314), a dialog check_every_turn crash at :2628 before the budget gate at :2636, and a crash inside the except BudgetExceededError fallback at :2642. Patch _check_run_limits to trip in the budget cases. Why not static: Whether execution-phase work (run-limit checks, remaining dialog turns) was skipped depends on the order of runtime events and on which statuses the detached evaluate-only path re-derives. Only running both paths and diffing the results shows this. Prevents: Finding A8-critical: a transient grader crash makes the row repairable (_owes_a_grade at batch.py:414-415), and --resume then launders TOKEN_BUDGET_EXCEEDED or an unfinished dialog into SUCCESS.
  • Add evaluate/resume twin parity with a run.json == task.json assertion. Put a shared helper assert_row_agrees(run_dir, task_id) in tests/ that compares status, error_message, weighted_score, grading_error and error_history between the run.json task_results row and the on-disk task.json. Every failed-regrade test must be parametrized over command in ("evaluate", "resume") and must call the helper. Start with test_failed_regrade_preserves_execution_fact_and_records_the_grader_incident (tests/test_execute_evaluate_loop.py:844) and test_the_resume_error_arm_restores_and_folds_back_ungraded (tests/test_detached_grading_boundaries.py:480). Add one case that seeds error_log_tail and asserts it survives while grading_error.log_tail is set. Why not static: Run.json and task.json disagree only after a CLI run writes both files, through different code paths (in-memory fold-back in run_command vs. the _write_back in evaluate). A lint rule cannot see whether two runtime artifacts agree. Prevents: Finding A3-medium (run_command.py:911-918 preserve arm is untested; a mutant of it survives). Finding A3-medium (the fold-back at run_command.py:932-933 is unasserted). Finding A3-medium (the log-tail routing at orchestrator.py:629-633). Finding A6-low (the restore return value is ignored, which causes the run.json/task.json mismatch). Finding A8-high (weighted_score 0.0 after a failed resume regrade).
  • Add a runtime model invariant plus a classifier table test. Add an EvaluationResult model_validator(mode="after"): error_phase == "grading" implies final_status is ERROR and grading_error is not None. Also add a parametrized test over every FinalStatus x {"execution", "grading", None} that pins has_execution_failure / is_grading_error. In that test, only (ERROR, "grading") is exempt among the execution-fact statuses, and TIMEOUT, BUILD_FAILED and the budget statuses stay True with phase "grading". Why not static: These are invariants between the values of three fields, so they hold or fail at construction and load time (including on hand-edited task.json). A lint rule cannot reason about values. The pyright discriminated-union redesign in the static bucket is the long-term fix, and this validator is the guard until that ships. Prevents: Finding A3-high: the has_execution_failure mutant that ignores the status survives every test. Finding A5-medium: one incident is stored twice and the model enforces no invariant. Finding 2: the dead branches stay hidden because nothing states that the shape cannot occur.
  • Add a Harbor WORK_DIR immutability guard. In a parametrized test, run evaluate --format harbor under every grading failure mode (checker raises, before_grading hook fails, grading timeout, weighted-score ValueError). Assert with path_utils' tree digest that the WORK_DIR digest is the same before and after, and that the exit code is 1. Why not static: Whether a write lands in the user's directory depends on the runtime EvaluateMode and on whether restore_pre_grade_record decides to write. A rule that requires every write call to be under a RUN_DIR guard would be a dominance analysis over semantic conditions, and that is not reliable with AST matching. Prevents: Finding A3-high: a grading failure under evaluate --format harbor plants task.json in the user's WORK_DIR, because the restore calls at evaluate_command.py:532 and :542 have no RUN_DIR guard.
  • Add a diff-scoped mutation-testing target. make mutate-diff runs mutmut (or cosmic-ray) only on lines changed in orchestrator.py, orchestration/batch.py, orchestration/regrade.py, cli/run_command.py, cli/evaluate_command.py and models/results.py, against the tests that touch them, and lists the surviving mutants. Run it in the review workflow for PRs that change grading, resume or status classification, as an advisory step, not a blocking gate. Why not static: A surviving mutant shows that tests are weak, and that can be known only by running the suite against changed code. Coverage alone said these lines were partly covered. Prevents: This review needed hand-made mutants to show six untested branches: has_execution_failure (results.py:728), run_command.py:911 and :932-933, orchestrator.py:629-633, :898-899 and :943-944 (finding A3-low error_history accumulation and the ReferenceTamperedError-over-TIMEOUT path).
  • Record the decision about what static analysis cannot reach. These findings need human review or a targeted test, not a rule: the _owes_a_grade docstring that contradicts its body (batch.py:414), the stale operator and log messages ("ungraded record" at regrade.py:607, run_command.py:928-929), the test name that contradicts its parametrization (test_detached_grading_boundaries.py:480), and a ReferenceTamperedError recorded as grading_error over an execution-fact prior (orchestrator.py:942-944). For the last one, add the regrade test that raises ReferenceTamperedError over a TIMEOUT prior and asserts that the tamper is classified as an execution fact. The repeated check_all_async patch block (9 copies) and the evaluate/resume argv block (2 copies) in tests/test_execute_evaluate_loop.py should become shared _checker_raises / _regrade_args helpers when the twin-parity parametrization is added. A rule is not worth it for test duplication of this size. Why not static: Docstring and message accuracy, test-name semantics and whether a tamper is a grading or an execution failure need a semantic judgment about intent, and no AST pattern expresses that. Prevents: Finding A7-low (docstring drift), finding A1-low (stale messages and test duplication), finding A3-low (misleading test name) and finding 1-medium (tamper misclassified in _record_error).

Top 5 Priority Actions

  1. Stop the critical status laundering at src/coder_eval/orchestration/batch.py:414. Set error_phase="grading" on a fresh-run crash only after all execution-phase work is done, or run _check_run_limits again over the seeded iterations in the detached regrade (orchestrator.py:2314, 2628, 2642). Add tests: a budget stop plus one grader crash and then --resume must keep TOKEN_BUDGET_EXCEEDED, and a check_criteria: every_turn dialog needs the same test.
  2. Fix the unmeasured 0.0 score at src/coder_eval/cli/run_command.py:911 and evaluate_command.py:527. When a regrade fails over an execution-fact prior, keep weighted_score None and success_criteria_results empty, or restore the prior record with the grading provenance added, so that _owes_a_grade (batch.py:416) still sends the row to regrade and tasks_measured does not count it. Add a run --resume case that asserts weighted_score is None.
  3. Add an EvaluateMode.RUN_DIR guard to both restore_pre_grade_record calls at src/coder_eval/cli/evaluate_command.py:532 and :542, so that a grading failure under evaluate --format harbor does not write task.json into the user's WORK_DIR. Add a harbor test where the checker raises and assert that no task.json is left behind.
  4. Add one EvaluationResult.is_grading_error property (src/coder_eval/models/results.py:728) and use it at all 7 hand-written copies (orchestrator.py:682/691, batch.py:414, run_command.py:913/920, evaluate_command.py:527). Delete the unreachable branches at evaluate_command.py:541-543 and run_command.py:919-921, and the backup-read fallback in regrade.py:590. Make prior_result required and rename the function so its name matches what it now does.
  5. Close the test gaps in the new contract. Add a parametrized FinalStatus x error_phase table test for has_execution_failure (results.py:728). Add a resume parametrization to the preserve-execution-fact test for run_command.py:911-918. Add asserts on grading_error and error_history fold-back (run_command.py:932-933) and on log-tail routing (orchestrator.py:629-633). Then add error_phase and grading_error to the run.json row in run_record.py and REPORT_SCHEMA.md:163, so downstream consumers can tell a grading ERROR from an execution ERROR.

Stats: 1 🔴 · 5 🟠 · 8 🟡 · 10 🔵 across 8 axes reviewed.

@cosminacho
cosminacho marked this pull request as draft November 5, 2026 13:47
@cosminacho
cosminacho marked this pull request as ready for review November 5, 2026 15:57
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Claude finished @cosminacho's task in 1m 40s —— View job


Review in Progress

Todo:

  • Read .github/code_review.md for review standards
  • Read CLAUDE.md for project conventions
  • Review the full diff (git diff origin/main...HEAD)
  • Read each changed file in full for context
  • Perform cross-file consistency checks
  • Analyze what's missing
  • Provide structured review feedback

@bai-uipath bai-uipath left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right goal, but please restructure before merge. Most of the complexity comes from deciding after the fact whether an exception was execution or grading, and from writing task.json and then undoing it.

  • Settle execution facts before grading: check run limits right after the turn without raising, grade, then raise. A grading crash can then never skip a budget or timeout fact, and the seeded limit error and the re-checks in the error path go away.
  • One grading error type: wrap the grading sites so failures surface as a single typed error handled in one place, instead of matching exceptions by identity across several flags.
  • A mid-dialog grading crash is an execution failure: the remaining turns never ran, so treat it as unfinished execution; the dialog special cases in the status check and in regrade go away.
  • Two fields, not three: keep error_phase and error_history; drop grading_error, which duplicates error_message / error_details.
  • Write once on regrade: don't persist a failed regrade; the caller appends the incident to the prior record and writes once. The stale-backup overwrite becomes impossible and the restore branches in run and evaluate collapse into one.
  • Split out the weighted_score 0.0 to None change: it's a scoring change unrelated to recovery, and averages that drop None (the Flow V2 comparison in coder_eval_uipath) would stop counting errored rows.

If you want the Discovery benchmark unblocked sooner, the typed error, the two fields and the owes-a-grade change are enough for a first PR.

Comment thread src/coder_eval/orchestrator.py Fixed
Comment thread src/coder_eval/orchestrator.py Fixed
@cosminacho
cosminacho force-pushed the codex/grading-error-recovery branch from 1e2496b to 4608e6b Compare November 5, 2026 21:22

@bai-uipath bai-uipath left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve: all six restructure points landed, and the settle, grade, then write-once shape is much easier to follow.

Before merge:

  • Delegate smoke prompt: reframing FizzBuzz as a UiPath Python Scope helper changes what the Delegate live E2E measures and has nothing to do with grading recovery.
  • Resume docs: the user guide still says resume never retries FAILURE or ERROR, and the owes-a-grade docstring still says any row with a score has been graded. Fix: update both for grading-phase ERROR.

Consider as follow-up:

  • Docker failure marker: the container signals a failed grade through a magic key in the incident's untyped details dict, which has to be popped in three places; if one is ever missed, every later container regrade of that row reads as failed. Fix: a sidecar file or a distinct exit code.
  • run.json can't see the phase: reports and evalboard still can't tell a grading ERROR from an execution ERROR.

Minor: an is_grading_error property would replace the four hand-written copies of the check (one carries a clause that is always true); telemetry fires for a failed regrade that is never persisted.

@cosminacho
cosminacho merged commit bc45225 into main Nov 5, 2026
16 of 17 checks passed
@cosminacho
cosminacho deleted the codex/grading-error-recovery branch November 5, 2026 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants