diff --git a/dokimi/changelog.d/20261113_120500_napetrov_fsm687-verifier.md b/dokimi/changelog.d/20261113_120500_napetrov_fsm687-verifier.md new file mode 100644 index 000000000..b476f0864 --- /dev/null +++ b/dokimi/changelog.d/20261113_120500_napetrov_fsm687-verifier.md @@ -0,0 +1,8 @@ + + +### Changed + +- **A `docker-*` reward-file verifier must write `reward.txt`: exit 0 alone is no longer a pass** — a Docker task (`task.toml` or `task.md`) whose verifier wrote no `/logs/verifier/reward.txt` used to score 1.0 on exit code 0. The cell is now errored, with no reward, and `run.log` names the missing file. A verifier that exits non-zero without a reward file still scores 0.0. A terminal-bench v1 task keeps its pytest summary as the verdict. A `reward.txt` with no number between 0 and 1 in it (`PASS`, empty, `75`) now errors the cell too, instead of scoring a fail, and so does a `docker cp` of it that fails or times out (the timeout used to score 0.0). Make such a verifier write `1` or `0` to `/logs/verifier/reward.txt`; pass rates of tasks whose verifier only set its exit code are not comparable with earlier runs. diff --git a/dokimi/data/skills/dokimi-design-evals/references/verifier-self-test.md b/dokimi/data/skills/dokimi-design-evals/references/verifier-self-test.md index a1290a9e9..19e60ee9b 100644 --- a/dokimi/data/skills/dokimi-design-evals/references/verifier-self-test.md +++ b/dokimi/data/skills/dokimi-design-evals/references/verifier-self-test.md @@ -130,8 +130,8 @@ reference state and on the state before it, and expect pass then fail. To run a `tests/test.sh` by hand, put that step's `tests/` at `/tests` and at `/verifier` in the task image and run `mkdir -p /logs/verifier && chmod +x /tests/test.sh && /tests/test.sh`, as the harness does. -Read `/logs/verifier/reward.txt`, as the harness does; the exit code counts only if the -file is missing. +Read `/logs/verifier/reward.txt`, as the harness does; if the file is missing, exit 0 +errors the cell and a non-zero exit is a fail. The task needs a top-level `instruction.md`; without it the task does not load. ## What a passing self-test does not show diff --git a/dokimi/docs/coding-harnesses.md b/dokimi/docs/coding-harnesses.md index 5e3137210..fcff41d0d 100644 --- a/dokimi/docs/coding-harnesses.md +++ b/dokimi/docs/coding-harnesses.md @@ -1100,7 +1100,12 @@ python cli.py tasks run --tasks \ partial credit such as `0.75`, is kept as the reward and counts as a fail. If no reward is found, the adapter's exit code is used as the fallback, except for a Harbor-backed cell, which is then errored (see the Harbor grading - table above). + table above), and a `docker-*` cell with a reward-file verifier (`task.toml` + or `task.md`), where exit 0 without `/logs/verifier/reward.txt` is errored + and a non-zero exit scores 0.0. A terminal-bench v1 verifier keeps its pytest + summary as the verdict. A `docker-*` reward file with no number between 0 + and 1 in it (`PASS`, empty, `75`), or a `docker cp` of it that fails or + times out, errors the cell. - `results[].metrics`: for LLM harnesses (`docker-claude*`), the per-run `UsageRecord` fields — `cost_usd`, `prompt_tokens`, `completion_tokens`, `total_tokens`, `cache_read_tokens`, `cache_write_tokens`, `latency_sec`, diff --git a/dokimi/docs/contributing-terminal-bench-task.md b/dokimi/docs/contributing-terminal-bench-task.md index d5b517736..46105f72f 100644 --- a/dokimi/docs/contributing-terminal-bench-task.md +++ b/dokimi/docs/contributing-terminal-bench-task.md @@ -152,9 +152,11 @@ The contract: - Write the outcome to `/logs/verifier/reward.txt` — `1` (or `1.0`) for pass, `0` (or `0.0`) for fail. A fraction such as `0.75` (partial credit) is kept - as the cell's reward and counts as a fail. The harness copies that file out; - if it is missing it falls back to the verifier's exit code, so also exit - non-zero on failure. + as the cell's reward and counts as a fail. The harness copies that file out. + Without it the exit code is no pass: exit 0 errors the cell and a non-zero + exit scores 0. This is the rule for this format; a terminal-bench v1 task + (`task.yaml`) is still graded by its pytest summary. A file with no number + between 0 and 1 in it (`PASS`, empty, `75`) errors the cell. - Do not let a failure abort before the reward file is written (capture the pytest exit code, write the file, then exit with it). diff --git a/dokimi/dokimi/harnesses/command.py b/dokimi/dokimi/harnesses/command.py index a2da8ace2..ca7fbc562 100644 --- a/dokimi/dokimi/harnesses/command.py +++ b/dokimi/dokimi/harnesses/command.py @@ -443,6 +443,48 @@ def _find_reward(output_dir: Path | None, stdout: str, stderr: str) -> float | N return parsed if parsed is not None else _parse_reward_text(stderr) +def _exit_code_verdict(returncode: int) -> float: + """The verdict of a reward-file verifier that wrote no ``reward.txt``. + + Exit 0 is not a verdict: a verifier that wrote no reward file passed nothing, + so the cell is errored, as a verifier that did not run is. Non-zero stays 0.0. + """ + if returncode == 0: + raise RuntimeError("verifier exited 0 but wrote no /logs/verifier/reward.txt; see run.log") + return 0.0 + + +def _reward_file_verdict(text: str) -> float: + """The reward in a verifier's ``reward.txt``; one with no number in it is no verdict. + + As in :func:`_exit_code_reward`, whatever the exit code: the cell is errored, not failed. + """ + reward = _parse_reward_text(text, reward_file=True) + if reward is None: + raise RuntimeError( + "/logs/verifier/reward.txt has no reward between 0 and 1 in it; see run.log" + ) + return reward + + +# What docker says when the file is not there (v20+, older CLIs, podman); a daemon error says none. +_CP_NO_FILE_RE = re.compile( + r"could not find the file|no such container:path|could not be found on container", re.I) + + +def _check_reward_copy(pulled: process.Completed) -> None: + """Raise unless ``docker cp`` of ``reward.txt`` worked or found no file there. + + A copy that failed or hung says nothing about the verifier, so it is not "no reward file". + """ + if pulled.timed_out: + raise RuntimeError("docker cp of /logs/verifier/reward.txt timed out; see run.log") + if pulled.returncode != 0 and not _CP_NO_FILE_RE.search(pulled.stderr or ""): + raise RuntimeError( + f"docker cp of /logs/verifier/reward.txt failed (exit {pulled.returncode}); see run.log" + ) + + def _parse_reward_text(text: str, *, reward_file: bool = False) -> float | None: """A reward from a ``reward.txt`` (``reward_file``) or from command output. diff --git a/dokimi/dokimi/harnesses/docker_solver.py b/dokimi/dokimi/harnesses/docker_solver.py index e32d3992e..462fc1194 100644 --- a/dokimi/dokimi/harnesses/docker_solver.py +++ b/dokimi/dokimi/harnesses/docker_solver.py @@ -71,7 +71,7 @@ solver_cli_flags, ) from .claude_json import _last_json_object, _parse_claude_json # noqa: F401 -from .command import _parse_reward_text +from .command import _check_reward_copy, _exit_code_verdict, _reward_file_verdict from .container_agent import (AGENT_BIN, DOCKER_STEP_TIMEOUT, FILES_DIR, TASK_CONTAINER_HARDENING, ContainerSandbox, Docker) from .models import HarnessResult, OperationRecord @@ -645,20 +645,16 @@ def _verify(self, task: TaskSpec, container: str, output_dir: Path, log, ["cp", f"{container}:/logs/verifier/reward.txt", str(reward_file)], log, check=False, ) - if pulled.timed_out: # not "no reward file": the exit code would pass a failing verifier - raise subprocess.TimeoutExpired(pulled.argv, DOCKER_STEP_TIMEOUT) if pulled.returncode == 0 and reward_file.is_file(): - return _parse_reward_text( - reward_file.read_text(encoding="utf-8", errors="replace"), reward_file=True - ) + return _reward_file_verdict(reward_file.read_text(encoding="utf-8", errors="replace")) if proc.returncode != 0: # no verdict at all if the container is gone, not a 0 running = self._docker(["inspect", "-f", "{{.State.Running}}", container], log, check=False) if running.timed_out: # no answer is not "stopped": say so, the cell is errored either way raise RuntimeError(f"task container {container} state inspect timed out at verify time; see run.log") if (getattr(running, "stdout", "") or "").strip() != "true": raise RuntimeError(f"task container {container} is not running at verify time; see run.log") - # No reward file written: fall back to verifier exit code. - return 1.0 if proc.returncode == 0 else 0.0 + _check_reward_copy(pulled) # a failed or hung copy errors the cell: it is not "no reward file" + return _exit_code_verdict(proc.returncode) def _plan(self, task: TaskSpec, record: Optional[Path] = None) -> NetworkPlan: return resolve_recorded(task, self.network, agent=self.mode != "oracle", record=record) diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index 5c90ddb1b..7a6a02baf 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,7 +1,7 @@ { "files": { "dokimi/harnesses/benchflow.py": {"lines": 869}, - "dokimi/harnesses/docker_solver.py": {"lines": 897, "rationale": "pre-existing when the gate was introduced"}, + "dokimi/harnesses/docker_solver.py": {"lines": 893, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1031, "rationale": "grew on main in #180 (replay a stored retry) after #93 seeded this baseline; split it in its own PR"}, "dokimi/report/model_compare.py": {"lines": 1161, "rationale": "grew on main in #73 (judge-validity --context stored) before the ratchet landed; split it in its own PR"}, "tests/test_arm_runner.py": {"lines": 1175, "rationale": "pre-existing when the gate was introduced"}, diff --git a/dokimi/tests/test_cleanup_timeouts.py b/dokimi/tests/test_cleanup_timeouts.py index 1296eb8fd..3b47aee57 100644 --- a/dokimi/tests/test_cleanup_timeouts.py +++ b/dokimi/tests/test_cleanup_timeouts.py @@ -253,24 +253,17 @@ def test_credential_file_removal_timeout_aborts_cell(tmp_path): assert len(test_calls) == 0, "Verification should not run after credential removal timeout" -def test_reward_file_copy_timeout_fails_task(tmp_path): - """Docker cp of reward.txt timeout fails the task (reward 0.0, rc 124). - - If the copy of reward.txt from the container times out, the verifier's - decision is lost. The task must fail as reward 0.0 rather than proceeding - with an empty reward file or an incorrect assumption. - """ +@pytest.mark.parametrize("reward", [1.0, 0.0]) +def test_reward_file_copy_timeout_errors_the_cell(tmp_path, reward): + """A hung ``docker cp`` of reward.txt lost the verdict: the cell is errored, not passed or failed.""" task = load_task(_fixture_task(tmp_path)) - - # Mock docker to timeout on cp of reward.txt from container - cli = _pipeline(reward=1.0, answer=lambda call: reply(timed_out=True) if ( + cli = _pipeline(reward=reward, answer=lambda call: reply(timed_out=True) if ( call.argv[1] == "cp" and "reward.txt" in " ".join(call.argv)) else None) result = create_harness("docker-claude", runner=cli).run(task, output_dir=tmp_path / "out") - # The timeout should result in returncode 124 and reward 0.0 - assert result.returncode == 124, f"Should timeout when reward.txt copy times out; got {result.returncode}" - assert result.reward == 0.0, "Should get reward 0.0 when reward.txt copy times out" + assert (result.reward, result.passed) == (None, False) + assert "docker cp of /logs/verifier/reward.txt timed out" in result.metrics["error"] def test_plugin_removal_timeout_aborts_cell(tmp_path): @@ -286,17 +279,6 @@ def test_plugin_removal_timeout_aborts_cell(tmp_path): assert not [c for c in cli.calls if c.argv[1] == "exec" and "test.sh" in " ".join(c.argv)] -def test_reward_copy_timeout_does_not_fall_back_to_the_exit_code(tmp_path): - """A verifier that exits 0 must not pass because the reward.txt copy timed out.""" - task = load_task(_fixture_task(tmp_path)) - cli = _pipeline(reward=0.0, answer=lambda call: reply(timed_out=True) if ( - call.argv[1] == "cp" and "reward.txt" in " ".join(call.argv)) else None) - - result = create_harness("docker-claude", runner=cli).run(task, output_dir=tmp_path / "out") - - assert (result.returncode, result.reward, result.passed) == (124, 0.0, False) - - def test_no_egress_file_when_sidecar_log_unknown(tmp_path): """An unreadable sidecar log writes no egress.jsonl: an empty file reads as a clean run.""" def docker(args, log=None, *, check=True, timeout=None): diff --git a/dokimi/tests/test_docker_task_md.py b/dokimi/tests/test_docker_task_md.py index 2fcf4f0d2..a791a365b 100644 --- a/dokimi/tests/test_docker_task_md.py +++ b/dokimi/tests/test_docker_task_md.py @@ -103,8 +103,9 @@ def _hermetic(tmp_path, monkeypatch): def _capture(stdout="") -> DockerCLI: - """A docker CLI where every call exits 0 and prints ``stdout`` (no reward file).""" - return DockerCLI(answer=lambda call: reply(stdout)) + """A docker CLI where every call exits 0 and prints ``stdout``. The verifier's + ``docker cp`` falls through to the fake's reward file (1.0), so a verify is a verdict.""" + return DockerCLI(answer=lambda call: None if call.argv[1:2] == ("cp",) else reply(stdout)) def _docker_via(cli: DockerCLI): @@ -375,6 +376,8 @@ def respond(call): got = answer(call) if answer else None if got is not None: return got + if call.argv[1:2] == ("cp",): # the verifier's reward file comes from the fake's own cp + return None return reply(image_workdir) if call.argv[0] == "docker" else None return DockerCLI(answer=respond, solver='{"total_cost_usd": 0.42}') diff --git a/dokimi/tests/test_docker_verify_reward.py b/dokimi/tests/test_docker_verify_reward.py new file mode 100644 index 000000000..bfc7fa5b1 --- /dev/null +++ b/dokimi/tests/test_docker_verify_reward.py @@ -0,0 +1,126 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""A docker verifier's exit code is a verdict only with a reward file behind it.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from dokimi.harnesses import load_task + + +def _harbor(root: Path, name: str = "hb-task") -> Path: + task = root / name + for sub in ("environment", "solution", "tests"): + (task / sub).mkdir(parents=True) + (task / "solution" / "solve.sh").write_text("#!/bin/bash\n", encoding="utf-8") + (task / "tests" / "test.sh").write_text("#!/bin/bash\n", encoding="utf-8") + (task / "instruction.md").write_text("Fix it.", encoding="utf-8") + (task / "task.toml").write_text( + '[metadata]\ntags = ["onedal"]\n\n[environment]\nworkdir = "/app"\n', encoding="utf-8" + ) + return task + + +def test_docker_verify_exit_zero_without_a_reward_file_is_an_error_not_a_pass(tmp_path): + import io + + from docker_cli_fake import DockerCLI + from dokimi.harnesses.docker_solver import DockerSolverHarness + + out = tmp_path / "o" + out.mkdir() + task = load_task(_harbor(tmp_path)) + harness = DockerSolverHarness(harness_id="docker-oracle", mode="oracle", + runner=DockerCLI(reward=None)) + # The verifier exited 0 but never wrote reward.txt: no verdict, so the cell is errored. + with pytest.raises(RuntimeError, match="reward.txt"): + harness._verify(task, "c1", out, log=io.StringIO()) + + +def test_docker_verify_nonzero_exit_without_a_reward_file_stays_a_fail(tmp_path): + import io + + from docker_cli_fake import DockerCLI, reply + from dokimi.harnesses.docker_solver import DockerSolverHarness + + out = tmp_path / "o" + out.mkdir() + task = load_task(_harbor(tmp_path)) + + def failing_verifier(call): + if "-lc" in call.argv: + return reply(returncode=1) + return None + + harness = DockerSolverHarness(harness_id="docker-oracle", mode="oracle", + runner=DockerCLI(reward=None, answer=failing_verifier)) + assert harness._verify(task, "c1", out, log=io.StringIO()) == 0.0 + + +@pytest.mark.parametrize("exit_code", [0, 1]) +@pytest.mark.parametrize("text", ["PASS\n", ""]) +def test_docker_verify_a_reward_file_with_no_number_is_an_error(tmp_path, text, exit_code): + import io + + from docker_cli_fake import DockerCLI, reply + from dokimi.harnesses.docker_solver import DockerSolverHarness + + out = tmp_path / "o" + out.mkdir() + task = load_task(_harbor(tmp_path)) + verifier = (lambda call: reply(returncode=exit_code) if "-lc" in call.argv else None) + harness = DockerSolverHarness(harness_id="docker-oracle", mode="oracle", + runner=DockerCLI(reward=text, answer=verifier)) + # A reward file with no reward is no verdict, whatever the exit code (as in a command harness). + with pytest.raises(RuntimeError, match="reward.txt has no reward"): + harness._verify(task, "c1", out, log=io.StringIO()) + + +_NO_FILE = "Error response from daemon: Could not find the file /logs/verifier/reward.txt in container c1" + + +@pytest.mark.parametrize(("stderr", "exit_code", "outcome"), [ + (_NO_FILE, 1, 0.0), # docker's own "could not find the file": still a verifier that wrote none + (_NO_FILE, 0, "wrote no /logs/verifier/reward.txt"), + ("Error response from daemon: connection refused", 0, r"docker cp .* failed \(exit 1\)"), + ("Error response from daemon: connection refused", 1, r"docker cp .* failed \(exit 1\)"), + # podman's wording for a missing file (from memory; no podman on this box): still no reward file. + ('"/logs/verifier/reward.txt" could not be found on container c1: no such file or directory', 1, + 0.0), + ('"/logs/verifier/reward.txt" could not be found on container c1: no such file or directory', 0, + "wrote no /logs/verifier/reward.txt"), + # A missing Docker socket says "no such file or directory" but names no reward file. + ("dial unix /var/run/docker.sock: connect: no such file or directory", 1, + r"docker cp .* failed \(exit 1\)"), + ("dial unix /var/run/docker.sock: connect: no such file or directory", 0, + r"docker cp .* failed \(exit 1\)"), +]) +def test_docker_verify_a_failed_reward_copy_is_not_a_missing_reward_file( + tmp_path, stderr, exit_code, outcome): + import io + + from docker_cli_fake import DockerCLI, reply + from dokimi.harnesses.docker_solver import DockerSolverHarness + + out = tmp_path / "o" + out.mkdir() + task = load_task(_harbor(tmp_path)) + + def answer(call): + if "-lc" in call.argv: + return reply(returncode=exit_code) + if call.argv[1] == "cp" and call.argv[2].endswith(":/logs/verifier/reward.txt"): + return reply(stderr=stderr, returncode=1) + return None + + harness = DockerSolverHarness(harness_id="docker-oracle", mode="oracle", + runner=DockerCLI(reward="1.0", answer=answer)) + if isinstance(outcome, float): + assert harness._verify(task, "c1", out, log=io.StringIO()) == outcome + else: + with pytest.raises(RuntimeError, match=outcome): + harness._verify(task, "c1", out, log=io.StringIO())