feat(cli): coder-eval execute + detached grading via evaluate <run_dir> - #154
Open
akshaylive wants to merge 11 commits into
Open
feat(cli): coder-eval execute + detached grading via evaluate <run_dir>#154akshaylive wants to merge 11 commits into
coder-eval execute + detached grading via evaluate <run_dir>#154akshaylive wants to merge 11 commits into
Conversation
akshaylive
requested review from
CarlesUIPath,
bai-uipath,
tmatup and
uipreliga
as code owners
September 3, 2026 20:10
|
Claude finished @akshaylive's task in 1m 49s —— View job Todo List
|
coder-eval execute — run tasks without grading themcoder-eval execute + detached grading via evaluate <run_dir>
`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
force-pushed
the
akshaya/coder_eval_execute
branch
from
September 3, 2026 22:43
3387bdb to
489383d
Compare
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>
…n detached grading Addresses the PR #154 review. The `execute`/`evaluate` split shipped with the right shape but several ways to produce a plausible number that is wrong. Verdict-changing: * `grading_sandbox_config` rewrote `driver: docker` -> `tempdir` unconditionally, so a container task's criteria ran on the grading host — scoring FAILURE for a trajectory `run` scored 1.0, running `rm -rf /verifier` unsandboxed, and neutralizing `Sandbox.adopt`'s own docker refusal. Now refused unless `--allow-host-grading`; an opted-in row is stamped `graded_on_host`. * `max_turns_exhausted` and `_check_run_limits` sat after the grading early return, so `execute` exited 0 where `run` exited 1 for identical agent output — and `_seed_from_prior_result` cannot restore a fact never captured. * Experiment aggregation filtered on `weighted_score is not None`, dropping ERROR/BUILD_FAILED rows from BOTH sides: an infrastructure-failure night scored higher than a clean one. Only `ungraded` leaves both sides now. * `verify_reference_unchanged` compared a staged-copy digest against the raw source, so any `.git`-carrying reference reported a permanent false mismatch. * A grading crash left ERROR on disk (`_finalize_result` writes before returning), making the row permanently un-regradeable and leaving run.json disagreeing with task.json. Trust boundary — a run directory is a shareable artifact: * A recorded config carrying shell is refused unless `--allow-recorded-commands` (hooks excluded on the in-place path, where they do not run). * `artifacts / prior.task_id` is containment-checked like its `sandbox_path` sibling. * `write_text_atomic` opens `O_EXCL|O_NOFOLLOW`, closing the `task.json.tmp` symlink primitive that bypassed the write-back's own guard. * `_sanitize_restored_path` drops relative entries and anything in the run dir. Reporting and evalboard: * `SuiteRollup.pass_rate` is `float | None` with a serialized `rows_graded`; ungraded rows leave `failed_samples`. * Telemetry omits `Score` rather than laundering `None` into a real-looking 0.0. * A detached grade records `graded_by_api_routing` instead of overwriting the run's. * `reports_stats` drops only the score, not the whole row — duration, tokens and turns are facts about the run, not verdicts. * The evalboard's ungraded fields had no readers, so a 12-task `execute` run rendered a red `0% - 0 / 12`. Swept every rate surface and gave `StatusCategory` an `assertNever` guard, since widening the union produced no compiler error anywhere and that is how `lib/overview.ts` was missed. New lint rules, each traceable to one of the above: CE049 (no `score or 0.0`), CE050 (no untyped `getattr` probe over a discriminated union), CE051 (no silent sandbox-driver rewrite). Adding fires-on-violation tests for CE047/CE048 also surfaced the scoping bug CE047 warns about: its `[/\\]src[/\\]` regex put every repo-relative path out of scope, so such a test would have passed vacuously. Two cheap extractions (`_terminal_status`, `_apply_resume`) plus `_fold_replicates` undo the complexity the grading switch added: `aggregate_results` F(54) -> E(36), below its pre-PR F(48). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_run-task-internal` started its host-heartbeat watchdog as an unconditional side effect of the command body. That thread's whole authority is `os._exit(137)`, and the only process it may reap that way is the container's own disposable main -- there is no container to orphan anywhere else. A test invokes the command in-process, legitimately: the command must refuse a malformed context.json, and proving that means calling it. The pytest worker inherited the thread, which found no heartbeat and exited the worker 40s later (20s grace + 20s stale window), inside whatever unrelated test file that worker had since moved on to. Every property of the failure came from the missing guard: it named a different test on each run and on each platform (opencode on Linux, sandbox_record_cli on Windows), carried no traceback because there is no exception to raise, and hid at high parallelism -- with 14 local workers the run ended before the timer fired, so it reproduced only on CI's 2. It also took the coverage gate with it: a dead worker returns no coverage data, so one killed process reported as "total of 65.13 is less than fail-under=80.00", naming neither the test nor the cause. Timing on both platforms is exactly 40s from that test to the worker's death. The watchdog is now defined and started only under CODER_EVAL_IN_CONTAINER, which docker_runner sets on the container's argv -- not on `driver`, since this same command rewrites `driver: docker` -> `tempdir` before building the in-container Orchestrator and a driver-based gate would disarm itself on exactly the path that needs it. CE052 makes it permanent: an `os._exit` in src/ must sit inside a branch testing that var. Its rule test asserts the real module passes, so the rule cannot pass vacuously. The behavioural test asserts on the live thread list rather than by patching `threading`, and fails when the guard is inverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
…-rate defects Addresses the second review of PR #154 (against 7d5d55d). Every fix is a place where the code substituted something plausible for something it did not know. Verdict parity — `run` must equal `execute` + `evaluate`: - `_terminal_status` put `max_turns_exhausted` ABOVE the grading switch, so an execute row finalized MAX_TURNS_EXHAUSTED. That status is an execution fact, so the first arm then pinned it forever: identical agent output scored SUCCESS/1.0 under `run` and MAX_TURNS_EXHAUSTED under `execute` -> `evaluate`. It is not knowable without grading (`run` returns SUCCESS when the criteria pass), so the fact is carried on the row and the status is left to the grade. - `partition_for_resume` routed on FinalStatus.category, so an execute row that also tripped a run limit (TIMEOUT, a budget stop) was called "already complete" and stayed permanently unscored -- while `evaluate <run_dir>` graded the identical bytes. The test is now the row's evidence: executed, never scored. - `evaluate` read `final_status` as this pass's own outcome. A preserved TIMEOUT exited 0 under "All criteria passed" (a CI wrapper went green on a failed row); a preserved ERROR printed the original run's crash message as though grading had crashed, claimed the row was left ungraded (false), and discarded a verdict just computed at 1.000. Trust boundary: - The recorded-config gate walked only success_criteria + hooks, so a shared run dir whose criteria were all file_exists passed it and still reached `uv pip install` / `npm install` / `git clone` with recorded values. The scan now covers sandbox provisioning and llm_judge; `git clone` gets a `--` separator (the URL sits in argv position 2). - `Sandbox.resolve_files` joined a criterion path onto the sandbox root with no containment check, and `Path(root) / '/etc/passwd'` is `/etc/passwd`. It was the one task-authored path skipping `_resolve_within_sandbox` -- defensible until `evaluate <run_dir>` began rebuilding criteria from a shareable artifact. - `_write_synthetic_task_json` was the one writer of task.json not routed through `write_text_atomic`, so it followed a symlink at its temp name. - `_assert_grade_honored` refused in memory only, leaving the graded record on disk for `execute --resume` and `aggregate` to re-absorb; it now quarantines to a `.graded` sidecar and keys on evidence rather than on the status label. write_text_atomic, both halves: - A fixed temp name plus O_EXCL turned a leftover from a SIGKILL into a permanent refusal to persist the record -- and `--resume` then re-ran the task into the same run dir and hit it again, re-paying for the agent every pass. The name is now unique per call; O_EXCL keeps its guarantee. - Creating it 0600 made every container-written task.json unreadable by the host across the docker bind mount on Linux (an unguarded read). Mode is 0666 so the umask applies, as `Path.write_text` did. Fabricated rates: - `tasks_graded` keeps ERROR rows, correct under `run` but not under `execute`, where nothing was measured at all: a 100-task execute night with 5 crashes published pass_rate 0.0 / error_share 1.0. Both are None when no row produced a verdict. - evalboard `turnBudgetRateForTasks` compared a raw "SUCCESS", booking an ungraded row as a budget miss; watchlist `attention()` scored an all-ungraded skill failRate 1.0 and put it top of an exec-triage hero. The remaining raw literals are converted to the typed helpers, and trends paints ungraded grey rather than red. Enforcement: - CE053: no bare run-record filename literal outside path_utils. The constant shipped with a rename-safety rationale while twelve literals stayed behind, including all three rglob sites its own comment cites; those are migrated. - `[tool.ruff.lint] external` is completed and now has a parity test -- CE047 and CE048 advertise `# noqa` codes ruff was rejecting with RUF102. Also: `graded_on_host` and `replicate_index` on evaluate's non-delegating branch; `run --allow-host-grading` without `--resume` is a BadParameter instead of a silent no-op; context.json's variant_id/replicate_index are validated, not just annotated; `_pick_worst_status`'s priority map is typed and indexed directly; `_skip_hooks_for_adopted` takes the command list instead of a magic string; the simulation grade=False stub raises instead of guaranteeing a downstream ValueError. Tests: the evalboard's new denominators (trends/watchlist/overview) had zero assertions; `_seed_from_prior_result`'s sensor compared `iterations` and `simulation` at their defaults, so it passed with the carry line deleted, and now asserts anti-vacuity first; the two `context.get("grade", True)` source greps are replaced by a behavioural test that patches Orchestrator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g diff - write_text_atomic created its temp file 0o666, relying on the umask to reduce it. Under umask 0 that is a world-WRITABLE run record. Readable is the requirement (the host reads a container-written task.json back across the bind mount); writable never was. 0o644 keeps the fix and cannot widen. - The sandbox-escape guard logged a RESOLVED absolute path, which CodeQL reads as clear-text sensitive data. It was also the wrong string and the wrong place: the resolved path is just the author's own pattern joined onto a tempdir, and reporting from _within_sandbox fired once per rejected glob match. resolve_files now reports ONCE per criterion, naming the pattern the task author actually wrote. - Dropped a redundant local `import re` in tests/test_custom_lint.py. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| # predecessor, so O_EXCL can never collide with our own leftovers. | ||
| tmp = path.with_name(f"{path.name}.{os.getpid()}.{secrets.token_hex(4)}.tmp") | ||
| flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0) | ||
| fd = os.open(tmp, flags, 0o644) |
| def _warn_escaped(self, path: str) -> None: | ||
| """Report that a criterion's declared path left the sandbox.""" | ||
| logger.warning( | ||
| "Criterion path %r resolves outside the sandbox (%s); treating it as no match.", path, self.sandbox_dir |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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).1.
coder-eval executecoder-eval runwith the grading half removed. The agent runs and the full trajectory lands intask.jsonas usual, but no criterion is checked,weighted_scorestaysNone, and the row finalizes as the newFinalStatus.NOT_GRADED.NOT_GRADEDis a fourth reporting categorycategory == "ungraded"— not a fold into the existing three. Folding intofailedwould depress every pass rate, intosucceededwould invent verdicts, intoerrorwould report a healthy run as broken.Ungraded rows leave both sides of every rate:
RunSummary/VariantAggregatepass_rateanderror_sharenow divide bytasks_graded(tasks_run - tasks_not_graded), identical totasks_runfor any graded run.tasks_not_gradedis part of the sum-to-tasks_runinvariant, not atasks_failedsub-counter, and is defaulted so existingrun.json/experiment.jsonstill parse.weighted_scoreis set toNoneexplicitly rather than left tocalculate_weighted_score, which writes0.0for an empty results list — indistinguishable from "graded and scored zero", and every downstreamscore or 0.0would launder it into a real-looking failure.Only
SUCCESS/FAILUREcollapse into it.ERROR,TIMEOUT,BUILD_FAILED,MAX_TURNS_EXHAUSTEDand the budget stops are facts about the run, not about grading — they still apply, andexecutestill exits non-zero on a crash.The switch
BatchRunConfig.grade→Orchestrator(grade=...), gating all four grading call sites. It crosses the docker boundary incontext.json, defaulting toTruein-container so a host predatingexecutekeeps grading.Deliberately not a task-config field: no 5-layer merge, no
-Dpath. A task YAML must never declare itself ungraded; only the invoking command decides.runandexecuteshare 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
--junit-xmlreports_junitstill emits<skipped>for an ungraded row met elsewhere.)stop_early:--resumeis supported — see part 4.2. Detached grading —
evaluate <run_dir>evaluatenow takes two shapes, told apart by a pure resolver (cli/evaluate_target.py) on one probe: a target holdingtask.jsonis 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.resolvedis post-merge, so variant overrides,-Dflags and dataset row expansion are already baked in — re-loading the source would silently grade a different task. Fallback tosource_filehappens only whenresolvedno longer validates, and says so loudly.Orchestrator(prior_result=...)seeds the fresh result, carrying:command_stats,model_used) recomputes fromiterations, so seeding it reproduces them exactly;iteration_count, which evaluate-only used to flatten to1;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;max_turns_exhausted,error_message,sdk_options).Grader-host
environment_infois preserved under agraded_bysub-dict rather than overwriting the run's.Two further parity fixes, both closing gaps the code already knew about:
command_base_pathis 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 resolvesrun_commandbinaries against ambient PATH and can disagree with the run it grades._join_litellm_actual_costskips whenprior_resultis 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_digestmismatch — 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 astask.execute.json. That in-place write is what makes plaincoder-eval aggregate <run_dir>rebuild a gradedrun.jsonwith zero new code.3.
Sandbox.adopt— and the pre-existing bug it fixesadopt(workspace)reusessetup's adoption half but skips every materializing step (_setup_template,_generate_cli_recorders, venv/package installs, the destructive$HOMEremediation), running only non-mutating derivation: mock-dir+x, venv discovery, plugin-tools pin._cleanup_on_exitstays False, so an adopted tree is never moved or deleted.In-place is more correct, not merely faster.
_setup_templatefilters its copy through_should_ignore_template_file, whose default list dropsnode_modules,dist,build,.venv,.git. Soevaluatetoday scores a file that is plainly there as missing: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/--copyoverride.adopthard-errors ondriver: docker: a container workspace is unreachable from the host, so adopting one would grade whatever happens to sit at that host path.4.
--resumenow distinguishes "executed" from "graded"--resumedecided a task was finished by asking "doestask.jsoncarry anyfinal_status".NOT_GRADEDis a final status, sorun --resumeover an executed run reported the tasks complete, graded nothing, and exited 0:"Finished" is relative to the resuming command.
partition_for_resume(tasks, *, grade)returns a four-wayResumePartition:run --resumeexecute --resumetask.json, unreadable, or nofinal_statusNOT_GRADEDFAILURE/ERRORA
NOT_GRADEDrow owesexecutenothing but owesruna grade, sorun --resumeruns 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_artifactsskipsto_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 fromrun.json.gradeis exempt from the config-drift warning:execute→run --resumeis a supported flow, and that warning's "already-finalized tasks keep their original-config results" text is actively wrong for it.execute --resumeis consequently supported and no longer refused.orchestration/regrade.pyis the single implementation shared by this path andevaluate'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_secondsfeedsaverage_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'sstarted_atandduration_seconds; the grading cost is preserved separately asenvironment_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_badgecategory tests, the published-action "everyFinalStatusmust be classified" test, and CE018's enum-parity check.<skipped>(already counted by_set_counts).ungradedonly.Not Gradedline; the pass rate readsn/afor a fully ungraded run instead of0.0%. An ordinary empty run keeps its original0/0rendering — different facts.average_scoremeans over graded rows only;_pick_worst_statusranks ungraded least-urgent so any real verdict wins.NOT_GRADEDhard-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.statusCategory—NOT_GRADED→"unknown", the category every consumer already treats as "no verdict here".Also:
evaluate's Typer command is now a thin wrapper overrun_evaluation(...)with real Python defaults — the same splitrun/executeuse. Calling a Typer command in-process hands unspecified options anOptionInfosentinel, which silently madein_place=Nonetruthy.Verification
make verifygreen (4612 passed, 92.07%) andmake evalboard-verifygreen (608 tests).The headline test asserts
execute+evaluatereaches the same status, score and per-criterion results as a singlerun— compared against a realrunrather than hardcoded values, so a change breaking both paths still fails. Alongside it:executethat assertspre_run's file is written, so a merely-skipped task cannot pass;runstill scores that same task1.0;aggregaterebuilds a gradedrun.jsonunaided; the trajectory survives the re-grade; the adopted workspace is not moved or deleted;task.execute.jsonpreserves the ungraded record;adoptwrites nothing, deletes nothing, and exposes the filtered directories;context.jsonround-trip, andrun/executesignature parity.Scoped out
Relaxing the non-empty
success_criteriavalidator.executeon 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