From b21463aab6cf9c6ebbd0bd5aa63bf2c2ceda6371 Mon Sep 17 00:00:00 2001 From: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com> Date: Thu, 11 Jun 2026 14:30:10 -0700 Subject: [PATCH] fix(evals): harden terminalbench-native harness against tmux death and setup hangs (#536) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two failure classes hit by a downstream team: 1) tmux/task-env death (TerminalBenchTaskEnv.__enter__, terminalbench_env.py): create_session ran inside the spin_up_terminal generator-CM with no exception safety — a tmux failure leaked the docker compose project (down deferred to GC, never if the env was retained) and surfaced as an opaque mid-run death. Now: exception-safe __enter__ with an idempotent _teardown(), a tmux/asciinema preflight in the container BEFORE the agent loop (TaskEnvironmentError naming the task image + remedy), and fail-that-task-cleanly semantics — the failure is recorded as a harness error (QueryTrace.error_kind="harness_error"), the container is downed, and the run continues. 2) OpenHands in-container SETUP hang misattributed as a model result: harness.run() had no bound (terminal-bench runs installed-agent setup with max_timeout_sec=inf inside the per-trial agent budget), and the summary conversion read the nonexistent results.trial_results attr and hardcoded errors=0, folding zero-model-request trials into resolve-rate as model misses. Now: global_agent_timeout_sec / global_timeout_multiplier are threaded config -> backend -> Harness kwargs (default 1800 s bound on SETUP+RUN; configurable per [run]/[[benchmarks]] TOML), and summarize_benchmark_results() classifies harness/infra failures out of the accuracy denominator keyed on token usage (zero/missing tokens + unresolved = the agent never contacted the model), NOT failure_mode — terminal-bench 0.2.18 leaves failure_mode UNSET on success AND on genuine misses, so a failure_mode-based check would misflag every real model miss. Genuine misses (tokens>0, is_resolved=false) stay in the denominator. QueryTrace gains error/error_kind (wired through to_dict/from_dict so the fields actually reach traces.jsonl; backward-compatible loads), the AgenticRunner flags zero-model-contact traces and TaskEnvironmentError as harness errors, and export/summary/console output exclude harness errors from resolve-rate while reporting them loudly. terminal-bench stays an undeclared dep on purpose: it requires Python >=3.12 while this project supports >=3.10,<3.14, so an unmarked extra would break uv lock. pyproject/uv.lock untouched. Co-authored-by: Claude Opus 4.8 (1M context) --- .../evals/backends/terminalbench_native.py | 157 ++++++++- src/openjarvis/evals/cli.py | 69 ++-- src/openjarvis/evals/core/agentic_runner.py | 85 ++++- src/openjarvis/evals/core/config.py | 30 ++ src/openjarvis/evals/core/environment.py | 15 + src/openjarvis/evals/core/export.py | 28 +- src/openjarvis/evals/core/trace.py | 11 + src/openjarvis/evals/core/types.py | 16 + .../evals/execution/terminalbench_env.py | 181 +++++++++-- tests/evals/core/test_config_timeouts.py | 72 +++++ tests/evals/test_agentic_runner.py | 114 +++++++ tests/evals/test_export.py | 78 +++++ tests/evals/test_terminalbench_env.py | 209 +++++++++++- .../test_terminalbench_native_backend.py | 302 ++++++++++++++++++ 14 files changed, 1290 insertions(+), 77 deletions(-) create mode 100644 tests/evals/core/test_config_timeouts.py create mode 100644 tests/evals/test_terminalbench_native_backend.py diff --git a/src/openjarvis/evals/backends/terminalbench_native.py b/src/openjarvis/evals/backends/terminalbench_native.py index fd772aa1..f599d9c9 100644 --- a/src/openjarvis/evals/backends/terminalbench_native.py +++ b/src/openjarvis/evals/backends/terminalbench_native.py @@ -5,11 +5,13 @@ Uses Harness for Docker-based execution and scoring. from __future__ import annotations +import inspect import logging from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional, Tuple from openjarvis.evals.core.backend import InferenceBackend +from openjarvis.evals.core.types import RunSummary LOGGER = logging.getLogger(__name__) @@ -20,6 +22,97 @@ try: except ImportError: _HAS_TB = False +# terminal-bench FailureMode values that are definitionally infrastructure +# failures (the harness broke before/while driving the agent), never a +# judgment on the model's answer. NOTE: clean trials leave failure_mode +# "unset" in terminal-bench 0.2.18 — both on success AND on genuine +# unresolved misses — so failure_mode alone can NOT be used to detect +# harness errors (it would misflag every real model miss). +_INFRA_FAILURE_MODES = frozenset({"agent_installation_failed", "unknown_agent_error"}) + +# Harness kwargs that older terminal-bench versions may not support. +_TIMEOUT_KWARGS = ("global_agent_timeout_sec", "global_timeout_multiplier") + + +def summarize_benchmark_results( + results: Any, + *, + model: str, + benchmark: str = "terminalbench-native", +) -> Tuple[RunSummary, List[Dict[str, str]]]: + """Convert terminal-bench ``BenchmarkResults`` into a ``RunSummary``. + + Trials are classified into three buckets: + + - resolved: ``is_resolved`` is True -> counted correct. + - model miss: unresolved, but the model was actually contacted -> + counted in the accuracy denominator. + - harness/infra failure: excluded from the accuracy denominator and + reported in ``RunSummary.errors`` plus the returned failure list. + + Zero-model-contact signal choice: terminal-bench 0.2.18 leaves + ``failure_mode`` UNSET both on clean success and on genuine unresolved + misses, so failure_mode cannot distinguish "the model tried and failed" + from "the agent never called the model". Token usage can: this backend + always runs terminus-2, which reports real LiteLLM usage, so an + unresolved trial with zero/missing input+output tokens means no model + request ever completed — an infrastructure failure (in-container setup + hang/death, tmux failure), not a model miss. CAVEAT: terminal-bench + "installed agents" (openhands, claude-code, ...) hardcode 0 tokens even + on success; if this backend ever honors ``agent_name`` for installed + agents, this heuristic must be gated on the agent type. + """ + trials = list(getattr(results, "results", None) or []) + + harness_failures: List[Dict[str, str]] = [] + scored = 0 + correct = 0 + + for tr in trials: + task_id = getattr(tr, "task_id", None) or getattr(tr, "trial_name", "unknown") + is_resolved = getattr(tr, "is_resolved", None) is True + fm = getattr(tr, "failure_mode", None) + fm_value = str(getattr(fm, "value", fm) or "unset").lower() + tokens = (getattr(tr, "total_input_tokens", None) or 0) + ( + getattr(tr, "total_output_tokens", None) or 0 + ) + + zero_model_contact = not is_resolved and tokens == 0 + infra_failure_mode = fm_value in _INFRA_FAILURE_MODES + + if zero_model_contact or infra_failure_mode: + harness_failures.append( + { + "task_id": str(task_id), + "failure_mode": fm_value, + "reason": ( + "zero_model_requests" if zero_model_contact else fm_value + ), + } + ) + continue + + scored += 1 + if is_resolved: + correct += 1 + + return ( + RunSummary( + benchmark=benchmark, + category="agentic", + backend="terminalbench-native", + model=model, + total_samples=len(trials), + scored_samples=scored, + correct=correct, + accuracy=correct / scored if scored else 0.0, + errors=len(harness_failures), + mean_latency_seconds=0.0, + total_cost_usd=0.0, + ), + harness_failures, + ) + class TerminalBenchNativeBackend(InferenceBackend): """Runs terminal-bench tasks natively via Harness with Docker execution. @@ -44,7 +137,22 @@ class TerminalBenchNativeBackend(InferenceBackend): system_prompt: str = "", max_tokens: int = 16384, n_concurrent: int = 4, + global_agent_timeout_sec: Optional[float] = 1800.0, + global_timeout_multiplier: Optional[float] = None, ) -> None: + """Args of note: + + global_agent_timeout_sec: Hard wall-clock bound for each trial's + agent phase. terminal-bench runs installed-agent SETUP inside + this same budget with an infinite tmux timeout, so this bounds + SETUP+RUN together (a setup-only timeout needs an upstream + terminal-bench change). When set, it REPLACES each task's own + ``max_agent_timeout_sec``. Set ``None`` or ``0`` to fall back + to per-task budgets. + global_timeout_multiplier: Scales per-task budgets when + ``global_agent_timeout_sec`` is not set. ``None`` keeps + terminal-bench's default (1.0). + """ if not _HAS_TB: raise ImportError("terminal-bench is required: pip install terminal-bench") @@ -59,6 +167,8 @@ class TerminalBenchNativeBackend(InferenceBackend): self._system_prompt = system_prompt self._max_tokens = max_tokens self._n_concurrent = n_concurrent + self._global_agent_timeout_sec = global_agent_timeout_sec + self._global_timeout_multiplier = global_timeout_multiplier self._results: Optional[BenchmarkResults] = None def run_harness(self, run_id: str) -> BenchmarkResults: @@ -91,10 +201,53 @@ class TerminalBenchNativeBackend(InferenceBackend): if self._max_samples is not None: harness_kwargs["n_tasks"] = self._max_samples + # Bound each trial's agent phase. Without this, an in-container + # installed-agent SETUP hang runs with an infinite tmux timeout, + # bounded only by whatever budget the task happens to declare. + if self._global_agent_timeout_sec: + harness_kwargs["global_agent_timeout_sec"] = float( + self._global_agent_timeout_sec + ) + if self._global_timeout_multiplier is not None: + harness_kwargs["global_timeout_multiplier"] = float( + self._global_timeout_multiplier + ) + + self._check_timeout_kwargs_supported(harness_kwargs) + harness = Harness(**harness_kwargs) self._results = harness.run() return self._results + @staticmethod + def _check_timeout_kwargs_supported(harness_kwargs: Dict[str, Any]) -> None: + """Fail loudly if this terminal-bench build lacks the timeout kwargs. + + terminal-bench is an undeclared, unpinned dependency, so installs may + predate the global timeout kwargs (added by 0.2.x). Passing an + unknown kwarg raises an opaque TypeError; dropping it silently would + re-create the unbounded-setup hang. Detect and explain instead. + """ + try: + params = inspect.signature(Harness.__init__).parameters + except (TypeError, ValueError): + return + if any(p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values()): + return + unsupported = [ + key + for key in _TIMEOUT_KWARGS + if key in harness_kwargs and key not in params + ] + if unsupported: + raise RuntimeError( + "The installed terminal-bench does not support " + f"{', '.join(unsupported)} (requires terminal-bench >= " + "0.2.18). Upgrade terminal-bench, or disable the bound by " + "setting global_agent_timeout_sec = 0 in the eval config " + "[run] section." + ) + def generate( self, prompt: str, @@ -121,4 +274,4 @@ class TerminalBenchNativeBackend(InferenceBackend): pass -__all__ = ["TerminalBenchNativeBackend"] +__all__ = ["TerminalBenchNativeBackend", "summarize_benchmark_results"] diff --git a/src/openjarvis/evals/cli.py b/src/openjarvis/evals/cli.py index 94715db7..d218770d 100644 --- a/src/openjarvis/evals/cli.py +++ b/src/openjarvis/evals/cli.py @@ -690,14 +690,22 @@ def _run_terminalbench_native( from openjarvis.engine.openai_compat_engines import normalize_openai_base_url from openjarvis.evals.backends.terminalbench_native import ( TerminalBenchNativeBackend, + summarize_benchmark_results, ) - from openjarvis.evals.core.types import RunSummary model = config.model # LiteLLM expects "openai/" for OpenAI-compatible servers litellm_model = f"openai/{model}" output_dir = getattr(config, "output_path", None) or "results/terminalbench-native/" + # Harness budgets: only forward explicit config values so the backend + # defaults (global_agent_timeout_sec=1800) apply otherwise. + timeout_kwargs = {} + if getattr(config, "global_agent_timeout_sec", None) is not None: + timeout_kwargs["global_agent_timeout_sec"] = config.global_agent_timeout_sec + if getattr(config, "global_timeout_multiplier", None) is not None: + timeout_kwargs["global_timeout_multiplier"] = config.global_timeout_multiplier + # Normalize to exactly one trailing "/v1" — LiteLLM's api_base wants the # full OpenAI-compatible prefix, and users pass both forms of the URL. if base_url: @@ -712,6 +720,7 @@ def _run_terminalbench_native( max_samples=config.max_samples, output_dir=output_dir, n_concurrent=config.max_workers or 4, + **timeout_kwargs, ) import re @@ -740,28 +749,20 @@ def _run_terminalbench_native( else: results = backend.run_harness(run_id) - # Convert BenchmarkResults to RunSummary - total = len(results.trial_results) if hasattr(results, "trial_results") else 0 - correct = 0 - if hasattr(results, "trial_results"): - for tr in results.trial_results: - if getattr(tr, "is_resolved", False): - correct += 1 - - accuracy = correct / total if total > 0 else 0.0 - return RunSummary( - benchmark="terminalbench-native", - category="agentic", - backend="terminalbench-native", - model=model, - total_samples=total, - scored_samples=total, - correct=correct, - accuracy=accuracy, - errors=0, - mean_latency_seconds=0.0, - total_cost_usd=0.0, - ) + # Convert BenchmarkResults to RunSummary, classifying harness/infra + # failures (e.g. zero-model-contact setup hangs) out of the resolve-rate. + summary, harness_failures = summarize_benchmark_results(results, model=model) + if harness_failures: + console.print( + f" [red bold]{len(harness_failures)} harness/infra failure(s) " + "excluded from resolve-rate:[/red bold]" + ) + for failure in harness_failures: + console.print( + f" [red]- {failure['task_id']}: {failure['reason']} " + f"(failure_mode={failure['failure_mode']})[/red]" + ) + return summary def _run_single( @@ -1039,7 +1040,9 @@ def _print_agentic_summary(console: Console, traces, config) -> None: from rich.table import Table completed = sum(1 for t in traces if t.completed) - resolved = sum(1 for t in traces if t.is_resolved is True) + harness_errors = [t for t in traces if t.error_kind == "harness_error"] + model_traces = [t for t in traces if t.error_kind != "harness_error"] + resolved = sum(1 for t in model_traces if t.is_resolved is True) timed_out = sum(1 for t in traces if t.timed_out) total_turns = sum(t.num_turns for t in traces) total_tool_calls = sum(t.total_tool_calls for t in traces) @@ -1067,8 +1070,11 @@ def _print_agentic_summary(console: Console, traces, config) -> None: table.add_row("Queries", str(len(traces))) table.add_row("Completed", f"{completed}/{len(traces)}") if any(t.is_resolved is not None for t in traces): - table.add_row("Resolved", f"{resolved}/{len(traces)}") + # Harness errors are excluded from the resolve-rate denominator: + # they are infra failures, not model misses. + table.add_row("Resolved", f"{resolved}/{len(model_traces)}") table.add_row("Timed out", str(timed_out)) + table.add_row("Harness errors", str(len(harness_errors))) table.add_row("Total turns", str(total_turns)) avg_t = f"{total_turns / len(traces):.1f}" if traces else "0" table.add_row("Avg turns/query", avg_t) @@ -1095,6 +1101,19 @@ def _print_agentic_summary(console: Console, traces, config) -> None: console.print(table) + if harness_errors: + console.print( + f"[red bold]{len(harness_errors)} harness/infra failure(s) " + "excluded from resolve-rate:[/red bold]" + ) + for t in harness_errors[:5]: + console.print(f"[red] {t.query_id}: {(t.error or '')[:300]}[/red]") + if len(harness_errors) > 5: + console.print( + f"[red] ... and {len(harness_errors) - 5} more " + "(see traces.jsonl)[/red]" + ) + def _run_from_config( config_path: str, diff --git a/src/openjarvis/evals/core/agentic_runner.py b/src/openjarvis/evals/core/agentic_runner.py index 3e9f8ad9..6c262bbd 100644 --- a/src/openjarvis/evals/core/agentic_runner.py +++ b/src/openjarvis/evals/core/agentic_runner.py @@ -20,6 +20,7 @@ from contextlib import nullcontext from pathlib import Path from typing import Any, Callable, Optional +from openjarvis.evals.core.environment import TaskEnvironmentError from openjarvis.evals.core.event_recorder import AgentEvent, EventRecorder, EventType from openjarvis.evals.core.trace import QueryTrace, TurnTrace @@ -176,9 +177,12 @@ class AgenticRunner: ) self._traces.append(trace) - status = ( - "TIMEOUT" if trace.timed_out else ("OK" if trace.completed else "FAIL") - ) + if trace.timed_out: + status = "TIMEOUT" + elif trace.error_kind == "harness_error": + status = "HARNESS_ERROR" + else: + status = "OK" if trace.completed else "FAIL" LOGGER.info( "Task %s: %s in %.1fs", query_id, @@ -260,11 +264,12 @@ class AgenticRunner: is_resolved=record.metadata.get("is_resolved"), ) - status = ( - "TIMEOUT" - if trace.timed_out - else ("OK" if trace.completed else "FAIL") - ) + if trace.timed_out: + status = "TIMEOUT" + elif trace.error_kind == "harness_error": + status = "HARNESS_ERROR" + else: + status = "OK" if trace.completed else "FAIL" LOGGER.info( "Task %s: %s in %.1fs", query_id, @@ -444,7 +449,23 @@ class AgenticRunner: _run_body() except Exception as exc: - LOGGER.warning("Agent failed on query %s: %s", query_id, exc) + # Distinguish infrastructure breakage (task env failed to start, + # Docker/tmux death) from agent failures: harness errors must be + # recorded distinctly so scoring excludes them from resolve-rate + # instead of silently counting a model miss. Either way the run + # continues with the next record (fail THIS task, not the run). + is_harness_error = isinstance(exc, TaskEnvironmentError) or bool( + record.metadata.get("harness_error") + ) + if is_harness_error: + LOGGER.error( + "Harness/environment failure on query %s (record %s): %s", + query_id, + getattr(record, "record_id", "?"), + exc, + ) + else: + LOGGER.warning("Agent failed on query %s: %s", query_id, exc) end_time = time.time() # Unsubscribe EventBus relays if agent_bus is not None: @@ -461,6 +482,8 @@ class AgenticRunner: total_wall_clock_s=end_time - start_time, completed=False, is_resolved=record.metadata.get("is_resolved"), + error=str(exc), + error_kind="harness_error" if is_harness_error else "agent_error", ) # Unsubscribe EventBus relays @@ -534,6 +557,48 @@ class AgenticRunner: model, turn.input_tokens, turn.output_tokens ) + # --- Zero-model-contact sanity check ---------------------------- + # Failed in-container setups (e.g. an installed agent's SETUP phase + # hanging or dying) historically produced traces that looked like + # model results: completed=True, a synthetic 1-event turn, + # is_resolved=False from run_tests, and zero tokens — silently + # dragging resolve-rate down as a fake model miss. Signal choice: + # token usage plus LM inference events is the reliable discriminator + # here — a genuine model miss has token usage (and/or LM events), + # while "the agent never called the model" has neither. We do NOT + # key off completion status or is_resolved, which are identical in + # both cases. run_agent_loop envs drive the model directly + # (bypassing usage reporting), so their turn_wall_clocks count as + # model contact. + had_lm_events = any( + e.event_type in (EventType.LM_INFERENCE_START, EventType.LM_INFERENCE_END) + for e in events + ) + had_loop_turns = bool( + task_env is not None and getattr(task_env, "turn_wall_clocks", None) + ) + turn_tokens = sum(t.input_tokens + t.output_tokens for t in turns) + error: Optional[str] = None + error_kind: Optional[str] = None + if ( + not had_lm_events + and not had_loop_turns + and in_tok + out_tok == 0 + and turn_tokens == 0 + ): + error = ( + "zero_model_requests: the agent produced no LM inference " + "events and reported zero token usage — the model was never " + "contacted. This is a harness/infrastructure failure (e.g. " + "in-container agent setup hang/death), not a model miss, and " + "is excluded from resolve-rate. If your agent genuinely " + "contacted the model, make it report token usage or emit " + "LM_INFERENCE events. Response tail: " + f"{(response_text or '')[-500:]!r}" + ) + error_kind = "harness_error" + LOGGER.error("Query %s: %s", query_id, error) + # Query-level energy from telemetry window query_gpu_energy = _compute_energy_delta(readings, "gpu_energy_j") query_cpu_energy = _compute_energy_delta(readings, "cpu_energy_j") @@ -563,6 +628,8 @@ class AgenticRunner: is_resolved=record.metadata.get("is_resolved"), query_mbu_avg_pct=query_mbu_avg, query_mbu_max_pct=query_mbu_max, + error=error, + error_kind=error_kind, ) # Correlate energy with trace diff --git a/src/openjarvis/evals/core/config.py b/src/openjarvis/evals/core/config.py index aeb16578..96459845 100644 --- a/src/openjarvis/evals/core/config.py +++ b/src/openjarvis/evals/core/config.py @@ -143,6 +143,16 @@ def load_eval_config(path: str | Path) -> EvalSuiteConfig: sheets_worksheet=run_raw.get("sheets_worksheet", "Results"), sheets_credentials_path=run_raw.get("sheets_credentials_path", ""), max_turns=(int(run_raw["max_turns"]) if "max_turns" in run_raw else None), + global_agent_timeout_sec=( + float(run_raw["global_agent_timeout_sec"]) + if "global_agent_timeout_sec" in run_raw + else None + ), + global_timeout_multiplier=( + float(run_raw["global_timeout_multiplier"]) + if "global_timeout_multiplier" in run_raw + else None + ), ) # Parse [[models]] @@ -212,6 +222,16 @@ def load_eval_config(path: str | Path) -> EvalSuiteConfig: max_tokens=int(b["max_tokens"]) if "max_tokens" in b else None, subset=b.get("subset"), record_ids=record_ids, + global_agent_timeout_sec=( + float(b["global_agent_timeout_sec"]) + if "global_agent_timeout_sec" in b + else None + ), + global_timeout_multiplier=( + float(b["global_timeout_multiplier"]) + if "global_timeout_multiplier" in b + else None + ), ) ) @@ -275,6 +295,14 @@ def expand_suite(suite: EvalSuiteConfig) -> List[RunConfig]: if bench.judge_model is not None: judge_model = bench.judge_model + # terminal-bench harness budgets: benchmark > [run] + global_agent_timeout_sec = suite.run.global_agent_timeout_sec + if bench.global_agent_timeout_sec is not None: + global_agent_timeout_sec = bench.global_agent_timeout_sec + global_timeout_multiplier = suite.run.global_timeout_multiplier + if bench.global_timeout_multiplier is not None: + global_timeout_multiplier = bench.global_timeout_multiplier + # Auto-generate output path model_slug = model.name.replace("/", "-").replace(":", "-") output_path = f"{output_dir}/{bench.name}_{model_slug}.jsonl" @@ -328,6 +356,8 @@ def expand_suite(suite: EvalSuiteConfig) -> List[RunConfig]: base_url=suite.backend_external_base_url, api_key=suite.backend_external_api_key, record_ids=bench.record_ids, + global_agent_timeout_sec=global_agent_timeout_sec, + global_timeout_multiplier=global_timeout_multiplier, ) ) diff --git a/src/openjarvis/evals/core/environment.py b/src/openjarvis/evals/core/environment.py index 664215ec..a18378ee 100644 --- a/src/openjarvis/evals/core/environment.py +++ b/src/openjarvis/evals/core/environment.py @@ -8,6 +8,18 @@ from typing import Any, Dict, Tuple from openjarvis.evals.core.types import EvalRecord +class TaskEnvironmentError(RuntimeError): + """A task execution environment failed to start or operate. + + Raised when infrastructure backing a task (Docker container, docker + compose project, tmux session, recording binaries, ...) breaks. This is + a harness/environment failure, **not** a model failure: runners record + it distinctly (``QueryTrace.error_kind == "harness_error"``) so scoring + can exclude the sample from resolve-rate instead of silently counting + it as a model miss. + """ + + class EnvironmentProvider(ABC): """Manages an external environment for evaluation benchmarks. @@ -50,3 +62,6 @@ class EnvironmentProvider(ABC): @abstractmethod def teardown(self) -> None: """Stop the environment and release resources.""" + + +__all__ = ["EnvironmentProvider", "TaskEnvironmentError"] diff --git a/src/openjarvis/evals/core/export.py b/src/openjarvis/evals/core/export.py index 4b3fd0cd..37d9fd0a 100644 --- a/src/openjarvis/evals/core/export.py +++ b/src/openjarvis/evals/core/export.py @@ -26,13 +26,22 @@ def _agg_stats(values: Sequence[Optional[float]]) -> dict[str, Optional[float]]: } +def _model_attributable(traces: list[QueryTrace]) -> list[QueryTrace]: + """Traces whose outcome is attributable to the model. + + Harness errors (infra/setup failures, zero-model-contact runs) are + excluded so they never count as model misses in resolve-rate. + """ + return [t for t in traces if t.error_kind != "harness_error"] + + def _compute_efficiency( traces: list[QueryTrace], total_gpu_energy: Optional[float], total_cpu_energy: Optional[float], ) -> dict[str, Optional[float]]: """Compute efficiency metrics from traces and aggregate energy.""" - scored = [t for t in traces if t.is_resolved is not None] + scored = [t for t in _model_attributable(traces) if t.is_resolved is not None] resolved = sum(1 for t in scored if t.is_resolved is True) accuracy = resolved / len(scored) if scored else None gpu_powers = [ @@ -247,8 +256,12 @@ def export_summary_json( cpu_energy_values.append(sum(cpu_vals)) total_cpu_energy = sum(cpu_energy_values) if cpu_energy_values else None - resolved = sum(1 for t in traces if t.is_resolved is True) - unresolved = sum(1 for t in traces if t.is_resolved is False) + # Harness errors (infra failures, zero-model-contact runs) are excluded + # from the resolve-rate denominator: they are not model misses. + harness_error_traces = [t for t in traces if t.error_kind == "harness_error"] + model_traces = _model_attributable(traces) + resolved = sum(1 for t in model_traces if t.is_resolved is True) + unresolved = sum(1 for t in model_traces if t.is_resolved is False) cost_values = [t.total_cost_usd for t in traces if t.total_cost_usd is not None] total_cost = sum(cost_values) if cost_values else None @@ -345,6 +358,7 @@ def export_summary_json( "completed": completed, "resolved": resolved, "unresolved": unresolved, + "harness_errors": len(harness_error_traces), "accuracy": accuracy, "turns": total_turns, "tool_calls": total_tool_calls, @@ -365,6 +379,12 @@ def export_summary_json( "efficiency": efficiency, } + if harness_error_traces: + summary["harness_error_details"] = [ + {"query_id": t.query_id, "error": (t.error or "")[:500]} + for t in harness_error_traces + ] + if action_totals: summary["action_energy_summary"] = action_totals @@ -399,7 +419,7 @@ def export_summary_json( accuracy_vals: list[float] = [ 1.0 if t.is_resolved is True else 0.0 - for t in traces + for t in _model_attributable(traces) if t.is_resolved is not None ] latency_vals = [t.total_wall_clock_s for t in traces if t.total_wall_clock_s > 0] diff --git a/src/openjarvis/evals/core/trace.py b/src/openjarvis/evals/core/trace.py index f2d230f5..8e2e1a1b 100644 --- a/src/openjarvis/evals/core/trace.py +++ b/src/openjarvis/evals/core/trace.py @@ -91,6 +91,13 @@ class QueryTrace: is_resolved: Optional[bool] = None query_mbu_avg_pct: Optional[float] = None query_mbu_max_pct: Optional[float] = None + # Error taxonomy. ``error_kind`` distinguishes infrastructure failures + # ("harness_error": task env / Docker / tmux broke, or the agent never + # contacted the model) from agent failures ("agent_error"). Harness + # errors are excluded from resolve-rate by export/summary code so they + # are never silently counted as model misses. + error: Optional[str] = None + error_kind: Optional[str] = None @property def num_turns(self) -> int: @@ -196,6 +203,8 @@ class QueryTrace: "is_resolved": self.is_resolved, "query_mbu_avg_pct": self.query_mbu_avg_pct, "query_mbu_max_pct": self.query_mbu_max_pct, + "error": self.error, + "error_kind": self.error_kind, } @classmethod @@ -216,6 +225,8 @@ class QueryTrace: is_resolved=d.get("is_resolved"), query_mbu_avg_pct=d.get("query_mbu_avg_pct"), query_mbu_max_pct=d.get("query_mbu_max_pct"), + error=d.get("error"), + error_kind=d.get("error_kind"), ) def save_jsonl(self, path: Path) -> None: diff --git a/src/openjarvis/evals/core/types.py b/src/openjarvis/evals/core/types.py index 0050a6b7..78605634 100644 --- a/src/openjarvis/evals/core/types.py +++ b/src/openjarvis/evals/core/types.py @@ -102,6 +102,15 @@ class RunConfig: # specific records (e.g. recovering silent-fake records without # re-running the entire benchmark). record_ids: Optional[List[str]] = None + # terminal-bench harness budgets (terminalbench-native backend). + # global_agent_timeout_sec bounds each trial's agent phase — SETUP+RUN + # together, since terminal-bench runs installed-agent setup inside the + # agent budget with an infinite tmux timeout. When set it REPLACES the + # per-task max_agent_timeout_sec; 0 disables the bound (per-task budgets + # apply); None uses the backend default (1800 s). + global_agent_timeout_sec: Optional[float] = None + # Scales per-task budgets when global_agent_timeout_sec is not set. + global_timeout_multiplier: Optional[float] = None @dataclass(slots=True) @@ -232,6 +241,9 @@ class ExecutionConfig: # to JarvisConfig.agent.max_turns (default 10). Bump to 30-50 for # thinking/reasoning models on agentic benchmarks (GAIA, LiveResearch). max_turns: Optional[int] = None + # terminal-bench harness budgets (see RunConfig for semantics). + global_agent_timeout_sec: Optional[float] = None + global_timeout_multiplier: Optional[float] = None @dataclass(slots=True) @@ -265,6 +277,10 @@ class BenchmarkConfig: max_tokens: Optional[int] = None subset: Optional[str] = None record_ids: Optional[List[str]] = None + # Per-benchmark override of the terminal-bench harness budgets + # (see RunConfig for semantics). + global_agent_timeout_sec: Optional[float] = None + global_timeout_multiplier: Optional[float] = None @dataclass(slots=True) diff --git a/src/openjarvis/evals/execution/terminalbench_env.py b/src/openjarvis/evals/execution/terminalbench_env.py index 196b5705..b17863e3 100644 --- a/src/openjarvis/evals/execution/terminalbench_env.py +++ b/src/openjarvis/evals/execution/terminalbench_env.py @@ -8,6 +8,8 @@ from pathlib import Path from types import TracebackType from typing import Any, MutableMapping, Optional, Type +from openjarvis.evals.core.environment import TaskEnvironmentError + LOGGER = logging.getLogger(__name__) @@ -29,8 +31,6 @@ class TerminalBenchTaskEnv: # ------------------------------------------------------------------ def __enter__(self) -> TerminalBenchTaskEnv: - from terminal_bench.terminal.terminal import spin_up_terminal - task = self._metadata.get("task") task_paths = self._metadata.get("task_paths") task_id = self._metadata.get("task_id", "unknown") @@ -41,6 +41,8 @@ class TerminalBenchTaskEnv: "Use the 'terminalbench-native' dataset." ) + from terminal_bench.terminal.terminal import spin_up_terminal + docker_image_prefix = f"tb__{task_id}".replace(".", "-") client_image_name = f"{docker_image_prefix}__client" client_container_name = f"oj-{task_id}".replace(".", "-") @@ -48,19 +50,59 @@ class TerminalBenchTaskEnv: self._logs_tmpdir = tempfile.TemporaryDirectory(prefix="oj_tb_logs_") logs_path = Path(self._logs_tmpdir.name) - self._terminal_cm = spin_up_terminal( - client_container_name=client_container_name, - client_image_name=client_image_name, - docker_compose_path=task_paths.docker_compose_path, - docker_image_name_prefix=docker_image_prefix, - sessions_logs_path=logs_path, - disable_recording=task.disable_asciinema, - ) - self._terminal = self._terminal_cm.__enter__() + # Everything below is exception-safe: a failure mid-startup (docker + # compose, tmux, asciinema) tears the spun-up terminal back down + # immediately instead of leaking the docker compose project until GC + # (or forever, when the env object is retained), and re-raises as a + # loud TaskEnvironmentError naming the task image so the runner can + # record a harness error for THIS task and continue with the rest. + try: + self._terminal_cm = spin_up_terminal( + client_container_name=client_container_name, + client_image_name=client_image_name, + docker_compose_path=task_paths.docker_compose_path, + docker_image_name_prefix=docker_image_prefix, + sessions_logs_path=logs_path, + disable_recording=task.disable_asciinema, + ) + self._terminal = self._terminal_cm.__enter__() - session = self._terminal.create_session( - "agent", is_active_stream=False, as_configured_user=True - ) + # Preflight BEFORE the agent loop: terminal-bench drives the + # agent through tmux (and records via asciinema unless the task + # disables it). A missing binary otherwise surfaces mid-run as + # an opaque RuntimeError or a fake TimeoutError. + self._preflight_container_binaries( + task, task_id, client_image_name, client_container_name + ) + + session = self._terminal.create_session( + "agent", is_active_stream=False, as_configured_user=True + ) + except BaseException as exc: + self._teardown(type(exc), exc, exc.__traceback__) + if not isinstance(exc, Exception): + # KeyboardInterrupt / SystemExit: clean up but never mask. + raise + if isinstance(exc, TaskEnvironmentError): + self._metadata["harness_error"] = str(exc) + raise + message = ( + f"Task '{task_id}': failed to start the task environment " + f"(image '{client_image_name}', container " + f"'{client_container_name}'): {exc}. This is a harness/" + "environment failure, not a model failure. Check that the " + "Docker daemon is healthy and that tmux is installed in the " + "task image." + ) + # docker compose stderr is only logged at DEBUG by + # terminal-bench; surface it here so the failure is actionable. + stderr = getattr(exc, "stderr", None) + if stderr: + if isinstance(stderr, bytes): + stderr = stderr.decode("utf-8", errors="replace") + message += f"\ndocker compose stderr (tail):\n{stderr[-2000:]}" + self._metadata["harness_error"] = message + raise TaskEnvironmentError(message) from exc self._metadata["terminal"] = self._terminal self._metadata["session"] = session @@ -68,24 +110,99 @@ class TerminalBenchTaskEnv: return self + def _preflight_container_binaries( + self, + task: Any, + task_id: str, + client_image_name: str, + client_container_name: str, + ) -> None: + """Verify tmux (and asciinema if recording) exist in the container. + + Raises: + TaskEnvironmentError: naming the task image and the missing + binary, with the remedy, before any agent work starts. + """ + container = getattr(self._terminal, "container", None) + if container is None: + # Terminal implementation without a container handle (e.g. a + # future terminal-bench version); fall through to terminal-bench's + # own checks rather than guessing. + return + + checks: list[tuple[str, list[str], str]] = [ + ( + "tmux", + ["tmux", "-V"], + f"install tmux in the task image '{client_image_name}'", + ), + ] + if not getattr(task, "disable_asciinema", False): + checks.append( + ( + "asciinema", + ["asciinema", "--version"], + f"install asciinema in the task image '{client_image_name}' " + "or set disable_asciinema in task.yaml", + ) + ) + + for binary, cmd, remedy in checks: + result = container.exec_run(cmd) + exit_code = getattr(result, "exit_code", 0) + if exit_code == 0: + continue + output = getattr(result, "output", b"") + if isinstance(output, bytes): + output = output.decode("utf-8", errors="replace") + raise TaskEnvironmentError( + f"Task '{task_id}': required binary '{binary}' is not usable " + f"in task image '{client_image_name}' (container " + f"'{client_container_name}'): exec exit code {exit_code}, " + f"output {str(output).strip()!r}. Remedy: {remedy}. This is " + "a harness/environment failure, not a model failure." + ) + + def _teardown( + self, + exc_type: Optional[Type[BaseException]] = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> None: + """Idempotent cleanup shared by ``__exit__`` and failed ``__enter__``. + + Secondary cleanup errors are logged, never raised, so they cannot + mask the original failure. + """ + self._metadata.pop("terminal", None) + self._metadata.pop("session", None) + self._metadata.pop("container", None) + + terminal_cm, self._terminal_cm, self._terminal = self._terminal_cm, None, None + if terminal_cm is not None: + try: + terminal_cm.__exit__(exc_type, exc_val, exc_tb) + except Exception: + LOGGER.exception( + "Secondary error while tearing down the terminal for " + "task %s (original error, if any, is re-raised)", + self._metadata.get("task_id", "unknown"), + ) + + logs_tmpdir, self._logs_tmpdir = self._logs_tmpdir, None + if logs_tmpdir is not None: + try: + logs_tmpdir.cleanup() + except Exception: + LOGGER.exception("Failed to clean up session logs tmpdir") + def __exit__( self, exc_type: Optional[Type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType], ) -> None: - self._metadata.pop("terminal", None) - self._metadata.pop("session", None) - self._metadata.pop("container", None) - - if self._terminal_cm is not None: - self._terminal_cm.__exit__(exc_type, exc_val, exc_tb) - self._terminal_cm = None - self._terminal = None - - if self._logs_tmpdir is not None: - self._logs_tmpdir.cleanup() - self._logs_tmpdir = None + self._teardown(exc_type, exc_val, exc_tb) # ------------------------------------------------------------------ # Test execution @@ -93,12 +210,6 @@ class TerminalBenchTaskEnv: def run_tests(self) -> tuple[bool, dict[str, Any]]: """Copy test scripts into container, execute, parse results.""" - from terminal_bench.parsers.base_parser import UnitTestStatus - from terminal_bench.parsers.parser_factory import ParserFactory - from terminal_bench.terminal.docker_compose_manager import ( - DockerComposeManager, - ) - task = self._metadata["task"] task_paths = self._metadata["task_paths"] terminal = self._terminal @@ -110,6 +221,12 @@ class TerminalBenchTaskEnv: self._metadata["test_results"] = results return False, results + from terminal_bench.parsers.base_parser import UnitTestStatus + from terminal_bench.parsers.parser_factory import ParserFactory + from terminal_bench.terminal.docker_compose_manager import ( + DockerComposeManager, + ) + try: paths_to_copy = [task_paths.run_tests_path] if task_paths.test_dir.exists(): diff --git a/tests/evals/core/test_config_timeouts.py b/tests/evals/core/test_config_timeouts.py new file mode 100644 index 00000000..8c7f34ba --- /dev/null +++ b/tests/evals/core/test_config_timeouts.py @@ -0,0 +1,72 @@ +"""Tests for terminal-bench harness timeout plumbing through TOML configs.""" + +from __future__ import annotations + +import textwrap + +from openjarvis.evals.core.config import expand_suite, load_eval_config + + +def _write(tmp_path, body: str): + path = tmp_path / "suite.toml" + path.write_text(textwrap.dedent(body)) + return path + + +BASE = """ + [meta] + name = "timeouts" + + [run] + output_dir = "results/" + {run_extra} + + [[models]] + name = "test-model" + + [[benchmarks]] + name = "terminalbench-native" + backend = "terminalbench-native" + {bench_extra} +""" + + +class TestTimeoutConfigPlumbing: + def test_run_level_timeouts_parse_and_expand(self, tmp_path): + path = _write( + tmp_path, + BASE.format( + run_extra=( + "global_agent_timeout_sec = 1200\n" + " global_timeout_multiplier = 1.5" + ), + bench_extra="", + ), + ) + suite = load_eval_config(path) + assert suite.run.global_agent_timeout_sec == 1200.0 + assert suite.run.global_timeout_multiplier == 1.5 + + (rc,) = expand_suite(suite) + assert rc.global_agent_timeout_sec == 1200.0 + assert rc.global_timeout_multiplier == 1.5 + + def test_benchmark_override_wins(self, tmp_path): + path = _write( + tmp_path, + BASE.format( + run_extra="global_agent_timeout_sec = 1200", + bench_extra="global_agent_timeout_sec = 300", + ), + ) + (rc,) = expand_suite(load_eval_config(path)) + assert rc.global_agent_timeout_sec == 300.0 + + def test_defaults_are_none(self, tmp_path): + path = _write(tmp_path, BASE.format(run_extra="", bench_extra="")) + suite = load_eval_config(path) + assert suite.run.global_agent_timeout_sec is None + assert suite.run.global_timeout_multiplier is None + (rc,) = expand_suite(suite) + assert rc.global_agent_timeout_sec is None + assert rc.global_timeout_multiplier is None diff --git a/tests/evals/test_agentic_runner.py b/tests/evals/test_agentic_runner.py index 122d3fe8..718bec2d 100644 --- a/tests/evals/test_agentic_runner.py +++ b/tests/evals/test_agentic_runner.py @@ -9,6 +9,7 @@ from typing import Any, Dict, List import pytest from openjarvis.evals.core.agentic_runner import AgenticRunner, _extract_patch +from openjarvis.evals.core.environment import TaskEnvironmentError # --------------------------------------------------------------------------- # Mock objects @@ -50,6 +51,66 @@ class MockFailingAgent: raise RuntimeError("Agent error") +class MockZeroContactAgent: + """Agent that returns without ever contacting the model. + + Mirrors the downstream failure signature: a hung/dead in-container + setup yields zero LM events and zero token usage, while run_tests + still stamps is_resolved=False. + """ + + def ask(self, query: str) -> dict: + return {"content": "setup log tail ...", "usage": {}} + + +class FailingTaskEnv: + """Task env whose __enter__ fails like a tmux/compose breakage.""" + + def __init__(self, metadata: Dict[str, Any]) -> None: + self._metadata = metadata + + def __enter__(self) -> "FailingTaskEnv": + message = ( + "Task 't1': required binary 'tmux' is not usable in task image " + "'tb__t1__client'" + ) + self._metadata["harness_error"] = message + raise TaskEnvironmentError(message) + + def __exit__(self, *args: Any) -> None: + return None + + +class ResolvingTaskEnv: + """Task env that stamps is_resolved into metadata like run_tests does.""" + + def __init__(self, metadata: Dict[str, Any], is_resolved: bool) -> None: + self._metadata = metadata + self._is_resolved = is_resolved + + def __enter__(self) -> "ResolvingTaskEnv": + return self + + def __exit__(self, *args: Any) -> None: + return None + + def run_tests(self): + self._metadata["is_resolved"] = self._is_resolved + return self._is_resolved, {} + + +class EnvDataset(MockDataset): + """Dataset whose create_task_env is configurable per record id.""" + + def __init__(self, records: List[MockRecord], env_factories: Dict[str, Any]): + super().__init__(records) + self._env_factories = env_factories + + def create_task_env(self, record: MockRecord): + factory = self._env_factories.get(record.record_id) + return factory(record.metadata) if factory is not None else None + + # --------------------------------------------------------------------------- # Tests # --------------------------------------------------------------------------- @@ -99,6 +160,59 @@ class TestAgenticRunner: assert len(traces) == 1 assert not traces[0].completed assert "Agent error" in traces[0].response_text + assert traces[0].error_kind == "agent_error" + assert "Agent error" in (traces[0].error or "") + + def test_harness_failure_recorded_and_run_continues(self): + """(c) one task's env breakage doesn't kill the run.""" + records = [ + MockRecord(record_id="r1", problem="broken env"), + MockRecord(record_id="r2", problem="healthy"), + ] + dataset = EnvDataset(records, {"r1": FailingTaskEnv}) + runner = AgenticRunner(agent=MockAgent(), dataset=dataset) + + traces = self._run_async(runner.run()) + assert len(traces) == 2 + # Failed task: recorded distinctly as a harness error, not a miss. + assert traces[0].error_kind == "harness_error" + assert not traces[0].completed + assert "tmux" in (traces[0].error or "") + assert traces[0].is_resolved is None + # Healthy task still ran to completion. + assert traces[1].completed + assert traces[1].error_kind is None + assert "Response to: healthy" in traces[1].response_text + + def test_zero_model_contact_flagged_as_harness_error(self): + """(e) zero model requests -> harness_error, not a model miss.""" + records = [MockRecord(record_id="r1", problem="task")] + dataset = EnvDataset( + records, + {"r1": lambda meta: ResolvingTaskEnv(meta, is_resolved=False)}, + ) + runner = AgenticRunner(agent=MockZeroContactAgent(), dataset=dataset) + + traces = self._run_async(runner.run()) + assert traces[0].error_kind == "harness_error" + assert "zero_model_requests" in (traces[0].error or "") + assert traces[0].total_input_tokens == 0 + assert traces[0].total_output_tokens == 0 + + def test_genuine_model_miss_not_flagged(self): + """(e) control: tokens>0 + is_resolved=False is a model miss.""" + records = [MockRecord(record_id="r1", problem="task")] + dataset = EnvDataset( + records, + {"r1": lambda meta: ResolvingTaskEnv(meta, is_resolved=False)}, + ) + runner = AgenticRunner(agent=MockAgent(), dataset=dataset) + + traces = self._run_async(runner.run()) + assert traces[0].is_resolved is False + assert traces[0].error_kind is None + assert traces[0].error is None + assert traces[0].total_input_tokens > 0 def test_synthetic_turn_created(self): records = [MockRecord(record_id="r1", problem="test")] diff --git a/tests/evals/test_export.py b/tests/evals/test_export.py index 81df1e8e..19ba653a 100644 --- a/tests/evals/test_export.py +++ b/tests/evals/test_export.py @@ -127,6 +127,84 @@ class TestExportSummaryJson: assert summary["totals"]["queries"] == 0 +class TestHarnessErrorExclusion: + """Harness errors must never count as model misses in resolve-rate.""" + + def _traces(self): + return [ + QueryTrace( + query_id="q0000", + workload_type="agentic", + completed=True, + is_resolved=True, + turns=[TurnTrace(turn_index=0, input_tokens=10, output_tokens=5)], + ), + QueryTrace( + query_id="q0001", + workload_type="agentic", + completed=True, + is_resolved=False, # genuine model miss: stays in denominator + turns=[TurnTrace(turn_index=0, input_tokens=10, output_tokens=5)], + ), + QueryTrace( + query_id="q0002", + workload_type="agentic", + completed=False, + is_resolved=False, # stamped by run_tests despite zero contact + error="zero_model_requests: the agent never contacted the model", + error_kind="harness_error", + ), + ] + + def test_summary_excludes_harness_errors_from_accuracy(self, tmp_path): + path = tmp_path / "summary.json" + export_summary_json(self._traces(), {"model": "m"}, path) + summary = json.loads(path.read_text()) + + totals = summary["totals"] + assert totals["harness_errors"] == 1 + assert totals["resolved"] == 1 + assert totals["unresolved"] == 1 # NOT 2: harness error excluded + assert totals["accuracy"] == 0.5 # NOT 1/3 + # Flat table_gen metrics exclude the harness error too. + assert summary["metrics"]["accuracy"]["n"] == 2 + assert summary["metrics"]["accuracy"]["mean"] == 0.5 + # Details surfaced for diagnosis. + details = summary["harness_error_details"] + assert details[0]["query_id"] == "q0002" + assert "zero_model_requests" in details[0]["error"] + + def test_efficiency_excludes_harness_errors(self): + result = _compute_efficiency(self._traces(), None, None) + assert result["accuracy"] == 0.5 + + def test_no_harness_errors_key_absent(self, tmp_path): + path = tmp_path / "summary.json" + export_summary_json(_make_traces(), {}, path) + summary = json.loads(path.read_text()) + assert summary["totals"]["harness_errors"] == 0 + assert "harness_error_details" not in summary + + +class TestTraceErrorFieldRoundTrip: + def test_round_trip_preserves_error_fields(self): + trace = QueryTrace( + query_id="q0", + workload_type="agentic", + error="zero_model_requests: ...", + error_kind="harness_error", + ) + restored = QueryTrace.from_dict(trace.to_dict()) + assert restored.error == trace.error + assert restored.error_kind == "harness_error" + + def test_old_trace_dicts_still_load(self): + """Backward compat: traces.jsonl written before the schema change.""" + restored = QueryTrace.from_dict({"query_id": "q0", "workload_type": "agentic"}) + assert restored.error is None + assert restored.error_kind is None + + class TestExportArtifactsManifest: def test_no_artifacts_dir(self, tmp_path): result = export_artifacts_manifest(tmp_path) diff --git a/tests/evals/test_terminalbench_env.py b/tests/evals/test_terminalbench_env.py index a348d437..ad22c395 100644 --- a/tests/evals/test_terminalbench_env.py +++ b/tests/evals/test_terminalbench_env.py @@ -1,15 +1,104 @@ -"""Tests for TerminalBenchTaskEnv (mocked terminal_bench dependency).""" +"""Tests for TerminalBenchTaskEnv (mocked terminal_bench dependency). + +These tests install a fake ``terminal_bench`` module tree into +``sys.modules`` so they run without the real package or a Docker daemon +(terminal-bench is an undeclared optional dep that CI never installs). +""" from __future__ import annotations +import sys +import types +from contextlib import contextmanager +from dataclasses import dataclass, field +from types import SimpleNamespace +from typing import Any, Dict, List + import pytest +from openjarvis.evals.core.environment import TaskEnvironmentError from openjarvis.evals.execution.terminalbench_env import TerminalBenchTaskEnv -# terminal_bench is an optional dep — skip all tests if unavailable -terminal_bench = pytest.importorskip( - "terminal_bench", reason="terminal_bench not installed" -) +# --------------------------------------------------------------------------- +# Fake terminal_bench seam +# --------------------------------------------------------------------------- + + +@dataclass +class FakeExecResult: + exit_code: int = 0 + output: bytes = b"" + + +@dataclass +class FakeContainer: + """Container stub whose exec_run results are configurable per binary.""" + + exec_results: Dict[str, FakeExecResult] = field(default_factory=dict) + exec_calls: List[List[str]] = field(default_factory=list) + + def exec_run(self, cmd: List[str]) -> FakeExecResult: + self.exec_calls.append(list(cmd)) + return self.exec_results.get(cmd[0], FakeExecResult()) + + +class FakeTerminal: + def __init__(self, events: List[str], container: FakeContainer) -> None: + self._events = events + self.container = container + self.create_session_error: Exception | None = None + + def create_session(self, name: str, **_kwargs: Any) -> str: + self._events.append(f"create_session({name})") + if self.create_session_error is not None: + raise self.create_session_error + return f"session-{name}" + + +@pytest.fixture() +def fake_tb(monkeypatch, tmp_path): + """Install a fake terminal_bench tree; return the shared test state.""" + events: List[str] = [] + container = FakeContainer() + terminal = FakeTerminal(events, container) + state = SimpleNamespace( + events=events, + container=container, + terminal=terminal, + spin_up_error=None, + ) + + @contextmanager + def spin_up_terminal(**kwargs: Any): + events.append("compose_up") + try: + if state.spin_up_error is not None: + raise state.spin_up_error + yield terminal + finally: + events.append("compose_down") + + mod_tb = types.ModuleType("terminal_bench") + mod_terminal_pkg = types.ModuleType("terminal_bench.terminal") + mod_terminal = types.ModuleType("terminal_bench.terminal.terminal") + mod_terminal.spin_up_terminal = spin_up_terminal + mod_tb.terminal = mod_terminal_pkg + mod_terminal_pkg.terminal = mod_terminal + monkeypatch.setitem(sys.modules, "terminal_bench", mod_tb) + monkeypatch.setitem(sys.modules, "terminal_bench.terminal", mod_terminal_pkg) + monkeypatch.setitem(sys.modules, "terminal_bench.terminal.terminal", mod_terminal) + + state.metadata = { + "task_id": "hello.world", + "task": SimpleNamespace(disable_asciinema=True), + "task_paths": SimpleNamespace(docker_compose_path=tmp_path / "compose.yaml"), + } + return state + + +# --------------------------------------------------------------------------- +# Existing behavior (now running without the real terminal_bench package) +# --------------------------------------------------------------------------- class TestTerminalBenchTaskEnv: @@ -46,3 +135,113 @@ class TestTerminalBenchTaskEnv: assert is_resolved is False assert results["error"] == "terminal_not_running" assert metadata["is_resolved"] is False + + +# --------------------------------------------------------------------------- +# Exception-safe __enter__ / preflight / teardown +# --------------------------------------------------------------------------- + + +class TestEnterExceptionSafety: + def test_success_path(self, fake_tb): + env = TerminalBenchTaskEnv(fake_tb.metadata) + with env: + assert fake_tb.metadata["terminal"] is fake_tb.terminal + assert fake_tb.metadata["session"] == "session-agent" + assert fake_tb.metadata["container"] == "oj-hello-world" + assert "compose_down" not in fake_tb.events + assert fake_tb.events.count("compose_down") == 1 + assert "terminal" not in fake_tb.metadata + + def test_create_session_failure_tears_down_terminal(self, fake_tb): + """(a) tmux failure in __enter__ -> terminal torn down, loud error.""" + fake_tb.terminal.create_session_error = RuntimeError( + "tmux is not installed in the container." + ) + env = TerminalBenchTaskEnv(fake_tb.metadata) + + with pytest.raises(TaskEnvironmentError) as excinfo: + env.__enter__() + + # No leak: compose project downed BEFORE the exception escaped. + assert "compose_down" in fake_tb.events + # Actionable: names the task, the image, and the failure. + message = str(excinfo.value) + assert "hello.world" in message + assert "tb__hello-world__client" in message + assert "tmux is not installed" in message + # Recorded for the runner / scorers; handles cleared. + assert fake_tb.metadata["harness_error"] == message + assert "terminal" not in fake_tb.metadata + assert "session" not in fake_tb.metadata + assert "container" not in fake_tb.metadata + assert env._terminal is None + assert env._terminal_cm is None + assert env._logs_tmpdir is None + + def test_preflight_catches_missing_tmux(self, fake_tb): + """(b) preflight catches missing tmux, naming the task image.""" + fake_tb.container.exec_results["tmux"] = FakeExecResult( + exit_code=127, + output=b'exec: "tmux": executable file not found in $PATH', + ) + env = TerminalBenchTaskEnv(fake_tb.metadata) + + with pytest.raises(TaskEnvironmentError) as excinfo: + env.__enter__() + + message = str(excinfo.value) + assert "tmux" in message + assert "tb__hello-world__client" in message # task image named + assert "127" in message + # Fired BEFORE the agent session was created. + assert not any(e.startswith("create_session") for e in fake_tb.events) + assert "compose_down" in fake_tb.events + assert fake_tb.metadata["harness_error"] == message + + def test_preflight_checks_asciinema_when_recording(self, fake_tb): + fake_tb.metadata["task"] = SimpleNamespace(disable_asciinema=False) + fake_tb.container.exec_results["asciinema"] = FakeExecResult(exit_code=127) + env = TerminalBenchTaskEnv(fake_tb.metadata) + + with pytest.raises(TaskEnvironmentError, match="asciinema"): + env.__enter__() + assert "disable_asciinema" in fake_tb.metadata["harness_error"] + assert "compose_down" in fake_tb.events + + def test_preflight_skips_asciinema_when_disabled(self, fake_tb): + fake_tb.container.exec_results["asciinema"] = FakeExecResult(exit_code=127) + env = TerminalBenchTaskEnv(fake_tb.metadata) + with env: + pass + assert ["asciinema", "--version"] not in fake_tb.container.exec_calls + + def test_compose_up_failure_includes_stderr(self, fake_tb): + import subprocess + + fake_tb.spin_up_error = subprocess.CalledProcessError( + returncode=1, + cmd=["docker", "compose", "up"], + stderr="no space left on device", + ) + env = TerminalBenchTaskEnv(fake_tb.metadata) + + with pytest.raises(TaskEnvironmentError) as excinfo: + env.__enter__() + assert "no space left on device" in str(excinfo.value) + + def test_keyboard_interrupt_not_masked(self, fake_tb): + fake_tb.terminal.create_session_error = KeyboardInterrupt() + env = TerminalBenchTaskEnv(fake_tb.metadata) + + with pytest.raises(KeyboardInterrupt): + env.__enter__() + # Still cleaned up, but the interrupt is not wrapped. + assert "compose_down" in fake_tb.events + + def test_teardown_is_idempotent(self, fake_tb): + env = TerminalBenchTaskEnv(fake_tb.metadata) + env.__enter__() + env.__exit__(None, None, None) + env.__exit__(None, None, None) + assert fake_tb.events.count("compose_down") == 1 diff --git a/tests/evals/test_terminalbench_native_backend.py b/tests/evals/test_terminalbench_native_backend.py new file mode 100644 index 00000000..4b3eea5d --- /dev/null +++ b/tests/evals/test_terminalbench_native_backend.py @@ -0,0 +1,302 @@ +"""Tests for the TerminalBench native backend (mocked terminal_bench). + +Covers: timeout kwargs threading (config -> backend -> Harness kwargs), +loud failure on terminal-bench builds without the timeout kwargs, and the +harness-error classification in ``summarize_benchmark_results`` — +including the zero-model-contact vs genuine-model-miss distinction. +""" + +from __future__ import annotations + +import sys +import types +from types import SimpleNamespace +from typing import Any, Dict, List, Optional + +import pytest + +import openjarvis.evals.backends.terminalbench_native as tbn +from openjarvis.evals.backends.terminalbench_native import ( + summarize_benchmark_results, +) + +# --------------------------------------------------------------------------- +# Helpers / fakes +# --------------------------------------------------------------------------- + + +def make_trial( + task_id: str, + *, + is_resolved: bool, + failure_mode: str = "unset", + input_tokens: Optional[int] = None, + output_tokens: Optional[int] = None, +) -> SimpleNamespace: + """Build a duck-typed terminal-bench 0.2.18 TrialResults.""" + return SimpleNamespace( + task_id=task_id, + trial_name=f"{task_id}.1-of-1", + is_resolved=is_resolved, + failure_mode=SimpleNamespace(value=failure_mode), + total_input_tokens=input_tokens, + total_output_tokens=output_tokens, + ) + + +class FakeHarness: + """Records constructor kwargs; run() returns the configured results.""" + + captured_kwargs: Dict[str, Any] = {} + results: Any = SimpleNamespace(results=[]) + + def __init__(self, **kwargs: Any) -> None: + type(self).captured_kwargs = kwargs + + def run(self) -> Any: + return type(self).results + + +class OldFakeHarness: + """A pre-timeout-kwargs Harness signature (no **kwargs).""" + + def __init__( + self, + output_path: Any = None, + run_id: Any = None, + dataset_name: Any = None, + dataset_version: Any = None, + model_name: Any = None, + n_concurrent_trials: Any = None, + cleanup: Any = None, + agent_name: Any = None, + agent_kwargs: Any = None, + n_tasks: Any = None, + ) -> None: + pass + + def run(self) -> Any: + return SimpleNamespace(results=[]) + + +@pytest.fixture() +def fake_tb_backend(monkeypatch): + """Enable the backend without the real terminal_bench package.""" + FakeHarness.captured_kwargs = {} + FakeHarness.results = SimpleNamespace(results=[]) + monkeypatch.setattr(tbn, "_HAS_TB", True) + monkeypatch.setattr(tbn, "Harness", FakeHarness, raising=False) + + mod_tb = types.ModuleType("terminal_bench") + mod_agents = types.ModuleType("terminal_bench.agents") + mod_agent_name = types.ModuleType("terminal_bench.agents.agent_name") + mod_agent_name.AgentName = lambda name: name + mod_tb.agents = mod_agents + mod_agents.agent_name = mod_agent_name + monkeypatch.setitem(sys.modules, "terminal_bench", mod_tb) + monkeypatch.setitem(sys.modules, "terminal_bench.agents", mod_agents) + monkeypatch.setitem(sys.modules, "terminal_bench.agents.agent_name", mod_agent_name) + return FakeHarness + + +# --------------------------------------------------------------------------- +# Timeout kwargs threading +# --------------------------------------------------------------------------- + + +class TestTimeoutKwargs: + def test_default_bound_reaches_harness(self, fake_tb_backend, tmp_path): + backend = tbn.TerminalBenchNativeBackend(output_dir=str(tmp_path)) + backend.run_harness("run-1") + kwargs = fake_tb_backend.captured_kwargs + assert kwargs["global_agent_timeout_sec"] == 1800.0 + assert "global_timeout_multiplier" not in kwargs + + def test_explicit_values_reach_harness(self, fake_tb_backend, tmp_path): + backend = tbn.TerminalBenchNativeBackend( + output_dir=str(tmp_path), + global_agent_timeout_sec=1234.0, + global_timeout_multiplier=2.0, + ) + backend.run_harness("run-1") + kwargs = fake_tb_backend.captured_kwargs + assert kwargs["global_agent_timeout_sec"] == 1234.0 + assert kwargs["global_timeout_multiplier"] == 2.0 + + def test_zero_disables_bound(self, fake_tb_backend, tmp_path): + backend = tbn.TerminalBenchNativeBackend( + output_dir=str(tmp_path), global_agent_timeout_sec=0 + ) + backend.run_harness("run-1") + assert "global_agent_timeout_sec" not in fake_tb_backend.captured_kwargs + + def test_old_terminal_bench_fails_loud( + self, fake_tb_backend, monkeypatch, tmp_path + ): + """An old Harness without the kwargs must not hang silently.""" + monkeypatch.setattr(tbn, "Harness", OldFakeHarness, raising=False) + backend = tbn.TerminalBenchNativeBackend(output_dir=str(tmp_path)) + with pytest.raises(RuntimeError, match="global_agent_timeout_sec"): + backend.run_harness("run-1") + + +# --------------------------------------------------------------------------- +# Harness-error classification (zero-model-contact detection) +# --------------------------------------------------------------------------- + + +class TestSummarizeBenchmarkResults: + def test_genuine_model_miss_is_not_flagged(self): + """MANDATORY: a real miss (tokens>0, failure_mode unset) stays a miss. + + terminal-bench 0.2.18 leaves failure_mode UNSET on success and on + genuine unresolved misses — classifying on failure_mode would flag + every real miss as a harness error and inflate accuracy. + """ + results = SimpleNamespace( + results=[ + make_trial( + "t-ok", is_resolved=True, input_tokens=900, output_tokens=100 + ), + make_trial( + "t-miss", is_resolved=False, input_tokens=800, output_tokens=50 + ), + make_trial( + "t-setup-dead", + is_resolved=False, + failure_mode="agent_installation_failed", + input_tokens=0, + output_tokens=0, + ), + ] + ) + summary, failures = summarize_benchmark_results(results, model="m") + assert summary.total_samples == 3 + assert summary.scored_samples == 2 # genuine miss stays in denominator + assert summary.correct == 1 + assert summary.accuracy == 0.5 # not 1.0 (miss kept), not 1/3 (infra out) + assert summary.errors == 1 + assert [f["task_id"] for f in failures] == ["t-setup-dead"] + assert failures[0]["reason"] == "zero_model_requests" + + def test_zero_contact_flagged_even_with_unset_failure_mode(self): + """Setup hang signature: unresolved, zero requests, failure_mode unset.""" + results = SimpleNamespace( + results=[ + make_trial("t-hang", is_resolved=False, input_tokens=0), + ] + ) + summary, failures = summarize_benchmark_results(results, model="m") + assert summary.errors == 1 + assert summary.scored_samples == 0 + assert failures[0]["reason"] == "zero_model_requests" + + def test_missing_token_fields_treated_as_zero_contact(self): + results = SimpleNamespace(results=[make_trial("t-none", is_resolved=False)]) + summary, failures = summarize_benchmark_results(results, model="m") + assert summary.errors == 1 + + def test_infra_failure_mode_flagged_despite_tokens(self): + results = SimpleNamespace( + results=[ + make_trial( + "t-crash", + is_resolved=False, + failure_mode="unknown_agent_error", + input_tokens=500, + output_tokens=20, + ), + ] + ) + summary, failures = summarize_benchmark_results(results, model="m") + assert summary.errors == 1 + assert failures[0]["reason"] == "unknown_agent_error" + + def test_resolved_with_zero_tokens_not_flagged(self): + """Installed agents report 0 tokens on success — never flag resolved.""" + results = SimpleNamespace( + results=[ + make_trial("t-ok", is_resolved=True, input_tokens=0, output_tokens=0), + ] + ) + summary, failures = summarize_benchmark_results(results, model="m") + assert summary.errors == 0 + assert summary.correct == 1 + assert summary.accuracy == 1.0 + + def test_empty_results(self): + summary, failures = summarize_benchmark_results( + SimpleNamespace(results=[]), model="m" + ) + assert summary.total_samples == 0 + assert summary.accuracy == 0.0 + assert failures == [] + + +# --------------------------------------------------------------------------- +# CLI wiring: config -> backend -> harness kwargs -> RunSummary +# --------------------------------------------------------------------------- + + +class TestRunTerminalbenchNativeWiring: + def _run(self, fake_tb_backend, tmp_path, trials: List[Any], **config_kwargs): + from rich.console import Console + + from openjarvis.evals.cli import _run_terminalbench_native + from openjarvis.evals.core.types import RunConfig + + fake_tb_backend.results = SimpleNamespace(results=trials) + config = RunConfig( + benchmark="terminalbench-native", + backend="terminalbench-native", + model="test-model", + output_path=str(tmp_path / "out"), + **config_kwargs, + ) + console = Console(record=True, width=120) + summary = _run_terminalbench_native(config, console) + return summary, console.export_text() + + def test_config_timeouts_reach_harness_kwargs(self, fake_tb_backend, tmp_path): + """(d) timeout kwargs travel config -> backend -> harness_kwargs.""" + self._run( + fake_tb_backend, + tmp_path, + [], + global_agent_timeout_sec=901.0, + global_timeout_multiplier=1.5, + ) + kwargs = fake_tb_backend.captured_kwargs + assert kwargs["global_agent_timeout_sec"] == 901.0 + assert kwargs["global_timeout_multiplier"] == 1.5 + + def test_config_defaults_use_backend_bound(self, fake_tb_backend, tmp_path): + self._run(fake_tb_backend, tmp_path, []) + assert fake_tb_backend.captured_kwargs["global_agent_timeout_sec"] == 1800.0 + + def test_summary_counts_real_trials(self, fake_tb_backend, tmp_path): + """Regression: results field is ``results``, not ``trial_results``. + + The old conversion read the nonexistent ``trial_results`` attribute + and hardcoded errors=0, rendering every run as 0 samples / 0.0. + """ + trials = [ + make_trial("t-ok", is_resolved=True, input_tokens=10, output_tokens=10), + make_trial("t-miss", is_resolved=False, input_tokens=10, output_tokens=2), + make_trial( + "t-hang", + is_resolved=False, + failure_mode="agent_timeout", + input_tokens=0, + output_tokens=0, + ), + ] + summary, output = self._run(fake_tb_backend, tmp_path, trials) + assert summary.total_samples == 3 + assert summary.scored_samples == 2 + assert summary.correct == 1 + assert summary.accuracy == 0.5 + assert summary.errors == 1 + # The harness failure is reported loudly with its task id. + assert "t-hang" in output + assert "zero_model_requests" in output