[FIX] Forbid emitting silently-unscored single-step trials

Benjamin Feuer · claude · Benjamin Feuer · commit 9203989f34de · 2026-06-17T06:13:53.000-04:00
A finished agentic eval had 63/300 trials finalize in a masked state:
labelled AgentTimeoutError, verifier/ dir empty, verifier_result == None
-- indistinguishable from the 111 trials that timed out but DID verify to
a reward, and silently dropped from the scored denominator at aggregation
(JobStats.increment only counts trials with verifier_result.rewards).

Root cause (confirmed on Jupiter artifacts): after the agent timed out,
exception_info was set to AgentTimeoutError. _run_verification() was then
invoked (its TimingInfo was set) but raised AddTestsDirError uploading the
tests dir into the timed-out sandbox. That exception propagated to the
outer `except Exception` in Trial.run(), where first-write-wins kept the
AgentTimeoutError label and swallowed the verifier-setup failure, leaving
verifier_result == None with no distinct signal.

Fix: enforce a lifecycle invariant in _cleanup_and_finalize (runs on every
path incl. cancellation): for single-step, verification-enabled trials with
no verifier_result, record an explicit, distinctly-typed
VerificationNotCompletedError (chaining any prior exception_info into the
message). Trials that produced a verifier_result -- including the
legitimately-scored-after-timeout case -- and verifier-disabled / multi-step
trials are left byte-for-byte unchanged.

New tests (red pre-fix, green post-fix):
- test_timeout_then_verifier_failure_is_not_masked
- test_emitted_result_json_is_not_silently_unscored

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/harbor/trial/trial.py b/src/harbor/trial/trial.py
@@ -67,6 +67,25 @@ class VerifierTimeoutError(asyncio.TimeoutError):
     pass
 
 
+class VerificationNotCompletedError(Exception):
+    """Raised/recorded when a trial finalizes without a ``verifier_result``.
+
+    Single-step trials are expected to produce a ``verifier_result`` whenever
+    verification is enabled. If the verifier is never invoked, is cut off
+    (cancellation), or raises before producing a result, the trial would
+    otherwise finalize *silently unscored* — and, because the outer handlers
+    record ``exception_info`` first-write-wins, the missing-score state can be
+    masked under an unrelated label (e.g. an earlier ``AgentTimeoutError``).
+
+    Recording this explicit, distinctly-typed failure makes that masked state
+    impossible to confuse with either a clean scored run or a benign
+    agent timeout: downstream aggregation surfaces it as its own error class
+    rather than dropping the trial from the scored denominator unnoticed.
+    """
+
+    pass
+
+
 class EnvironmentStartTimeoutError(asyncio.TimeoutError):
     pass
 
@@ -529,12 +548,64 @@ async def _cleanup_and_finalize(self) -> None:
             if self.result.exception_info is None:
                 self.result.exception_info = ExceptionInfo.from_exception(e)
 
+        self._enforce_verifier_result_invariant()
+
         self.result.finished_at = datetime.now(timezone.utc)
 
         self._trial_paths.result_path.write_text(self.result.model_dump_json(indent=4))
 
         await self._invoke_hooks(TrialEvent.END)
 
+    def _enforce_verifier_result_invariant(self) -> None:
+        """Forbid emitting a finalized trial that was silently never scored.
+
+        For single-step trials with verification enabled, a missing
+        ``verifier_result`` means the verifier was never invoked, was cut off
+        (cancellation), or raised before producing a result. Such a trial must
+        not be emitted as if it were a benign completion: the outer ``run()``
+        handlers record ``exception_info`` first-write-wins, so an earlier,
+        unrelated failure (e.g. ``AgentTimeoutError``) would otherwise mask the
+        "never scored" outcome and the trial would vanish from the scored
+        denominator unnoticed.
+
+        We record an explicit, distinctly-typed
+        :class:`VerificationNotCompletedError`, chaining any pre-existing
+        ``exception_info`` into the message so no diagnostic information is
+        lost. Trials that *did* produce a ``verifier_result`` (including the
+        legitimately-scored-after-timeout case) are left byte-for-byte
+        unchanged, as are verifier-disabled and multi-step trials.
+        """
+        if self.config.verifier.disable or self._task.has_steps:
+            return
+        if self.result.verifier_result is not None:
+            return
+
+        prior = self.result.exception_info
+        if prior is not None:
+            message = (
+                "Trial finalized without a verifier_result: the verifier was "
+                "never invoked or did not produce a result. A prior "
+                f"'{prior.exception_type}' was recorded "
+                f"({prior.exception_message!r}) but the operative outcome is "
+                "that this trial was never scored."
+            )
+        else:
+            message = (
+                "Trial finalized without a verifier_result: the verifier was "
+                "never invoked or did not produce a result, and this trial was "
+                "never scored."
+            )
+
+        error = VerificationNotCompletedError(message)
+        self.result.exception_info = ExceptionInfo.from_exception(error)
+        try:
+            self._trial_paths.exception_message_path.write_text(message)
+        except Exception:
+            self._logger.warning(
+                "Failed to write exception message for unscored trial "
+                f"{self.config.trial_name} (best-effort)"
+            )
+
     async def _maybe_download_logs(
         self, source_dir: str, target_dir: Path | UPath
     ) -> None:
