From 24b2ab65ddde0d231a9fddb6adc28c286bdc09e0 Mon Sep 17 00:00:00 2001 From: rockymadden Date: Thu, 24 Sep 2026 09:17:07 -0600 Subject: [PATCH 1/3] feat(criteria): let a run_command criterion opt in to post-failure grading An agent turn timeout drops every `run_command` criterion, so a task whose graders are pure artifact checks scores 0.00 on artifacts that are correct. `read_only: true` declares one command an artifact-only check and admits it to the post-failure diagnostic pass. It defaults to false, so no existing task changes behaviour, and the result stays diagnostic: an ERROR run keeps its status and its 0.0 canonical score. Co-Authored-By: Claude Opus 5 (1M context) --- docs/REPORT_SCHEMA.md | 7 +-- docs/TASK_DEFINITION_GUIDE.md | 13 +++++ plugins/coder-eval/reference/criteria.md | 1 + src/coder_eval/models/criteria.py | 31 +++++++++++- src/coder_eval/orchestrator.py | 16 +++--- tests/test_success_criterion_union.py | 17 +++++++ tests/test_timeout_orchestrator.py | 63 ++++++++++++++++++++++++ 7 files changed, 137 insertions(+), 11 deletions(-) diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index 472287d9b..35117c83d 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -191,9 +191,10 @@ fields so subclass keys round-trip. When an agent crashes or its turn times out, coder-eval runs only deterministic, read-only artifact criteria while the sandbox is still live: `file_exists`, `file_contains`, `file_matches_regex`, `file_check`, `json_check`, -`reference_comparison`, and `classification_match`. Judges, trajectory checks, -`run_command`, and `uipath_eval` are recorded with -`evaluation_status="not_evaluated"`; they are not invoked on this recovery path. +`reference_comparison`, and `classification_match`. A `run_command` criterion joins them +only when the task author sets [`read_only: true`](TASK_DEFINITION_GUIDE.md#run_command) +on it. Judges, trajectory checks, plain `run_command`, and `uipath_eval` are recorded +with `evaluation_status="not_evaluated"`; they are not invoked on this recovery path. The diagnostic list is additive evidence. An `ERROR` run remains `ERROR`, and its canonical score remains 0.0. diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index d0a300103..afc82fa66 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -904,6 +904,12 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary expected_stdout: "Hello, World!" # Optional: check stdout content stdout_match: "exact" # "exact" (default), "contains", or "regex" description: "Script must output the correct text" + +# Graded even when the agent's turn times out or the agent crashes +- type: "run_command" + command: "python graders/check_flow.py" + read_only: true # declaration, not enforcement -- see below + description: "Flow must contain the approval node" ``` | Field | Default | Description | @@ -914,6 +920,13 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary | `expected_stdout` | `null` | When set, stdout is also checked | | `stdout_match` | `"exact"` | Match mode: `exact` (stripped), `contains` (substring), `regex` (pattern) | | `score_from_stdout` | `false` | Read a float score (0.0–1.0) from the first stdout line (remaining lines become details); a non-zero exit code or a parse failure scores 0.0. Mutually exclusive with `expected_stdout`. | +| `read_only` | `false` | Declares the command inspects artifacts only. Its sole effect: the criterion is also graded after a terminal agent failure — see [Post-failure criterion evidence](REPORT_SCHEMA.md#post-failure-criterion-evidence). | + +`read_only` is an author declaration, **not** a restriction. coder-eval cannot decide +whether a shell command is pure, so it verifies nothing and confines nothing: the +command runs exactly as it always does. Set it only when the command reads artifacts and +nothing else. Leave it `false` when the command writes state or calls a live service (a +`uip maestro flow debug` grader starts a real cloud job, so it must stay `false`). ### `file_matches_regex` diff --git a/plugins/coder-eval/reference/criteria.md b/plugins/coder-eval/reference/criteria.md index 4267c0e57..33835d424 100644 --- a/plugins/coder-eval/reference/criteria.md +++ b/plugins/coder-eval/reference/criteria.md @@ -249,6 +249,7 @@ Optional: | `expected_stdout` | Expected stdout content. When set, stdout is also checked. | | `stdout_match` | How to match stdout: 'exact' (stripped), 'contains' (substring), 'regex' (pattern) | | `score_from_stdout` | When true, read a float score (0.0-1.0) from the first line of stdout. Remaining lines are captured as details. Non-zero exit code or parse failure -> score 0.0. Mutually exclusive with expected_stdout. | +| `read_only` | The task author's DECLARATION that this command only inspects artifacts: it writes nothing, reaches no live service, and returns the same verdict every run. Its only effect is eligibility -- the criterion is also graded after a terminal agent failure (turn timeout or agent crash), while the sandbox is still live, so a timed-out run records what the artifacts were worth. That result is diagnostic and never moves final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command is not decidable here, the command runs exactly as it would normally, and nothing stops a mutating command from being marked. Leave it false for anything that writes state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job). | ### `skill_triggered` diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index e4ee9764d..a9394a15d 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -140,7 +140,17 @@ class BaseSuccessCriterion(BaseModel, ABC): """True if this criterion requires agent turn records to evaluate correctly.""" supports_post_failure_evaluation: ClassVar[bool] = False - """True for deterministic, read-only artifact checks safe to run after agent failure.""" + """True for criterion TYPES that are always deterministic, read-only artifact checks.""" + + @property + def evaluable_after_agent_failure(self) -> bool: + """True when THIS criterion is safe to run after a terminal agent failure. + + The per-instance answer the orchestrator asks, so a type whose safety + depends on how the criterion is authored (``run_command``) can decide it + from its own fields instead of from the class. + """ + return self.supports_post_failure_evaluation @property def is_stop_armed(self) -> bool: @@ -398,6 +408,25 @@ class RunCommandCriterion(BaseSuccessCriterion): "Mutually exclusive with expected_stdout." ), ) + read_only: bool = Field( + default=False, + description=( + "The task author's DECLARATION that this command only inspects artifacts: it writes " + "nothing, reaches no live service, and returns the same verdict every run. Its only " + "effect is eligibility -- the criterion is also graded after a terminal agent failure " + "(turn timeout or agent crash), while the sandbox is still live, so a timed-out run " + "records what the artifacts were worth. That result is diagnostic and never moves " + "final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command " + "is not decidable here, the command runs exactly as it would normally, and nothing " + "stops a mutating command from being marked. Leave it false for anything that writes " + "state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job)." + ), + ) + + @property + def evaluable_after_agent_failure(self) -> bool: + """Author-declared, not proven -- see ``read_only``.""" + return self.read_only @model_validator(mode="after") def check_score_from_stdout_exclusivity(self) -> RunCommandCriterion: diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index eccfd7f15..22cf18558 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -862,6 +862,13 @@ def _post_failure_exception_reason(error: Exception) -> str: suffix = f": {message}" if message else "" return f"post-failure grading could not complete ({type(error).__name__}{suffix})" + @staticmethod + def _unavailable_reason(criterion: SuccessCriterion) -> str: + reason = "the criterion is not a deterministic, read-only artifact check" + if criterion.type == "run_command": + reason += " (declare 'read_only: true' on it if the command only inspects artifacts)" + return reason + @staticmethod def _not_evaluated_result(criterion: SuccessCriterion, reason: str) -> CriterionResult: return CriterionResult( @@ -903,7 +910,7 @@ async def _evaluate_post_failure_criteria(self) -> None: runnable: list[SuccessCriterion] = [] unavailable_positions: set[int] = set() for position, criterion in enumerate(self.task.success_criteria): - if not criterion.supports_post_failure_evaluation: + if not criterion.evaluable_after_agent_failure: unavailable_positions.add(position) else: runnable.append(criterion) @@ -925,12 +932,7 @@ async def _evaluate_post_failure_criteria(self) -> None: recovered: CriteriaResults = [] for position, criterion in enumerate(self.task.success_criteria): if position in unavailable_positions: - recovered.append( - self._not_evaluated_result( - criterion, - "the criterion is not a deterministic, read-only artifact check", - ) - ) + recovered.append(self._not_evaluated_result(criterion, self._unavailable_reason(criterion))) else: recovered.append(next(checked_iter)) self.result.post_failure_criteria_results = recovered diff --git a/tests/test_success_criterion_union.py b/tests/test_success_criterion_union.py index 3cde2e9df..75c9f0774 100644 --- a/tests/test_success_criterion_union.py +++ b/tests/test_success_criterion_union.py @@ -122,6 +122,23 @@ def test_model_dump_exclude_unset_round_trip(): assert round_tripped.success_criteria[0].type == "file_exists" +def test_read_only_survives_exclude_unset_round_trip(): + """``read_only`` gates post-failure grading, so a dropped flag silently loses a score.""" + task = _make_task([{"type": "run_command", "description": "d", "command": "true", "read_only": True}]) + + round_tripped = TaskDefinition.model_validate(task.model_dump(exclude_unset=True)) + + criterion = round_tripped.success_criteria[0] + assert criterion.read_only is True + assert criterion.evaluable_after_agent_failure is True + + +def test_run_command_is_not_post_failure_evaluable_by_default(): + task = _make_task([{"type": "run_command", "description": "d", "command": "true"}]) + + assert task.success_criteria[0].evaluable_after_agent_failure is False + + def test_validate_registry_passes(): CriterionRegistry.discover() validate_registry() diff --git a/tests/test_timeout_orchestrator.py b/tests/test_timeout_orchestrator.py index 080113c79..daa02c202 100644 --- a/tests/test_timeout_orchestrator.py +++ b/tests/test_timeout_orchestrator.py @@ -429,6 +429,69 @@ async def cleanup() -> None: ] +@pytest.mark.asyncio +async def test_read_only_run_command_is_graded_after_a_terminal_agent_error(tmp_path) -> None: + """A declared read-only grader records artifact truth without rescuing the run.""" + task = _make_task(turn_timeout=1200, task_timeout=1500) + task.success_criteria = [ + RunCommandCriterion( + type="run_command", + command="test -f artifact.txt", + read_only=True, + description="declared read-only grader", + ), + RunCommandCriterion( + type="run_command", + command="touch should-not-run", + description="undeclared sandbox command", + ), + ] + run_dir = tmp_path / "run" / "read_only_post_failure" + run_dir.mkdir(parents=True) + orchestrator = Orchestrator(task=task, run_dir=run_dir, variant_id="test-variant") + orchestrator._setup = AsyncMock() # type: ignore[method-assign] + orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] + terminal_error = TurnTimeoutError(1200, task_id=task.task_id, iteration=1) + orchestrator._evaluation_loop = AsyncMock(side_effect=terminal_error) # type: ignore[method-assign] + + sandbox = Sandbox(SandboxConfig(driver="tempdir"), task_id=task.task_id) + sandbox_dir = sandbox.setup() + (sandbox_dir / "artifact.txt").write_text("finished", encoding="utf-8") + orchestrator.sandbox = sandbox + orchestrator.success_checker = SuccessChecker(sandbox) + + # Recorded before teardown removes the directory; asserting inside `cleanup` + # would hide the failure in the orchestrator's finally block. + marker_seen: list[bool] = [] + + async def cleanup() -> None: + marker_seen.append((sandbox_dir / "should-not-run").exists()) + sandbox.cleanup() + + orchestrator._cleanup = cleanup # type: ignore[method-assign] + + mock_agent = MagicMock() + mock_agent.kill_sync = MagicMock() + mock_agent.get_sdk_options = MagicMock(return_value=None) + orchestrator.agent = mock_agent + + result = await orchestrator.run() + + assert marker_seen == [False], "an undeclared run_command must not execute on this path" + + declared, undeclared = result.post_failure_criteria_results + assert declared.evaluation_status == "evaluated" + assert declared.score == 1.0 + assert undeclared.evaluation_status == "not_evaluated" + assert "read_only: true" in (undeclared.details or "") + + # The evidence is additive: the run is still the failure it was. + assert result.final_status == "ERROR" + assert result.error_message == str(terminal_error) + assert result.weighted_score == 0.0 + assert result.success_criteria_results == [] + + @pytest.mark.parametrize( "recovery_error", [ From fbf44b80da7204af4bfd3d272a73c62368b38173 Mon Sep 17 00:00:00 2001 From: rockymadden Date: Thu, 24 Sep 2026 10:07:50 -0600 Subject: [PATCH 2/3] docs(criteria): name the budget-breach trigger on the post-failure path `_run_evaluation_with_failure_evidence` catches BudgetExceededError alongside AgentCrashError and TurnTimeoutError, so a `read_only` command also runs after a token or cost breach that stopped grading part-way. The field description, both guides and the method docstring said otherwise. Co-Authored-By: Claude Opus 5 (1M context) --- docs/REPORT_SCHEMA.md | 5 +++-- docs/TASK_DEFINITION_GUIDE.md | 4 ++-- plugins/coder-eval/reference/criteria.md | 2 +- src/coder_eval/models/criteria.py | 8 +++++--- src/coder_eval/orchestrator.py | 6 ++++-- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index 35117c83d..48df545b3 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -188,8 +188,9 @@ fields so subclass keys round-trip. ### Post-failure criterion evidence -When an agent crashes or its turn times out, coder-eval runs only deterministic, -read-only artifact criteria while the sandbox is still live: `file_exists`, +When an agent crashes, its turn times out, or a token/cost budget breach stops grading +part-way, coder-eval runs only deterministic, read-only artifact criteria while the +sandbox is still live: `file_exists`, `file_contains`, `file_matches_regex`, `file_check`, `json_check`, `reference_comparison`, and `classification_match`. A `run_command` criterion joins them only when the task author sets [`read_only: true`](TASK_DEFINITION_GUIDE.md#run_command) diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index afc82fa66..cd4aa5832 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -905,7 +905,7 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary stdout_match: "exact" # "exact" (default), "contains", or "regex" description: "Script must output the correct text" -# Graded even when the agent's turn times out or the agent crashes +# Graded even when the turn times out, the agent crashes, or a budget breach cuts grading short - type: "run_command" command: "python graders/check_flow.py" read_only: true # declaration, not enforcement -- see below @@ -920,7 +920,7 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary | `expected_stdout` | `null` | When set, stdout is also checked | | `stdout_match` | `"exact"` | Match mode: `exact` (stripped), `contains` (substring), `regex` (pattern) | | `score_from_stdout` | `false` | Read a float score (0.0–1.0) from the first stdout line (remaining lines become details); a non-zero exit code or a parse failure scores 0.0. Mutually exclusive with `expected_stdout`. | -| `read_only` | `false` | Declares the command inspects artifacts only. Its sole effect: the criterion is also graded after a terminal agent failure — see [Post-failure criterion evidence](REPORT_SCHEMA.md#post-failure-criterion-evidence). | +| `read_only` | `false` | Declares the command inspects artifacts only. Its sole effect: the criterion is also graded on the post-failure diagnostic path — after a turn timeout, an agent crash, or a budget breach that stopped grading. See [Post-failure criterion evidence](REPORT_SCHEMA.md#post-failure-criterion-evidence). | `read_only` is an author declaration, **not** a restriction. coder-eval cannot decide whether a shell command is pure, so it verifies nothing and confines nothing: the diff --git a/plugins/coder-eval/reference/criteria.md b/plugins/coder-eval/reference/criteria.md index 33835d424..e36f549db 100644 --- a/plugins/coder-eval/reference/criteria.md +++ b/plugins/coder-eval/reference/criteria.md @@ -249,7 +249,7 @@ Optional: | `expected_stdout` | Expected stdout content. When set, stdout is also checked. | | `stdout_match` | How to match stdout: 'exact' (stripped), 'contains' (substring), 'regex' (pattern) | | `score_from_stdout` | When true, read a float score (0.0-1.0) from the first line of stdout. Remaining lines are captured as details. Non-zero exit code or parse failure -> score 0.0. Mutually exclusive with expected_stdout. | -| `read_only` | The task author's DECLARATION that this command only inspects artifacts: it writes nothing, reaches no live service, and returns the same verdict every run. Its only effect is eligibility -- the criterion is also graded after a terminal agent failure (turn timeout or agent crash), while the sandbox is still live, so a timed-out run records what the artifacts were worth. That result is diagnostic and never moves final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command is not decidable here, the command runs exactly as it would normally, and nothing stops a mutating command from being marked. Leave it false for anything that writes state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job). | +| `read_only` | The task author's DECLARATION that this command only inspects artifacts: it writes nothing, reaches no live service, and returns the same verdict every run. Its only effect is eligibility -- the criterion is also graded on the post-failure diagnostic path, while the sandbox is still live, so a run that died mid-flight still records what the artifacts were worth. Three failures reach that path: a turn timeout, an agent crash, and a token or cost budget breach that stopped grading part-way. The command runs on every one of them. That result is diagnostic and never moves final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command is not decidable here, the command runs exactly as it would normally, and nothing stops a mutating command from being marked. Leave it false for anything that writes state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job). | ### `skill_triggered` diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index a9394a15d..d29722322 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -413,9 +413,11 @@ class RunCommandCriterion(BaseSuccessCriterion): description=( "The task author's DECLARATION that this command only inspects artifacts: it writes " "nothing, reaches no live service, and returns the same verdict every run. Its only " - "effect is eligibility -- the criterion is also graded after a terminal agent failure " - "(turn timeout or agent crash), while the sandbox is still live, so a timed-out run " - "records what the artifacts were worth. That result is diagnostic and never moves " + "effect is eligibility -- the criterion is also graded on the post-failure diagnostic " + "path, while the sandbox is still live, so a run that died mid-flight still records " + "what the artifacts were worth. Three failures reach that path: a turn timeout, an " + "agent crash, and a token or cost budget breach that stopped grading part-way. The " + "command runs on every one of them. That result is diagnostic and never moves " "final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command " "is not decidable here, the command runs exactly as it would normally, and nothing " "stops a mutating command from being marked. Leave it false for anything that writes " diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 22cf18558..08ea2a0b1 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -893,8 +893,10 @@ async def _evaluate_post_failure_criteria(self) -> None: """Evaluate diagnostic criteria before the live sandbox is torn down. Results stay outside the canonical scored list. Only criteria that - declare themselves deterministic and read-only run on this path. This - excludes judges and checks that execute sandbox commands. + declare themselves deterministic and read-only run on this path. That + excludes judges and trajectory checks, and every ``run_command`` + criterion except one the task author marked ``read_only`` -- which DOES + execute a sandbox command here. """ if self.result is None: return From e5f2bfeba8cac00bf1b108d715c28393d7566e32 Mon Sep 17 00:00:00 2001 From: rockymadden Date: Thu, 24 Sep 2026 11:55:16 -0600 Subject: [PATCH 3/3] fix(docs): drop the budget-breach trigger, cut read_only to its contract A budget breach cannot reach the post-failure path. `_check_run_limits` runs after `check_all_async` on the graded single-shot path, so the canonical vector is complete and the guard in `_run_evaluation_with_failure_evidence` re-raises; the dialog site catches the error locally; `execute` returns early. The previous commit claimed diagnostics that are never written. Also: the Field description no longer overclaims `execute` and is cut to the contract, `_unavailable_reason` narrows with isinstance like regrade.py and tasks.py, and the guide tells authors to keep a read_only timeout short. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/notes/orchestration.md | 17 ++++++++++++++++ docs/REPORT_SCHEMA.md | 9 ++++---- docs/TASK_DEFINITION_GUIDE.md | 10 +++++++-- plugins/coder-eval/reference/criteria.md | 2 +- plugins/coder-eval/skills/task/SKILL.md | 4 ++++ src/coder_eval/models/criteria.py | 19 +++++++---------- src/coder_eval/orchestrator.py | 7 ++++++- tests/test_success_criterion_union.py | 8 ++++++++ tests/test_timeout_orchestrator.py | 26 +++++++++++++++++++++--- 9 files changed, 79 insertions(+), 23 deletions(-) diff --git a/.claude/notes/orchestration.md b/.claude/notes/orchestration.md index a37c65882..7a65e2241 100644 --- a/.claude/notes/orchestration.md +++ b/.claude/notes/orchestration.md @@ -108,6 +108,23 @@ VERDICT, never the facts — the seeding cannot restore a fact the execute phase captured. The budget gate runs AFTER the criteria on the graded path purely for partial-credit visibility, and there is no partial credit under `execute`. +### Post-failure evidence is declared, not proven + +A timed-out run used to score 0.00 on artifacts that every grader accepted, because the +whole uipath-maestro-flow suite grades through `run_command` and the path ran only file +checks. `read_only: true` admits a single named command. + +The gate could not stay a ClassVar: `run_command` is safe or unsafe per criterion, not +per type, and no analysis decides which — an arbitrary shell command can start a cloud +job (`uip maestro flow debug` does). So the flag is the AUTHOR's claim, the ClassVar +stayed the type-level answer, and `evaluable_after_agent_failure` became the per-instance +one every caller asks. Widening the flag to the base would let a judge declare itself +deterministic, which is a different and false claim. + +What keeps it honest: results land in `post_failure_criteria_results`, which +`calculate_weighted_score` never reads. A timed-out run stays ERROR at 0.0 no matter what +the diagnostic pass finds — the flag buys evidence, never a verdict. + ### Rates need verdict evidence, not bucket counts A published rate divides by rows that actually carry a verdict, not by a bucket count. The diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index 48df545b3..f7002e3ec 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -188,16 +188,17 @@ fields so subclass keys round-trip. ### Post-failure criterion evidence -When an agent crashes, its turn times out, or a token/cost budget breach stops grading -part-way, coder-eval runs only deterministic, read-only artifact criteria while the -sandbox is still live: `file_exists`, +When an agent crashes or its turn times out on a graded run, coder-eval runs only +deterministic, read-only artifact criteria while the sandbox is still live: `file_exists`, `file_contains`, `file_matches_regex`, `file_check`, `json_check`, `reference_comparison`, and `classification_match`. A `run_command` criterion joins them only when the task author sets [`read_only: true`](TASK_DEFINITION_GUIDE.md#run_command) on it. Judges, trajectory checks, plain `run_command`, and `uipath_eval` are recorded with `evaluation_status="not_evaluated"`; they are not invoked on this recovery path. The diagnostic list is additive evidence. An `ERROR` run remains `ERROR`, and its -canonical score remains 0.0. +canonical score remains 0.0. The list stays empty under `coder-eval execute`, and on a +token/cost budget breach, which fires only after every criterion is already scored in +`success_criteria_results`. ### TurnRecord diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index cd4aa5832..59d613a81 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -905,7 +905,7 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary stdout_match: "exact" # "exact" (default), "contains", or "regex" description: "Script must output the correct text" -# Graded even when the turn times out, the agent crashes, or a budget breach cuts grading short +# Also graded when the turn times out or the agent crashes - type: "run_command" command: "python graders/check_flow.py" read_only: true # declaration, not enforcement -- see below @@ -920,7 +920,7 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary | `expected_stdout` | `null` | When set, stdout is also checked | | `stdout_match` | `"exact"` | Match mode: `exact` (stripped), `contains` (substring), `regex` (pattern) | | `score_from_stdout` | `false` | Read a float score (0.0–1.0) from the first stdout line (remaining lines become details); a non-zero exit code or a parse failure scores 0.0. Mutually exclusive with `expected_stdout`. | -| `read_only` | `false` | Declares the command inspects artifacts only. Its sole effect: the criterion is also graded on the post-failure diagnostic path — after a turn timeout, an agent crash, or a budget breach that stopped grading. See [Post-failure criterion evidence](REPORT_SCHEMA.md#post-failure-criterion-evidence). | +| `read_only` | `false` | Declares the command inspects artifacts only. Its sole effect: a graded run also runs the criterion after a turn timeout or an agent crash — see [Post-failure criterion evidence](REPORT_SCHEMA.md#post-failure-criterion-evidence). | `read_only` is an author declaration, **not** a restriction. coder-eval cannot decide whether a shell command is pure, so it verifies nothing and confines nothing: the @@ -928,6 +928,12 @@ command runs exactly as it always does. Set it only when the command reads artif nothing else. Leave it `false` when the command writes state or calls a live service (a `uip maestro flow debug` grader starts a real cloud job, so it must stay `false`). +Keep a `read_only` criterion's `timeout` short. The diagnostic pass runs after the agent +is gone, and the commands are not interruptible: a `task_timeout` that expires mid-pass +cancels the await, not the shell subprocess, so it keeps running while the sandbox is +torn down. Without a `task_timeout` the pass is bounded only by the sum of these +timeouts. + ### `file_matches_regex` Checks if file content matches a regular expression pattern. **Binary scoring.** diff --git a/plugins/coder-eval/reference/criteria.md b/plugins/coder-eval/reference/criteria.md index e36f549db..526e1fbdd 100644 --- a/plugins/coder-eval/reference/criteria.md +++ b/plugins/coder-eval/reference/criteria.md @@ -249,7 +249,7 @@ Optional: | `expected_stdout` | Expected stdout content. When set, stdout is also checked. | | `stdout_match` | How to match stdout: 'exact' (stripped), 'contains' (substring), 'regex' (pattern) | | `score_from_stdout` | When true, read a float score (0.0-1.0) from the first line of stdout. Remaining lines are captured as details. Non-zero exit code or parse failure -> score 0.0. Mutually exclusive with expected_stdout. | -| `read_only` | The task author's DECLARATION that this command only inspects artifacts: it writes nothing, reaches no live service, and returns the same verdict every run. Its only effect is eligibility -- the criterion is also graded on the post-failure diagnostic path, while the sandbox is still live, so a run that died mid-flight still records what the artifacts were worth. Three failures reach that path: a turn timeout, an agent crash, and a token or cost budget breach that stopped grading part-way. The command runs on every one of them. That result is diagnostic and never moves final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command is not decidable here, the command runs exactly as it would normally, and nothing stops a mutating command from being marked. Leave it false for anything that writes state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job). | +| `read_only` | Declares this command an artifact-only check: it writes nothing and reaches no live service. Its one effect is that a graded run also runs it after an agent crash or turn timeout, on the diagnostic path, where the result is recorded but never scored. Nothing verifies the declaration; see the Task Definition Guide for when to set it. | ### `skill_triggered` diff --git a/plugins/coder-eval/skills/task/SKILL.md b/plugins/coder-eval/skills/task/SKILL.md index 7226cdeaa..440de214f 100644 --- a/plugins/coder-eval/skills/task/SKILL.md +++ b/plugins/coder-eval/skills/task/SKILL.md @@ -120,6 +120,10 @@ Rules that matter: filename — a criterion matching that literal is a **smoke check**, not evidence: it only proves the agent typed back what it was told. Keep it if you like, at a low weight, and put the weight on a criterion that checks the resulting *behaviour*. +- Set `read_only: true` on a `run_command` grader that only inspects artifacts: a run + whose agent crashed or timed out then still records what the artifacts were worth, + instead of scoring nothing. Never set it on a command that writes state or calls a + live service — nothing verifies the claim, and the command runs unchanged. - `weight` reflects importance: `0.5` nice-to-have, `1.0` standard, `1.5`–`2.0` critical. `weight: 0` makes a criterion informational (reported, but excluded from the score and the pass/fail gate). diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index d29722322..1e661aa73 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -140,7 +140,9 @@ class BaseSuccessCriterion(BaseModel, ABC): """True if this criterion requires agent turn records to evaluate correctly.""" supports_post_failure_evaluation: ClassVar[bool] = False - """True for criterion TYPES that are always deterministic, read-only artifact checks.""" + """Type-level only: True for criterion TYPES that are always deterministic, read-only + artifact checks. It cannot answer for an instance -- ``run_command`` decides per + criterion -- so every caller asks ``evaluable_after_agent_failure`` instead.""" @property def evaluable_after_agent_failure(self) -> bool: @@ -411,17 +413,10 @@ class RunCommandCriterion(BaseSuccessCriterion): read_only: bool = Field( default=False, description=( - "The task author's DECLARATION that this command only inspects artifacts: it writes " - "nothing, reaches no live service, and returns the same verdict every run. Its only " - "effect is eligibility -- the criterion is also graded on the post-failure diagnostic " - "path, while the sandbox is still live, so a run that died mid-flight still records " - "what the artifacts were worth. Three failures reach that path: a turn timeout, an " - "agent crash, and a token or cost budget breach that stopped grading part-way. The " - "command runs on every one of them. That result is diagnostic and never moves " - "final_status or weighted_score. UNVERIFIED AND UNENFORCED: purity of a shell command " - "is not decidable here, the command runs exactly as it would normally, and nothing " - "stops a mutating command from being marked. Leave it false for anything that writes " - "state or calls a live tenant (e.g. 'uip maestro flow debug', which starts a cloud job)." + "Declares this command an artifact-only check: it writes nothing and reaches no live " + "service. Its one effect is that a graded run also runs it after an agent crash or " + "turn timeout, on the diagnostic path, where the result is recorded but never scored. " + "Nothing verifies the declaration; see the Task Definition Guide for when to set it." ), ) diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 08ea2a0b1..c10f1deb2 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -54,6 +54,7 @@ PreRunCommand, PreservationMode, ReferenceComparisonCriterion, + RunCommandCriterion, SimulationConfig, SimulationTelemetry, SuccessCriterion, @@ -865,7 +866,7 @@ def _post_failure_exception_reason(error: Exception) -> str: @staticmethod def _unavailable_reason(criterion: SuccessCriterion) -> str: reason = "the criterion is not a deterministic, read-only artifact check" - if criterion.type == "run_command": + if isinstance(criterion, RunCommandCriterion): reason += " (declare 'read_only: true' on it if the command only inspects artifacts)" return reason @@ -897,6 +898,10 @@ async def _evaluate_post_failure_criteria(self) -> None: excludes judges and trajectory checks, and every ``run_command`` criterion except one the task author marked ``read_only`` -- which DOES execute a sandbox command here. + + Reached from an agent crash or a turn timeout only. A budget breach + raises after the canonical vector is already complete, so its guard in + ``_run_evaluation_with_failure_evidence`` re-raises before this runs. """ if self.result is None: return diff --git a/tests/test_success_criterion_union.py b/tests/test_success_criterion_union.py index 75c9f0774..778a4dea9 100644 --- a/tests/test_success_criterion_union.py +++ b/tests/test_success_criterion_union.py @@ -139,6 +139,14 @@ def test_run_command_is_not_post_failure_evaluable_by_default(): assert task.success_criteria[0].evaluable_after_agent_failure is False +@pytest.mark.parametrize("tag", sorted(MINIMAL_PAYLOADS)) +def test_post_failure_property_tracks_the_type_answer(tag: str): + """Only ``run_command`` may diverge from its ClassVar, and only via ``read_only``.""" + criterion = _make_task([{"type": tag, **MINIMAL_PAYLOADS[tag]}]).success_criteria[0] + + assert criterion.evaluable_after_agent_failure is criterion.supports_post_failure_evaluation + + def test_validate_registry_passes(): CriterionRegistry.discover() validate_registry() diff --git a/tests/test_timeout_orchestrator.py b/tests/test_timeout_orchestrator.py index daa02c202..ec0565787 100644 --- a/tests/test_timeout_orchestrator.py +++ b/tests/test_timeout_orchestrator.py @@ -440,11 +440,22 @@ async def test_read_only_run_command_is_graded_after_a_terminal_agent_error(tmp_ read_only=True, description="declared read-only grader", ), + RunCommandCriterion( + type="run_command", + command="test -f missing.txt", + read_only=True, + description="declared read-only grader that fails", + ), RunCommandCriterion( type="run_command", command="touch should-not-run", description="undeclared sandbox command", ), + LLMJudgeCriterion( + type="llm_judge", + prompt="Grade the artifact.", + description="paid judge", + ), ] run_dir = tmp_path / "run" / "read_only_post_failure" run_dir.mkdir(parents=True) @@ -479,11 +490,20 @@ async def cleanup() -> None: assert marker_seen == [False], "an undeclared run_command must not execute on this path" - declared, undeclared = result.post_failure_criteria_results - assert declared.evaluation_status == "evaluated" - assert declared.score == 1.0 + passed, failed, undeclared, judge = result.post_failure_criteria_results + assert passed.evaluation_status == "evaluated" + assert passed.score == 1.0 + + # A real failing verdict, NOT the not_evaluated placeholder -- both score 0.0. + assert failed.evaluation_status == "evaluated" + assert failed.score == 0.0 + assert "Not evaluated after terminal agent failure" not in (failed.details or "") + + # The opt-in hint belongs only to the type that has the opt-in. assert undeclared.evaluation_status == "not_evaluated" assert "read_only: true" in (undeclared.details or "") + assert judge.evaluation_status == "not_evaluated" + assert "read_only: true" not in (judge.details or "") # The evidence is additive: the run is still the failure it was. assert result.final_status == "ERROR"