diff --git a/tests/unit/test_trial_verifier_invariant.py b/tests/unit/test_trial_verifier_invariant.py
@@ -0,0 +1,231 @@
+"""Regression tests for the trial verifier-result invariant.
+
+A single-step trial with verification enabled must never finalize and emit a
+``result.json`` in a *silently unscored* state: ``verifier_result is None`` with
+no explicit, correctly-typed failure recorded. In production, 63/300 trials in
+one agentic eval finalized exactly this way — labelled ``AgentTimeoutError`` (the
+swallowed first-write-wins exception from an earlier agent timeout) with an empty
+``verifier/`` dir and ``verifier_result == None`` — making them indistinguishable
+from the trials that genuinely timed out but *did* verify to a reward, and
+silently dropping them from the scored denominator during aggregation.
+
+The verifier was actually invoked (its ``TimingInfo`` was set) but raised
+``AddTestsDirError`` while uploading the tests dir into the timed-out sandbox;
+that failure propagated to the outer ``except Exception`` handler in
+``Trial.run()`` and was swallowed because ``exception_info`` was already set to
+``AgentTimeoutError``.
+
+These tests drive a real :class:`~harbor.trial.trial.Trial` with a stubbed agent
+that times out and a stubbed environment whose ``upload_dir`` fails (so the
+verifier is invoked but raises before producing a result). Pre-fix, the emitted
+result is masked under ``AgentTimeoutError`` with ``verifier_result is None``;
+post-fix, the trial finalizes with an explicit ``VerificationNotCompletedError``.
+"""
+
+import asyncio
+import json
+from pathlib import Path
+
+from harbor.agents.base import BaseAgent
+from harbor.environments.base import BaseEnvironment
+from harbor.environments.capabilities import EnvironmentCapabilities
+from harbor.models.agent.context import AgentContext
+from harbor.models.environment_type import EnvironmentType
+from harbor.models.trial.config import (
+    AgentConfig,
+    EnvironmentConfig,
+    TaskConfig,
+    TrialConfig,
+    VerifierConfig,
+)
+from harbor.trial.trial import Trial
+
+
+class TimeoutAgent(BaseAgent):
+    """Agent that sleeps past the (very small) agent timeout, then is cancelled.
+
+    Mirrors the production failure: ``_execute_agent`` wraps ``run`` in
+    ``asyncio.wait_for`` and raises ``AgentTimeoutError`` on timeout.
+    """
+
+    @staticmethod
+    def name() -> str:
+        return "timeout-agent"
+
+    def version(self) -> str:
+        return "1.0.0"
+
+    async def setup(self, environment: BaseEnvironment) -> None:
+        pass
+
+    async def run(
+        self, instruction: str, environment: BaseEnvironment, context: AgentContext
+    ) -> None:
+        await asyncio.sleep(3600)
+
+
+class UploadFailsEnvironment(BaseEnvironment):
+    """Mounted env whose ``upload_dir`` fails, so the verifier raises.
+
+    ``Verifier.verify`` uploads the tests dir before running and wraps any
+    failure in ``AddTestsDirError``. This reproduces the timed-out-sandbox
+    state where verification is *invoked* but cannot produce a result.
+    """
+
+    @staticmethod
+    def type() -> EnvironmentType:
+        return EnvironmentType.DOCKER
+
+    @property
+    def capabilities(self) -> EnvironmentCapabilities:
+        return EnvironmentCapabilities(mounted=True)
+
+    def _validate_definition(self):
+        pass
+
+    async def start(self, force_build: bool) -> None:
+        pass
+
+    async def stop(self, delete: bool):
+        pass
+
+    async def prepare_logs_for_host(self) -> None:
+        pass
+
+    async def upload_file(self, source_path, target_path):
+        pass
+
+    async def upload_dir(self, source_dir, target_dir):
+        raise RuntimeError("simulated sandbox upload failure after agent timeout")
+
+    async def download_file(self, source_path, target_path):
+        pass
+
+    async def download_dir(self, source_dir, target_dir):
+        pass
+
+    async def exec(self, command, cwd=None, env=None, timeout_sec=None):
+        pass
+
+
+def _create_task_dir(root: Path) -> Path:
+    """Create a minimal valid task directory with a real test.sh."""
+    task_dir = root / "test-task"
+    task_dir.mkdir()
+
+    # Tiny agent timeout so TimeoutAgent.run() is cut off by wait_for.
+    (task_dir / "task.toml").write_text(
+        "[agent]\ntimeout_sec = 0.05\n[verifier]\ntimeout_sec = 10.0\n[environment]\n"
+    )
+    (task_dir / "instruction.md").write_text("Do nothing.")
+
+    env_dir = task_dir / "environment"
+    env_dir.mkdir()
+    (env_dir / "Dockerfile").write_text("FROM ubuntu:24.04\n")
+
+    tests_dir = task_dir / "tests"
+    tests_dir.mkdir()
+    (tests_dir / "test.sh").write_text(
+        "#!/bin/bash\necho 1 > /logs/verifier/reward.txt\n"
+    )
+
+    return task_dir
+
+
+async def _make_trial(tmp_path: Path) -> Trial:
+    """Trial with TimeoutAgent + UploadFailsEnvironment and verification ON."""
+    task_dir = _create_task_dir(tmp_path)
+    trials_dir = tmp_path / "trials"
+    trials_dir.mkdir()
+
+    config = TrialConfig(
+        task=TaskConfig(path=task_dir),
+        trials_dir=trials_dir,
+        agent=AgentConfig(
+            import_path="tests.unit.test_trial_verifier_invariant:TimeoutAgent"
+        ),
+        environment=EnvironmentConfig(
+            import_path=(
+                "tests.unit.test_trial_verifier_invariant:UploadFailsEnvironment"
+            ),
+            delete=True,
+        ),
+        # Verification ENABLED: this is the path that must never silently drop.
+        verifier=VerifierConfig(disable=False),
+    )
+    trial = await Trial.create(config)
+    assert isinstance(trial._agent, TimeoutAgent)
+    assert isinstance(trial._environment, UploadFailsEnvironment)
+    return trial
+
+
+class TestVerifierResultInvariant:
+    """A finalized single-step trial must never be silently unscored."""
+
+    async def test_timeout_then_verifier_failure_is_not_masked(self, tmp_path: Path):
+        """Agent times out, verifier is invoked but raises before producing a
+        result. The trial must NOT finalize masked under ``AgentTimeoutError``
+        with ``verifier_result is None`` — it must record an explicit
+        ``VerificationNotCompletedError``.
+        """
+        trial = await _make_trial(tmp_path)
+
+        result = await trial.run()
+
+        # The verifier never produced a result (the bug's necessary condition).
+        if result.verifier_result is not None:
+            raise AssertionError(
+                "expected verifier_result to be None for this failure mode"
+            )
+
+        # The masked state must be impossible: the operative outcome (never
+        # scored) must be recorded explicitly, not hidden under the earlier
+        # agent-timeout label.
+        if result.exception_info is None:
+            raise AssertionError(
+                "trial finalized unscored with no exception_info recorded "
+                "(silently-dropped masked state)"
+            )
+        if result.exception_info.exception_type == "AgentTimeoutError":
+            raise AssertionError(
+                "unscored trial is masked under AgentTimeoutError; downstream "
+                "cannot distinguish it from a trial that timed out but verified"
+            )
+        if result.exception_info.exception_type != "VerificationNotCompletedError":
+            raise AssertionError(
+                "expected VerificationNotCompletedError, got "
+                f"{result.exception_info.exception_type}"
+            )
+
+    async def test_emitted_result_json_is_not_silently_unscored(self, tmp_path: Path):
+        """The persisted ``result.json`` (what downstream harvests) must carry
+        the explicit failure, and the trial must be finalized.
+        """
+        trial = await _make_trial(tmp_path)
+
+        await trial.run()
+
+        result_path = trial._trial_paths.result_path
+        if not result_path.exists():
+            raise AssertionError("result.json was not emitted")
+
+        emitted = json.loads(result_path.read_text())
+
+        # Finalized (finished_at set) — the trial did complete its lifecycle.
+        if emitted.get("finished_at") is None:
+            raise AssertionError("emitted result.json has no finished_at")
+
+        if emitted.get("verifier_result") is not None:
+            raise AssertionError("expected verifier_result null in emitted result")
+
+        exc = emitted.get("exception_info")
+        if exc is None:
+            raise AssertionError(
+                "emitted result.json is silently unscored: verifier_result null "
+                "with no exception_info"
+            )
+        if exc.get("exception_type") != "VerificationNotCompletedError":
+            raise AssertionError(
+                "emitted result.json masks the unscored state under "
+                f"{exc.get('exception_type')}"
+            )
