From 45768b74e61c190a82126fe2ae8e757112926867 Mon Sep 17 00:00:00 2001 From: tmatup <51425734+tmatup@users.noreply.github.com> Date: Fri, 14 Aug 2026 23:31:03 +0000 Subject: [PATCH 1/2] feat(evaluation): preserve criteria after agent failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit πŸ€– Generated with Codex Co-Authored-By: [Codex](mailto:noreply@openai.com) --- src/coder_eval/cli/plan_command.py | 5 + .../evaluation/judge_persistence.py | 93 +++++---- src/coder_eval/models/limits.py | 3 + src/coder_eval/models/results.py | 26 ++- src/coder_eval/orchestration/run_limits.py | 34 ++++ src/coder_eval/orchestrator.py | 177 ++++++++++++++++-- tests/test_cost_accounting_paths.py | 20 ++ tests/test_criterion_result_round_trip.py | 49 +++++ tests/test_judge_persistence.py | 31 ++- tests/test_plan_command.py | 30 ++- tests/test_run_limits_models.py | 35 ++++ tests/test_run_limits_orchestrator.py | 2 + tests/test_timeout_orchestrator.py | 117 ++++++++++++ 13 files changed, 556 insertions(+), 66 deletions(-) create mode 100644 src/coder_eval/orchestration/run_limits.py diff --git a/src/coder_eval/cli/plan_command.py b/src/coder_eval/cli/plan_command.py index 264d863d..25318717 100644 --- a/src/coder_eval/cli/plan_command.py +++ b/src/coder_eval/cli/plan_command.py @@ -61,6 +61,7 @@ def plan_command( # Lazy import to avoid circular dependency at module level from ..orchestration.early_stop import EarlyStopConfigError, validate_early_stop from ..orchestration.experiment import DEFAULT_EXPERIMENT_PATH, load_experiment, resolve_task_for_variant + from ..orchestration.run_limits import validate_run_limits # Always load experiment (defaults to experiments/default.yaml) exp_path = experiment if isinstance(experiment, Path) else DEFAULT_EXPERIMENT_PATH @@ -136,6 +137,10 @@ def plan_command( resolved, _lineage, _ = resolve_task_for_variant(default_exp, task, exp_def, variant) # Early-stop guardrails (no-op unless a criterion carries a stop_early: block). validate_early_stop(resolved) + for message in validate_run_limits(resolved): + console.print( + f" [yellow]⚠[/yellow] [yellow]Variant '{variant.variant_id}': {message}[/yellow]" + ) agent_type = str(resolved.agent.type) if resolved.agent else "unknown" agent_model = resolved.agent.model if resolved.agent else None model_str = f" ({agent_model})" if agent_model else "" diff --git a/src/coder_eval/evaluation/judge_persistence.py b/src/coder_eval/evaluation/judge_persistence.py index d600cfb7..10aefcf0 100644 --- a/src/coder_eval/evaluation/judge_persistence.py +++ b/src/coder_eval/evaluation/judge_persistence.py @@ -3,9 +3,9 @@ The full judge transcript (tool calls, raw verdict, rendered prompt and system prompt) can run 10-100 KB. Inlining it into every ``task.json`` inflates the row record for consumers (suite rollups, report renderers) -that don't need it. Spilling each transcript to a sibling -``judge-.yaml`` next to ``task.json`` keeps the row record lean and -lets reviewers grep transcripts independently. +that don't need it. Spilling each transcript to a sibling YAML file next to +``task.json`` keeps the row record lean and lets reviewers grep transcripts +independently. YAML (over JSON) for the sibling: the transcript carries multi-line text (``judge_prompt``, ``judge_system_prompt``, ``raw_verdict``) which YAML's @@ -121,50 +121,48 @@ def _ordered_transcript_dict(transcript_dump: dict[str, Any]) -> dict[str, Any]: def spill_judge_transcripts(result: EvaluationResult, output_dir: Path) -> int: """Write each judge result's inline transcript to a sibling YAML file. - For each ``JudgeCriterionResult`` in ``result.success_criteria_results`` - that carries a non-None ``transcript``, writes ``judge-.yaml`` in - ``output_dir`` (creating the directory if needed) and sets + For each ``JudgeCriterionResult`` in the canonical or post-failure result + list that carries a non-None ``transcript``, writes a distinct sibling YAML + file in ``output_dir`` (creating the directory if needed) and sets ``transcript_path`` on the result to the sibling filename. The inline ``transcript`` is **left in place** so in-memory consumers (HTML rendering at the end of the orchestrator run) still see it. - Callers writing ``task.json`` should pass - ``exclude={"success_criteria_results": {"__all__": {"transcript"}}}`` - to ``model_dump_json`` so the on-disk record carries only the path. + Callers writing ``task.json`` should exclude ``transcript`` from both result + lists so the on-disk record carries only the path. Returns the count of transcripts spilled (informational; no-op when 0). """ output_dir.mkdir(parents=True, exist_ok=True) spilled = 0 - # ORDER IS LOAD-BEARING. ``judge-{idx}.yaml`` is keyed off the criterion's - # position in ``success_criteria_results``; ``load_judge_transcripts`` reads - # ``transcript_path`` (which we set below) to find each sibling, so the - # filename naming scheme itself can change freely. What MUST stay stable is - # the indexβ†’file mapping for the lifetime of any task.json that references - # these siblings: writers that reorder ``success_criteria_results`` between - # spill and read would break the binding. Today's only writer is the - # orchestrator and the order is preserved through model_dump_json/ - # model_validate_json, so this is safe β€” keep it that way. - for idx, cr in enumerate(result.success_criteria_results): - if not isinstance(cr, JudgeCriterionResult): - continue - if cr.transcript is None: - continue - sibling_name = f"judge-{idx}.yaml" - sibling_path = output_dir / sibling_name - ordered = _ordered_transcript_dict(cr.transcript.model_dump()) - sibling_path.write_text( - yaml.dump( - ordered, - Dumper=_BlockLiteralDumper, - sort_keys=False, - allow_unicode=True, - width=100, - ), - encoding="utf-8", - ) - cr.transcript_path = sibling_name - spilled += 1 + # ORDER IS LOAD-BEARING. Each filename is keyed off the criterion's + # position in its result list; ``load_judge_transcripts`` reads the stored + # path, so each list must retain its order through persistence. + result_groups = ( + ("judge", result.success_criteria_results), + ("post-failure-judge", result.post_failure_criteria_results), + ) + for prefix, criteria_results in result_groups: + for idx, cr in enumerate(criteria_results): + if not isinstance(cr, JudgeCriterionResult): + continue + if cr.transcript is None: + continue + sibling_name = f"{prefix}-{idx}.yaml" + sibling_path = output_dir / sibling_name + ordered = _ordered_transcript_dict(cr.transcript.model_dump()) + sibling_path.write_text( + yaml.dump( + ordered, + Dumper=_BlockLiteralDumper, + sort_keys=False, + allow_unicode=True, + width=100, + ), + encoding="utf-8", + ) + cr.transcript_path = sibling_name + spilled += 1 if spilled: logger.debug("spilled %d judge transcript(s) to %s", spilled, output_dir) return spilled @@ -173,11 +171,11 @@ def spill_judge_transcripts(result: EvaluationResult, output_dir: Path) -> int: def load_judge_transcripts(result: EvaluationResult, task_dir: Path) -> int: """Read sibling judge transcript files and attach them to each result. - For each criterion result in ``result.success_criteria_results`` that has - a ``transcript_path`` set (and no inline ``transcript`` β€” already-loaded + For each criterion result in either result list that has a + ``transcript_path`` set (and no inline ``transcript`` β€” already-loaded results are left alone), reads the sibling file relative to ``task_dir`` - and attaches the parsed dict on ``transcript`` so HTML / markdown - renderers see the same shape they get during the original run. + and attaches the parsed dict on ``transcript`` so HTML / markdown renderers + see the same shape they get during the original run. Missing sibling files are skipped silently and logged at debug level β€” runs predating this feature have no sibling files and render fine via @@ -187,7 +185,8 @@ def load_judge_transcripts(result: EvaluationResult, task_dir: Path) -> int: Returns the count of transcripts loaded. """ loaded = 0 - for cr in result.success_criteria_results: + criterion_results = result.success_criteria_results + result.post_failure_criteria_results + for cr in criterion_results: path = getattr(cr, "transcript_path", None) if not path: continue @@ -198,8 +197,8 @@ def load_judge_transcripts(result: EvaluationResult, task_dir: Path) -> int: continue # SECURITY: transcript_path comes from task.json, which may travel across # trust boundaries (CI artifacts, shared eval bundles). spill_judge_transcripts - # only ever writes the literal ``f"judge-{idx}.yaml"`` β€” a basename, no - # separators, no ``..``. Allowlist the basename shape directly so a tampered + # only ever writes generated basename-only paths, with no separators or + # ``..``. Allowlist that shape directly so a tampered # ``transcript_path: '/etc/passwd'`` or ``../../secrets`` is refused at the # door rather than relying on ``is_relative_to`` to catch it after a join. # Check BOTH PurePosixPath (forward-slash separator) AND PureWindowsPath @@ -275,8 +274,8 @@ def load_judge_transcripts(result: EvaluationResult, task_dir: Path) -> int: # which (depending on model_config of the loaded subclass) might # validate or reject. The HTML renderer accepts both typed # JudgeTranscript and dict-shape so either shape works downstream. - # NOTE: With the ``CriterionResultUnion`` discriminator on - # ``EvaluationResult.success_criteria_results``, ``cr`` is now a + # NOTE: With the ``CriterionResultUnion`` discriminator on both + # ``EvaluationResult`` criterion-result lists, ``cr`` is now a # properly-typed ``JudgeCriterionResult`` after reload (not a base # ``CriterionResult`` with the field in ``__pydantic_extra__``), so # the assignment lands on the declared field directly. diff --git a/src/coder_eval/models/limits.py b/src/coder_eval/models/limits.py index e6febfea..6cafb5f6 100644 --- a/src/coder_eval/models/limits.py +++ b/src/coder_eval/models/limits.py @@ -154,3 +154,6 @@ class RunLimits(BaseModel): # plan_command's generic per-variant "resolution failed" branch, which # prints red text but does NOT flip the exit code by design (unlike # EarlyStopConfigError), so a model-level raise would silently pass CI. + # Other cross-field semantics that are warnings rather than errors live in + # orchestration/run_limits.py::validate_run_limits for the same post-merge + # visibility without rejecting or mutating the resolved values. diff --git a/src/coder_eval/models/results.py b/src/coder_eval/models/results.py index 2f03222e..212ab667 100644 --- a/src/coder_eval/models/results.py +++ b/src/coder_eval/models/results.py @@ -82,6 +82,15 @@ class CriterionResult(BaseModel): ) details: str | None = Field(default=None, description="Additional details about the result") error: str | None = Field(default=None, description="Error message if the check failed") + evaluation_status: Literal["evaluated", "not_evaluated"] = Field( + default="evaluated", + description=( + "Whether the criterion ran. ``not_evaluated`` is distinct from an evaluated " + "criterion whose score is 0.0 or whose checker returned an error. Defaults to " + "``evaluated`` so task.json files written before this field existed retain their " + "original meaning." + ), + ) pass_threshold: float = Field( default=0.9, ge=0.0, @@ -531,6 +540,16 @@ class EvaluationResult(BaseModel): "files without ``result_kind`` are inferred from ``criterion_type``." ), ) + post_failure_criteria_results: list[CriterionResultUnion] = Field( + default_factory=list, + description=( + "Diagnostic criterion evidence collected after a terminal agent failure while the " + "sandbox is still readable. These results are intentionally separate from " + "success_criteria_results: they do not affect weighted_score, task gating, or suite " + "aggregation. A result with evaluation_status='not_evaluated' records that its " + "required inputs or remaining task-timeout budget were unavailable." + ), + ) # Detailed transcript iterations: list[TurnRecord] = Field( @@ -952,9 +971,12 @@ def judge_cost_usd(result: EvaluationResult) -> float | None: Covers both flavors: ``llm_judge`` prices its own one-shot call from the criterion's model, ``agent_judge`` inherits the SDK's cost on the sub-agent's - turn. ``None`` when no criterion reported cost. + turn. Post-failure diagnostic judges are included because their calls still + incur real spend even though their results cannot affect the canonical score. + ``None`` when no criterion reported cost. """ - usages = [u for cr in result.success_criteria_results if (u := getattr(cr, "token_usage", None)) is not None] + criterion_results = result.success_criteria_results + result.post_failure_criteria_results + usages = [u for cr in criterion_results if (u := getattr(cr, "token_usage", None)) is not None] return sum_costs(*(u.total_cost_usd for u in usages)) diff --git a/src/coder_eval/orchestration/run_limits.py b/src/coder_eval/orchestration/run_limits.py new file mode 100644 index 00000000..6a0161b3 --- /dev/null +++ b/src/coder_eval/orchestration/run_limits.py @@ -0,0 +1,34 @@ +"""Post-merge validation for cross-field run-limit semantics.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + + +if TYPE_CHECKING: + from coder_eval.models import TaskDefinition + + +INEFFECTIVE_TASK_TIMEOUT_WARNING = ( + "A larger task_timeout cannot extend the agent's single iteration; the agent budget is turn_timeout." +) + + +def validate_run_limits(task: TaskDefinition) -> tuple[str, ...]: + """Return non-blocking warnings for the fully resolved run limits. + + The comparison belongs after config merge because either timeout may come + from any of the five layers. The warning is about one agent call: even when + dialog simulation makes several calls, a larger task-wide timeout cannot + extend any call beyond its turn timeout. + """ + limits = task.run_limits + if limits is None or limits.task_timeout is None or limits.turn_timeout is None: + return () + if limits.task_timeout <= limits.turn_timeout: + return () + return ( + f"run_limits.task_timeout ({limits.task_timeout}s) exceeds " + + f"run_limits.turn_timeout ({limits.turn_timeout}s). " + + INEFFECTIVE_TASK_TIMEOUT_WARNING, + ) diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 83f67da3..7054aa76 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -22,6 +22,8 @@ from .errors import ( AgentCrashError, BudgetExceededError, + CheckerMisuseError, + JudgeInfrastructureError, TaskTimeoutError, TurnTimeoutError, ) @@ -36,6 +38,7 @@ ApiRoute, BedrockRoute, ConfigLineageEntry, + CriteriaResults, CriterionResult, DirectRoute, EvaluationResult, @@ -48,6 +51,7 @@ PreservationMode, SimulationConfig, SimulationTelemetry, + SuccessCriterion, TaskConfigRecord, TaskDefinition, TokenUsage, @@ -58,6 +62,7 @@ ) from .orchestration.early_stop import EarlyStopWatcher, early_stop_active, validate_early_stop from .orchestration.evaluation import load_reference +from .orchestration.run_limits import validate_run_limits from .path_utils import format_task_log_id, task_log_path from .sandbox import Sandbox from .simulation import DialogStopReason, SimulatorResult, UserSimulator, evaluate_stop @@ -403,6 +408,10 @@ def __init__( # task run even though _check_expected_turns is called after every turn. self._expected_turns_warning_emitted: bool = False + # One-shot flag: a resolved task may be inspected more than once during + # setup, but its ineffective timeout relationship should be logged once. + self._run_limits_warning_emitted: bool = False + # Canonical id shared with run_dir layout, tqdm label, and streaming events. self._log_task_id = format_task_log_id(variant_id, task.task_id, replicate_index) @@ -494,16 +503,11 @@ def _kill_agent_subprocess_sync() -> None: asyncio_task_to_cancel=asyncio.current_task(), label=f"task_timeout ({self.task.task_id})", ) as wd: - try: - success = await self._evaluation_loop() - except asyncio.CancelledError: - if wd.fired: - raise TaskTimeoutError( - task_timeout or 0, - task_id=self.task.task_id, - elapsed_seconds=time.time() - start_time, - ) from None - raise + success = await self._run_evaluation_with_failure_evidence( + watchdog=wd, + task_timeout=task_timeout, + start_time=start_time, + ) # Belt-and-suspenders: if the loop returned normally but the # watchdog fired during post-loop work or the inner coro # swallowed the cancel, still classify as TIMEOUT. @@ -628,6 +632,139 @@ def _kill_agent_subprocess_sync() -> None: return self.result + async def _run_evaluation_with_failure_evidence( + self, + *, + watchdog: ThreadedWatchdog, + task_timeout: int | None, + start_time: float, + ) -> bool: + """Run the loop and collect diagnostics before its watchdog closes.""" + try: + return await self._evaluation_loop() + except asyncio.CancelledError: + if watchdog.fired: + self._record_post_failure_not_evaluated( + "the task_timeout budget was exhausted before post-failure grading could run" + ) + raise TaskTimeoutError( + task_timeout or 0, + task_id=self.task.task_id, + elapsed_seconds=time.time() - start_time, + ) from None + raise + except TaskTimeoutError: + self._record_post_failure_not_evaluated( + "the task_timeout budget was exhausted before post-failure grading could run" + ) + raise + except (AgentCrashError, TurnTimeoutError, BudgetExceededError) as terminal_error: + if ( + isinstance(terminal_error, BudgetExceededError) + and self.result is not None + and len(self.result.success_criteria_results) == len(self.task.success_criteria) + ): + raise + try: + await self._evaluate_post_failure_criteria() + except asyncio.CancelledError: + if watchdog.fired: + self._record_post_failure_not_evaluated( + "the task_timeout budget expired during post-failure grading" + ) + raise TaskTimeoutError( + task_timeout or 0, + task_id=self.task.task_id, + elapsed_seconds=time.time() - start_time, + ) from None + raise + except (JudgeInfrastructureError, CheckerMisuseError): + raise + except Exception as recovery_error: + self._record_post_failure_not_evaluated( + f"post-failure grading could not complete ({type(recovery_error).__name__})" + ) + logger.warning( + "[%s] Post-failure criteria evaluation failed; preserving the original terminal error", + self.task.task_id, + exc_info=True, + ) + raise + + @staticmethod + def _not_evaluated_result(criterion: SuccessCriterion, reason: str) -> CriterionResult: + return CriterionResult( + criterion_type=criterion.type, + description=criterion.description, + score=0.0, + details=f"Not evaluated after terminal agent failure: {reason}.", + evaluation_status="not_evaluated", + pass_threshold=criterion.pass_threshold, + gating=criterion.is_gating, + ) + + def _record_post_failure_not_evaluated(self, reason: str) -> None: + """Record a full diagnostic result vector when recovery cannot run.""" + if self.result is None: + return + self.result.post_failure_criteria_results = [ + self._not_evaluated_result(criterion, reason) for criterion in self.task.success_criteria + ] + + async def _evaluate_post_failure_criteria(self) -> None: + """Evaluate diagnostic criteria before the live sandbox is torn down. + + Results stay outside the canonical scored list. Agent-dependent checks + require at least one preserved turn; artifact-only checks can still run + when the agent failed before producing trajectory evidence. + """ + if self.result is None: + return + if self.success_checker is None or self.sandbox is None: + self._record_post_failure_not_evaluated("the sandbox or success checker was unavailable") + return + + runnable: list[SuccessCriterion] = [] + unavailable_positions: set[int] = set() + for position, criterion in enumerate(self.task.success_criteria): + if criterion.requires_agent and not self.result.iterations: + unavailable_positions.add(position) + else: + runnable.append(criterion) + + checked: CriteriaResults = [] + if runnable: + reference_code, reference_dir, self._reference_code = load_reference( + task=self.task, + task_file=self.task_file, + cached_reference=self._reference_code, + ) + checked = await self.success_checker.check_all_async( + runnable, + reference_code=reference_code, + reference_dir=reference_dir, + turn_records=self.result.iterations, + ) + + if len(checked) != len(runnable): + raise ValueError( + f"Post-failure checker returned {len(checked)} results for {len(runnable)} runnable criteria" + ) + + checked_iter = iter(checked) + recovered: CriteriaResults = [] + for position, criterion in enumerate(self.task.success_criteria): + if position in unavailable_positions: + recovered.append( + self._not_evaluated_result( + criterion, + "no turn record survived for this agent-dependent criterion", + ) + ) + else: + recovered.append(next(checked_iter)) + self.result.post_failure_criteria_results = recovered + async def _drain_killed_turn(self) -> None: """Move a hard-killed turn's partial record from the agent onto the result. @@ -773,7 +910,7 @@ def _finalize_result(self, start_time: float) -> None: # Persist self.report_path.parent.mkdir(parents=True, exist_ok=True) # noqa: CE002 β€” mkdir on local FS is nanoseconds - # Spill any judge transcripts to sibling judge-.yaml files BEFORE + # Spill any judge transcripts to sibling YAML files BEFORE # we dump task.json, so transcript_path is set on each judge result. # The inline `transcript` field stays in memory β€” HTML rendering below # uses it directly. We strip it from the JSON dump via `exclude=...`. @@ -791,11 +928,14 @@ def _finalize_result(self, start_time: float) -> None: report_tmp.write_text( # noqa: CE002 β€” small JSON write at end of run self.result.model_dump_json( indent=2, - # Strip inline transcripts: they live in sibling judge-.yaml + # Strip inline transcripts: they live in sibling YAML files # next to task.json, referenced by transcript_path. Excluding # `transcript` here avoids ~20-100 KB of bloat per judge result # in the row record without losing any data. - exclude={"success_criteria_results": {"__all__": {"transcript"}}}, + exclude={ + "success_criteria_results": {"__all__": {"transcript"}}, + "post_failure_criteria_results": {"__all__": {"transcript"}}, + }, ), encoding="utf-8", ) @@ -907,6 +1047,16 @@ def _check_expected_turns(self, *, iteration: int) -> None: ) self._expected_turns_warning_emitted = True + def _warn_on_ineffective_task_timeout(self) -> None: + """Log resolved cross-field run-limit warnings once per task run.""" + if self._run_limits_warning_emitted: + return + messages = validate_run_limits(self.task) + for message in messages: + logger.warning("[%s] %s", self.task.task_id, message) + if messages: + self._run_limits_warning_emitted = True + @property def _cost_correlation_run_id(self) -> str: """The LiteLLM cost-log correlation run id β€” a stable hash of the run dir. @@ -988,6 +1138,7 @@ async def _setup(self) -> None: # paths (the CLI already validated during resolution). No-op unless # some criterion carries a stop_early: block. validate_early_stop(self.task) + self._warn_on_ineffective_task_timeout() # Build the early-stop watcher once, up front, when armed (>= 1 criterion # with a stop_early: block and the run_limits.stop_early kill switch not diff --git a/tests/test_cost_accounting_paths.py b/tests/test_cost_accounting_paths.py index 9193f7dc..98ac1b6f 100644 --- a/tests/test_cost_accounting_paths.py +++ b/tests/test_cost_accounting_paths.py @@ -188,6 +188,26 @@ def test_judge_cost_rolls_up_onto_the_row(self): assert row["total_cost_usd"] == pytest.approx(0.15) assert row["agent_cost_usd"] == pytest.approx(0.1) + def test_post_failure_judge_cost_rolls_up_without_affecting_score(self): + result = _result([_turn(1, TokenUsage(uncached_input_tokens=10, output_tokens=1, total_cost_usd=0.1))]) + result.final_status = FinalStatus.ERROR + result.weighted_score = 0.0 + result.total_token_usage = TokenUsage(uncached_input_tokens=10, output_tokens=1, total_cost_usd=0.1) + result.post_failure_criteria_results = [ + JudgeCriterionResult( + criterion_type="llm_judge", + description="diagnostic", + score=1.0, + token_usage=TokenUsage(uncached_input_tokens=5000, output_tokens=500, total_cost_usd=0.02), + ) + ] + + row = eval_result_to_task_dict(result) + + assert row["judge_cost_usd"] == pytest.approx(0.02) + assert row["total_cost_usd"] == pytest.approx(0.12) + assert row["weighted_score"] == 0.0 + def test_no_judge_means_no_judge_cost(self): """None, not 0.0 β€” 'no judge ran' must stay distinct from 'a judge ran free'.""" result = _result([_turn(1, TokenUsage(uncached_input_tokens=10, output_tokens=1, total_cost_usd=0.1))]) diff --git a/tests/test_criterion_result_round_trip.py b/tests/test_criterion_result_round_trip.py index 4d03a828..26d882dc 100644 --- a/tests/test_criterion_result_round_trip.py +++ b/tests/test_criterion_result_round_trip.py @@ -198,3 +198,52 @@ def test_mixed_result_types_in_one_list() -> None: reloaded = EvaluationResult.model_validate_json(er.model_dump_json()) types = [type(r).__name__ for r in reloaded.success_criteria_results] assert types == ["JudgeCriterionResult", "ClassificationCriterionResult", "CriterionResult"] + + +def test_legacy_result_defaults_to_evaluated_and_no_post_failure_evidence() -> None: + """Old task.json payloads keep their meaning when the new fields are absent.""" + legacy_payload = { + "task_id": "t", + "task_description": "d", + "agent_type": "claude-code", + "started_at": "2026-05-12T00:00:00", + "final_status": "ERROR", + "iteration_count": 1, + "success_criteria_results": [ + {"criterion_type": "file_exists", "description": "f", "score": 0.0}, + ], + } + + result = EvaluationResult.model_validate(legacy_payload) + + assert result.success_criteria_results[0].evaluation_status == "evaluated" + assert result.post_failure_criteria_results == [] + + +def test_post_failure_evaluation_status_round_trip() -> None: + result = _make_eval([]) + result.final_status = FinalStatus.ERROR + result.weighted_score = 0.0 + result.post_failure_criteria_results = [ + CriterionResult( + criterion_type="file_exists", + description="artifact exists", + score=1.0, + evaluation_status="evaluated", + ), + CriterionResult( + criterion_type="command_executed", + description="agent ran validator", + score=0.0, + details="Not evaluated after terminal agent failure: no turn record survived.", + evaluation_status="not_evaluated", + ), + ] + + reloaded = EvaluationResult.model_validate_json(result.model_dump_json()) + + assert reloaded.weighted_score == 0.0 + assert [r.evaluation_status for r in reloaded.post_failure_criteria_results] == [ + "evaluated", + "not_evaluated", + ] diff --git a/tests/test_judge_persistence.py b/tests/test_judge_persistence.py index 4d02bb03..7eb183cd 100644 --- a/tests/test_judge_persistence.py +++ b/tests/test_judge_persistence.py @@ -1,6 +1,6 @@ """Tests for the spill/load helpers in ``coder_eval.evaluation.judge_persistence``. -The orchestrator spills judge transcripts to ``judge-.json`` next to +The orchestrator spills judge transcripts to sibling YAML files next to ``task.json`` so the row record stays lean. Re-render paths reload them. These tests verify the round-trip and back-compat with old runs that inlined the transcript. @@ -133,6 +133,31 @@ def test_spill_preserves_index_for_multiple_judges(tmp_path: Path) -> None: assert (tmp_path / "judge-1.yaml").is_file() +def test_post_failure_judge_uses_distinct_sibling_and_round_trips(tmp_path: Path) -> None: + judge = _make_judge_result(transcript=_make_transcript()) + result = _make_evaluation_result(criteria=[]) + result.final_status = FinalStatus.ERROR + result.post_failure_criteria_results = [judge] + + assert spill_judge_transcripts(result, tmp_path) == 1 + assert judge.transcript_path == "post-failure-judge-0.yaml" + + raw = result.model_dump_json( + exclude={ + "success_criteria_results": {"__all__": {"transcript"}}, + "post_failure_criteria_results": {"__all__": {"transcript"}}, + } + ) + assert "raw_verdict" not in raw + + reloaded = EvaluationResult.model_validate_json(raw) + assert load_judge_transcripts(reloaded, tmp_path) == 1 + recovered = reloaded.post_failure_criteria_results[0] + assert isinstance(recovered, JudgeCriterionResult) + assert recovered.transcript is not None + assert recovered.transcript.raw_verdict == '{"score":0.75,"rationale":"ok"}' + + def test_spill_skips_non_judge_results(tmp_path: Path) -> None: """Plain CriterionResult instances are no-ops β€” no sibling file written.""" plain = CriterionResult( @@ -306,8 +331,8 @@ def test_load_rejects_dotdot_traversal(tmp_path: Path) -> None: def test_load_rejects_subdir_path(tmp_path: Path) -> None: """A path with a separator (even within task_dir) is rejected β€” the spill helper - only ever writes ``judge-.yaml`` as a basename, so anything with a slash is - by definition not from us.""" + only ever writes basenames, so anything with a slash is by definition not + from us.""" judge = _make_judge_result(transcript=None) judge.transcript_path = "subdir/judge-0.yaml" result = _make_evaluation_result(criteria=[judge]) diff --git a/tests/test_plan_command.py b/tests/test_plan_command.py index 51896846..16a8d663 100644 --- a/tests/test_plan_command.py +++ b/tests/test_plan_command.py @@ -7,7 +7,14 @@ import typer from coder_eval.cli.plan_command import plan_command -from coder_eval.models import AgentConfig, ExperimentDefinition, ExperimentVariant, TaskDefinition, parse_agent_config +from coder_eval.models import ( + AgentConfig, + ExperimentDefinition, + ExperimentVariant, + RunLimits, + TaskDefinition, + parse_agent_config, +) from coder_eval.models.enums import AgentKind @@ -209,6 +216,27 @@ def test_plan_with_default_experiment(self, tmp_path: Path) -> None: printed = " ".join(str(call) for call in mock_console.print.call_args_list) assert "test-exp" in printed + def test_plan_warns_when_task_timeout_cannot_extend_single_iteration(self, tmp_path: Path) -> None: + task_file = tmp_path / "task.yaml" + task_file.write_text("placeholder") + experiment = _make_experiment(variants=[ExperimentVariant(variant_id="default")]) + task = _make_task(agent=parse_agent_config(type=AgentKind.CLAUDE_CODE)) + resolved_task = task.model_copy(update={"run_limits": RunLimits(task_timeout=1500, turn_timeout=1200)}) + + with ( + patch("coder_eval.cli.plan_command.check_tools"), + patch("coder_eval.cli.plan_command.check_api_keys"), + patch("coder_eval.cli.plan_command.load_task", return_value=(task, "mock yaml")), + patch(f"{_EXP}.load_experiment", return_value=experiment), + patch(f"{_EXP}.resolve_task_for_variant", return_value=(resolved_task, {}, 1)), + patch("coder_eval.cli.plan_command.console") as mock_console, + ): + plan_command(task_files=[task_file]) + + printed = " ".join(str(call) for call in mock_console.print.call_args_list) + assert "A larger task_timeout cannot extend the agent's single iteration" in printed + assert "the agent budget is turn_timeout" in printed + def test_plan_exits_when_default_experiment_missing(self, tmp_path: Path) -> None: """When default experiment file is missing and no --experiment given, plan should exit.""" task_file = tmp_path / "task.yaml" diff --git a/tests/test_run_limits_models.py b/tests/test_run_limits_models.py index e6d6d6be..12241a1c 100644 --- a/tests/test_run_limits_models.py +++ b/tests/test_run_limits_models.py @@ -11,6 +11,7 @@ RunLimits, TaskDefinition, ) +from coder_eval.orchestration.run_limits import INEFFECTIVE_TASK_TIMEOUT_WARNING, validate_run_limits def _minimal_task(**overrides) -> TaskDefinition: @@ -129,6 +130,40 @@ def test_extra_forbid_still_rejects_unknowns(self): RunLimits.model_validate({"expected_turn": 5}) +class TestRunLimitsCrossFieldWarnings: + def test_warning_wording_states_the_single_iteration_semantic(self): + assert INEFFECTIVE_TASK_TIMEOUT_WARNING == ( + "A larger task_timeout cannot extend the agent's single iteration; the agent budget is turn_timeout." + ) + + @pytest.mark.parametrize( + ("task_timeout", "turn_timeout", "warns"), + [ + (121, 120, True), + (120, 120, False), + (119, 120, False), + (None, 120, False), + (120, None, False), + ], + ) + def test_warns_only_when_task_timeout_exceeds_turn_timeout(self, task_timeout, turn_timeout, warns): + task = _minimal_task(run_limits={"task_timeout": task_timeout, "turn_timeout": turn_timeout}) + + messages = validate_run_limits(task) + + assert bool(messages) is warns + if warns: + assert INEFFECTIVE_TASK_TIMEOUT_WARNING in messages[0] + + def test_dialog_simulation_still_warns_for_each_agent_call(self): + task = _minimal_task( + run_limits={"task_timeout": 121, "turn_timeout": 120}, + simulation={"enabled": True, "persona": "user", "goal": "finish"}, + ) + + assert INEFFECTIVE_TASK_TIMEOUT_WARNING in validate_run_limits(task)[0] + + class TestRunLimitsOnTaskDefinition: def test_default_is_none(self): assert _minimal_task().run_limits is None diff --git a/tests/test_run_limits_orchestrator.py b/tests/test_run_limits_orchestrator.py index 228168f5..2245d25f 100644 --- a/tests/test_run_limits_orchestrator.py +++ b/tests/test_run_limits_orchestrator.py @@ -281,6 +281,8 @@ async def test_run_arm_maps_budget_to_status( assert "budget exceeded" in (result.error_message or "") # Captured error_log_tail key allowlist must include both new statuses. assert result.error_details == {} + assert len(result.post_failure_criteria_results) == 1 + assert result.post_failure_criteria_results[0].evaluation_status == "not_evaluated" # Inspect the actual create_error_context call to confirm the component label. assert mock_ctx.call_args.kwargs["component"] == expected_component diff --git a/tests/test_timeout_orchestrator.py b/tests/test_timeout_orchestrator.py index 2bca85b1..de36073f 100644 --- a/tests/test_timeout_orchestrator.py +++ b/tests/test_timeout_orchestrator.py @@ -7,10 +7,12 @@ import pytest +from coder_eval.errors import JudgeInfrastructureError from coder_eval.errors.timeout import TaskTimeoutError, TurnTimeoutError from coder_eval.models import ( AgentKind, ClaudeCodeAgentConfig, + CommandExecutedCriterion, CriterionResult, EvaluationResult, FileExistsCriterion, @@ -147,6 +149,8 @@ async def slow_loop(): result = await orchestrator.run() assert result.final_status == "TIMEOUT" assert f"Task timed out after {task_timeout}s" in (result.error_message or "") + assert len(result.post_failure_criteria_results) == 1 + assert result.post_failure_criteria_results[0].evaluation_status == "not_evaluated" @pytest.mark.asyncio @@ -331,6 +335,119 @@ async def turn_out_communicate(_prompt, **kwargs): await orchestrator._evaluation_loop() +@pytest.mark.asyncio +async def test_turn_timeout_records_post_failure_evidence_without_rescoring(tmp_path) -> None: + """A terminal turn timeout preserves artifact truth without changing the ERROR score.""" + task = _make_task(turn_timeout=1200, task_timeout=1500) + task.success_criteria = [ + FileExistsCriterion(type="file_exists", path="artifact.txt", description="artifact exists"), + CommandExecutedCriterion( + type="command_executed", + tool_name="Bash", + description="agent ran validator", + ), + ] + run_dir = tmp_path / "run" / "post_failure_evidence" + run_dir.mkdir(parents=True) + orchestrator = Orchestrator(task=task, run_dir=run_dir, variant_id="test-variant") + orchestrator._setup = AsyncMock() # type: ignore[method-assign] + orchestrator._cleanup = AsyncMock() # type: ignore[method-assign] + orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] + orchestrator._evaluation_loop = AsyncMock( # type: ignore[method-assign] + side_effect=TurnTimeoutError(1200, task_id=task.task_id, iteration=1) + ) + + mock_sandbox = MagicMock() + mock_sandbox.sandbox_dir = tmp_path / "sandbox" + mock_sandbox.sandbox_dir.mkdir() + orchestrator.sandbox = mock_sandbox + + mock_checker = MagicMock() + mock_checker.check_all_async = AsyncMock( + return_value=[ + CriterionResult( + criterion_type="file_exists", + description="artifact exists", + score=1.0, + ) + ] + ) + orchestrator.success_checker = mock_checker + + mock_agent = MagicMock() + mock_agent.kill_sync = MagicMock() + mock_agent.get_sdk_options = MagicMock(return_value=None) + orchestrator.agent = mock_agent + + with patch("coder_eval.orchestrator.load_reference", return_value=(None, None, None)): + result = await orchestrator.run() + + assert result.final_status == "ERROR" + assert result.weighted_score == 0.0 + assert result.success_criteria_results == [] + assert len(result.post_failure_criteria_results) == 2 + artifact, agent_dependent = result.post_failure_criteria_results + assert artifact.score == 1.0 + assert artifact.evaluation_status == "evaluated" + assert agent_dependent.score == 0.0 + assert agent_dependent.evaluation_status == "not_evaluated" + assert "no turn record survived" in (agent_dependent.details or "") + + checked_criteria = mock_checker.check_all_async.await_args.args[0] + assert [criterion.type for criterion in checked_criteria] == ["file_exists"] + + persisted = EvaluationResult.model_validate_json((run_dir / "task.json").read_text()) + assert persisted.final_status == "ERROR" + assert persisted.weighted_score == 0.0 + assert [r.evaluation_status for r in persisted.post_failure_criteria_results] == [ + "evaluated", + "not_evaluated", + ] + + +@pytest.mark.asyncio +async def test_post_failure_judge_infrastructure_error_still_escalates(tmp_path) -> None: + task = _make_task(turn_timeout=1200, task_timeout=1500) + task.success_criteria = [ + FileExistsCriterion(type="file_exists", path="artifact.txt", description="artifact exists") + ] + orchestrator = Orchestrator(task=task, run_dir=tmp_path / "run", variant_id="test-variant") + orchestrator._setup = AsyncMock() # type: ignore[method-assign] + orchestrator._cleanup = AsyncMock() # type: ignore[method-assign] + orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] + orchestrator._evaluation_loop = AsyncMock( # type: ignore[method-assign] + side_effect=TurnTimeoutError(1200, task_id=task.task_id, iteration=1) + ) + orchestrator.sandbox = MagicMock() + orchestrator.success_checker = MagicMock() + orchestrator.success_checker.check_all_async = AsyncMock(side_effect=JudgeInfrastructureError("judge unavailable")) + orchestrator.agent = MagicMock() + orchestrator.agent.get_sdk_options.return_value = None + + with patch("coder_eval.orchestrator.load_reference", return_value=(None, None, None)): + result = await orchestrator.run() + + assert result.final_status == "ERROR" + assert result.error_message == "judge unavailable" + assert result.weighted_score == 0.0 + + +def test_runtime_timeout_warning_is_emitted_once(tmp_path, caplog) -> None: + import logging + + task = _make_task(turn_timeout=1200, task_timeout=1500) + orchestrator = Orchestrator(task=task, run_dir=tmp_path / "run", variant_id="test-variant") + + with caplog.at_level(logging.WARNING, logger="coder_eval.orchestrator"): + orchestrator._warn_on_ineffective_task_timeout() + orchestrator._warn_on_ineffective_task_timeout() + + messages = [record.message for record in caplog.records if "single iteration" in record.message] + assert len(messages) == 1 + assert "A larger task_timeout cannot extend the agent's single iteration" in messages[0] + assert "the agent budget is turn_timeout" in messages[0] + + @pytest.mark.asyncio async def test_task_timeout_fires_when_inner_coro_swallows_cancel(tmp_path) -> None: """Belt-and-suspenders: if ``_evaluation_loop`` catches ``CancelledError`` From fbf38572ac9d7289ef728cab0c0a31ed9287884f Mon Sep 17 00:00:00 2001 From: tmatup <51425734+tmatup@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:18:59 +0000 Subject: [PATCH 2/2] fix(evaluation): harden post-failure evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restrict recovery grading to deterministic read-only checks, preserve the original terminal failure, and surface diagnostic evidence across report consumers. πŸ€– Generated with Codex Co-Authored-By: [Codex](mailto:noreply@openai.com) --- docs/REPORT_SCHEMA.md | 20 ++- docs/TASK_DEFINITION_GUIDE.md | 11 +- .../[...task]/__tests__/criteria.test.tsx | 49 ++++++ .../app/runs/[id]/[...task]/_sections.tsx | 39 ++++- evalboard/app/runs/[id]/[...task]/page.tsx | 7 + evalboard/lib/__tests__/providerCalls.test.ts | 12 ++ evalboard/lib/__tests__/runs.test.ts | 34 ++++ evalboard/lib/runs.ts | 55 ++++--- .../evaluation/judge_persistence.py | 8 +- src/coder_eval/models/criteria.py | 10 ++ src/coder_eval/models/results.py | 11 +- src/coder_eval/orchestrator.py | 51 +++--- src/coder_eval/reports_html.py | 66 ++++++-- src/coder_eval/reports_junit.py | 83 ++++++---- tests/test_judge_persistence.py | 43 +++-- tests/test_reports_html.py | 30 ++++ tests/test_reports_junit.py | 43 ++++- tests/test_run_limits_orchestrator.py | 24 +++ tests/test_timeout_orchestrator.py | 150 ++++++++++++++---- 19 files changed, 586 insertions(+), 160 deletions(-) create mode 100644 evalboard/app/runs/[id]/[...task]/__tests__/criteria.test.tsx diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index b656cae9..f9d5554a 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -28,7 +28,8 @@ read). Times are ISO-8601. | `/variant.json` / `.md` | `VariantAggregate` | Per variant | `` is the zero-padded replicate index. Judge transcripts spill to sibling files -(e.g. `judge-0.yaml`) referenced by a `transcript_path`. +(`judge-N.yaml`, or `post-failure-judge-N.yaml` for diagnostic records) referenced +by a `transcript_path`. --- @@ -126,6 +127,7 @@ The authoritative per-replicate record. | `max_turns_exhausted` | `bool` | Ran out of turns. | | `iteration_count` | `int` | Number of turns. | | `success_criteria_results` | `list[CriterionResult]` | Per-criterion results β€” see [below](#criterionresult). | +| `post_failure_criteria_results` | `list[CriterionResult]` | Diagnostic artifact evidence collected after a terminal agent failure. It does not affect `final_status`, `weighted_score`, gating, or suite aggregation. | **Transcript:** `iterations: list[TurnRecord]` (accepts legacy alias `turns`) β€” see [TurnRecord](#turnrecord). @@ -166,6 +168,8 @@ files without `result_kind` are inferred from `criterion_type`. Base fields (`result_kind="basic"`): `criterion_type`, `description`, `score` (0.0–1.0), `details`, `error`, +`evaluation_status` (`evaluated` by default; `not_evaluated` means the check did +not run and is distinct from an evaluated 0.0), `pass_threshold` (default 0.9), `gating` (default `true`; `false` = informational / weight-0, excluded from the score and the pass/fail gate). The base allows extra fields so subclass keys round-trip. @@ -173,10 +177,22 @@ fields so subclass keys round-trip. - **`classification`** adds `observed_label`, `expected_label` (sentinels like `(none)` / `(other)` allowed). Emitted by `classification_match`, `skill_triggered`. - **`judge`** adds `findings`, `token_usage` (kept distinct from the agent total), and - `transcript_path` (a sibling `judge-N.yaml`). The full `transcript` is **stripped + `transcript_path` (a sibling `judge-N.yaml`, or `post-failure-judge-N.yaml` for + diagnostic records). The full `transcript` is **stripped from `task.json`** β€” read it from the referenced file. Emitted by `llm_judge`, `agent_judge`. +### Post-failure criterion evidence + +When an agent crashes or its turn times out, coder-eval runs only deterministic, +read-only artifact criteria while the sandbox is still live: `file_exists`, +`file_contains`, `file_matches_regex`, `file_check`, `json_check`, +`reference_comparison`, and `classification_match`. Judges, trajectory checks, +`run_command`, and `uipath_eval` are recorded with +`evaluation_status="not_evaluated"`; they are not invoked on this recovery path. +The diagnostic list is additive evidence. An `ERROR` run remains `ERROR`, and its +canonical score remains 0.0. + ### TurnRecord `iteration`, `user_input`, `agent_output`, `commands` (`list[CommandTelemetry]`), diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index d4ef0335..8306c492 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -239,7 +239,7 @@ run_limits: # Structural caps max_turns: 20 # hard cap on agent inner-loop turns per iteration expected_turns: 8 # SOFT efficiency budget (visible turns) β€” never aborts - task_timeout: 600 # wall-clock cap across all iterations, seconds + task_timeout: 300 # wall-clock cap for the full run envelope, seconds turn_timeout: 300 # per-communicate() timeout, seconds # Budget caps @@ -254,8 +254,8 @@ run_limits: |-------|---------|------------|-------------| | `max_turns` | *unset* | `> 0` | Hard cap on agent inner-loop turns per iteration. Unset uses the SDK default. | | `expected_turns` | *unset* | `>= 1` | **Soft** target for cumulative visible turns. Exceeding it warns and badges the report; it never aborts. See [`expected_turns`](#expected_turns-soft-efficiency-budget). | -| `task_timeout` | *unset* | `>= 30` | Max seconds for the whole evaluation loop (all iterations). | -| `turn_timeout` | *unset* | `>= 10` | Max seconds for a single agent `communicate()` call. | +| `task_timeout` | *unset* | `>= 30` | Max seconds for the full run envelope, including agent work, grading, and post-run work. | +| `turn_timeout` | *unset* | `>= 10` | Max seconds for the agent's single `communicate()` iteration. | | `max_input_tokens` | *unset* | `>= 1` | Max cumulative input (prompt) tokens. | | `max_output_tokens` | *unset* | `>= 1` | Max cumulative output (completion) tokens. | | `max_total_tokens` | *unset* | `>= 1` | Max cumulative input + output tokens. Distinct from [`simulation.max_total_tokens`](#simulation-multi-turn-user-dialog) β€” see the note below. | @@ -265,6 +265,11 @@ run_limits: | `stop_early` | *unset* | `false` or unset | Run-level early-stop **kill switch** β€” there is no master arm. Unset: the criteria's own `stop_early:` blocks decide. `false`: force-disarm every block for this run. `true` (the removed master arm) is rejected at resolution. See [`stop_early`](#stop_early-opt-in-early-stop). | | `stop_early_gate_threshold` | `1.0` | `[0.0, 1.0]` (but `> 0.0` is enforced at resolution on an armed task) | Minimum weighted score over the armed subset required for an **early-stopped** run to gate as a pass. See [`stop_early`](#stop_early-opt-in-early-stop). | +If resolved `task_timeout` is larger than `turn_timeout`, `plan` and runtime emit +a non-blocking warning: a larger `task_timeout` cannot extend the agent's single +iteration; the agent budget is `turn_timeout`. The values are not rejected or +changed because `task_timeout` still governs grading and other run-envelope work. + The authoritative source is `src/coder_eval/models/limits.py`. A lint rule (CE030) fails the build if a field defined there goes undocumented in this guide, so the table can't quietly fall behind the model. diff --git a/evalboard/app/runs/[id]/[...task]/__tests__/criteria.test.tsx b/evalboard/app/runs/[id]/[...task]/__tests__/criteria.test.tsx new file mode 100644 index 00000000..874a3c32 --- /dev/null +++ b/evalboard/app/runs/[id]/[...task]/__tests__/criteria.test.tsx @@ -0,0 +1,49 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, test } from "vitest"; +import type { CriterionResult } from "@/lib/runs"; +import { CriteriaSection } from "../_sections"; + +function criterion( + overrides: Partial = {}, +): CriterionResult { + return { + criterionType: "file_exists", + description: "artifact exists", + score: 1, + details: null, + error: null, + evaluationStatus: "evaluated", + passThreshold: 0.9, + gating: true, + ...overrides, + }; +} + +describe("CriteriaSection", () => { + test("renders unavailable post-failure checks without calling them failures", () => { + render( + , + ); + + expect( + screen.getByText("Post-failure artifact evidence (2)"), + ).toBeInTheDocument(); + expect(screen.getByText("NOT EVALUATED")).toBeInTheDocument(); + expect(screen.getByText("no score")).toBeInTheDocument(); + expect( + screen.getByText(/does not affect status, score, or pass\/fail gating/), + ).toBeInTheDocument(); + }); +}); diff --git a/evalboard/app/runs/[id]/[...task]/_sections.tsx b/evalboard/app/runs/[id]/[...task]/_sections.tsx index 45767f7d..12cfea2e 100644 --- a/evalboard/app/runs/[id]/[...task]/_sections.tsx +++ b/evalboard/app/runs/[id]/[...task]/_sections.tsx @@ -105,39 +105,62 @@ export function FlowDebugSection({ flowDebug }: { flowDebug: FlowDebugResult }) ); } -export function CriteriaSection({ criteria }: { criteria: CriterionResult[] }) { +export function CriteriaSection({ + criteria, + title = "Success criteria", + diagnostic = false, +}: { + criteria: CriterionResult[]; + title?: string; + diagnostic?: boolean; +}) { return (

- Success criteria ({criteria.length}) - {criteria.some((c) => !c.gating) && ( + {title} ({criteria.length}) + {!diagnostic && criteria.some((c) => !c.gating) && ( {criteria.filter((c) => !c.gating).length}{" "} informational )}

+ {diagnostic && ( +

+ Diagnostic evidence collected after the agent failed. It + does not affect status, score, or pass/fail gating. +

+ )}
{criteria.map((c, i) => { // Compare against the criterion's own threshold, not === 1: // fractional criteria pass below 1.0 (default threshold 0.9). const passed = (c.score ?? 0) >= c.passThreshold; + const evaluated = c.evaluationStatus === "evaluated"; return ( - + {evaluated ? ( + + ) : ( + + NOT EVALUATED + + )} {c.description ?? c.criterionType ?? `criterion ${i + 1}`} - score {c.score ?? "β€”"} + {evaluated + ? `score ${c.score ?? "β€”"}` + : "no score"}
} diff --git a/evalboard/app/runs/[id]/[...task]/page.tsx b/evalboard/app/runs/[id]/[...task]/page.tsx index 139f27ad..e95bd7d8 100644 --- a/evalboard/app/runs/[id]/[...task]/page.tsx +++ b/evalboard/app/runs/[id]/[...task]/page.tsx @@ -306,6 +306,13 @@ export default async function TaskPage({ {flowDebug && } + {task.postFailureCriteria.length > 0 && ( + + )} {conversation.length > 0 && ( )} diff --git a/evalboard/lib/__tests__/providerCalls.test.ts b/evalboard/lib/__tests__/providerCalls.test.ts index 35b37708..c858a27c 100644 --- a/evalboard/lib/__tests__/providerCalls.test.ts +++ b/evalboard/lib/__tests__/providerCalls.test.ts @@ -88,6 +88,14 @@ beforeEach(async () => { `${RUN}/default/${TASK}/00/task.json`, JSON.stringify({ final_status: "success", + post_failure_criteria_results: [ + { + criterion_type: "file_exists", + description: "artifact exists", + score: 1, + evaluation_status: "evaluated", + }, + ], iterations: [ { model_used: "deepseek/deepseek-v4-pro", @@ -139,5 +147,9 @@ describe("readTaskDetail: providerCalls", () => { }); expect(turn.calls[1].callId).toBe("call-2"); expect(turn.calls[1].costUsd).toBe(0.0034); + expect(detail!.postFailureCriteria).toHaveLength(1); + expect(detail!.postFailureCriteria[0].evaluationStatus).toBe( + "evaluated", + ); }); }); diff --git a/evalboard/lib/__tests__/runs.test.ts b/evalboard/lib/__tests__/runs.test.ts index 91c8341a..c0031d37 100644 --- a/evalboard/lib/__tests__/runs.test.ts +++ b/evalboard/lib/__tests__/runs.test.ts @@ -20,12 +20,46 @@ import { findMatureSourceRuns, isExcludedArtifact, type MessageEvent, + parseCriterionResults, sortArtifacts, toTaskRow, visibleTurnsFromRaw, walkArtifacts, } from "../runs"; +describe("parseCriterionResults", () => { + test("preserves evaluated versus not-evaluated evidence", () => { + const results = parseCriterionResults([ + { + criterion_type: "file_exists", + description: "artifact exists", + score: 1, + evaluation_status: "evaluated", + }, + { + criterion_type: "run_command", + description: "validator runs", + score: 0, + evaluation_status: "not_evaluated", + }, + ]); + + expect(results.map((result) => result.evaluationStatus)).toEqual([ + "evaluated", + "not_evaluated", + ]); + }); + + test("legacy results default to evaluated", () => { + const [result] = parseCriterionResults([ + { criterion_type: "file_exists", score: 0 }, + ]); + expect(result.evaluationStatus).toBe("evaluated"); + expect(result.passThreshold).toBe(0.9); + expect(result.gating).toBe(true); + }); +}); + describe("toTaskRow", () => { test("propagates total_turns and expected_turns", () => { const row = toTaskRow({ diff --git a/evalboard/lib/runs.ts b/evalboard/lib/runs.ts index ea977d28..7aa72f63 100644 --- a/evalboard/lib/runs.ts +++ b/evalboard/lib/runs.ts @@ -118,6 +118,7 @@ export interface CriterionResult { score: number | null; details: string | null; error: string | null; + evaluationStatus: "evaluated" | "not_evaluated"; // Mirrors the Python CriterionResult fields. `gating: false` (weight: 0) means // the criterion is informational β€” measured, but excluded from the score and // the pass/fail gate, so it must not render as PASS/FAIL. Both default the way @@ -126,6 +127,32 @@ export interface CriterionResult { gating: boolean; } +interface RawCriterionResult { + criterion_type?: string; + description?: string; + score?: number; + details?: string; + error?: string | null; + evaluation_status?: "evaluated" | "not_evaluated"; + pass_threshold?: number; + gating?: boolean; +} + +export function parseCriterionResults( + criteria: RawCriterionResult[] | undefined, +): CriterionResult[] { + return (criteria ?? []).map((criterion) => ({ + criterionType: criterion.criterion_type ?? null, + description: criterion.description ?? null, + score: criterion.score ?? null, + details: criterion.details ?? null, + error: criterion.error ?? null, + evaluationStatus: criterion.evaluation_status ?? "evaluated", + passThreshold: criterion.pass_threshold ?? 0.9, + gating: criterion.gating ?? true, + })); +} + export interface ElementExecution { elementId: string; elementType: string | null; @@ -267,6 +294,7 @@ export interface TaskDetail extends TaskResultSummary { errorMessage: string | null; taskDescription: string | null; criteria: CriterionResult[]; + postFailureCriteria: CriterionResult[]; artifacts: ArtifactRef[]; flowDebug: FlowDebugResult | null; toolCalls: ToolCall[]; @@ -2003,30 +2031,16 @@ export async function readTaskDetail( initial_prompt?: string; }; }; - success_criteria_results?: Array<{ - criterion_type?: string; - description?: string; - score?: number; - details?: string; - error?: string | null; - pass_threshold?: number; - gating?: boolean; - }>; + success_criteria_results?: RawCriterionResult[]; + post_failure_criteria_results?: RawCriterionResult[]; iterations?: TurnEntry[]; environment_info?: RawRunJson["environment_info"]; }>(path.join(contentDir, "task.json")); - const criteria: CriterionResult[] = ( - task?.success_criteria_results ?? [] - ).map((c) => ({ - criterionType: c.criterion_type ?? null, - description: c.description ?? null, - score: c.score ?? null, - details: c.details ?? null, - error: c.error ?? null, - passThreshold: c.pass_threshold ?? 0.9, - gating: c.gating ?? true, - })); + const criteria = parseCriterionResults(task?.success_criteria_results); + const postFailureCriteria = parseCriterionResults( + task?.post_failure_criteria_results, + ); const artifactRoot = path.join(contentDir, "artifacts"); // relPath is stored relative to the run root so the /api/file route can @@ -2093,6 +2107,7 @@ export async function readTaskDetail( errorMessage: task?.error_message ?? null, taskDescription, criteria, + postFailureCriteria, artifacts, flowDebug, toolCalls, diff --git a/src/coder_eval/evaluation/judge_persistence.py b/src/coder_eval/evaluation/judge_persistence.py index 10aefcf0..aad1fbf7 100644 --- a/src/coder_eval/evaluation/judge_persistence.py +++ b/src/coder_eval/evaluation/judge_persistence.py @@ -57,6 +57,12 @@ logger = logging.getLogger(__name__) +TASK_JSON_TRANSCRIPT_EXCLUDE = { + "success_criteria_results": {"__all__": {"transcript"}}, + "post_failure_criteria_results": {"__all__": {"transcript"}}, +} + + # Windows reserved device basenames. The Win32 API maps these to character # devices regardless of the directory they sit in β€” opening ``CON`` or ``NUL.yaml`` # inside ``task_dir`` resolves to the console or the null device, not a file. @@ -207,7 +213,7 @@ def load_judge_transcripts(result: EvaluationResult, task_dir: Path) -> int: # regular char) but resolves to a nested file on Windows. Rejecting under # either interpretation enforces the basename-only policy regardless of # which platform the task.json travels to next. - if PurePosixPath(path).name != path or PureWindowsPath(path).name != path: + if path in {".", ".."} or PurePosixPath(path).name != path or PureWindowsPath(path).name != path: logger.warning("Refusing to load judge transcript with non-basename path: %s", path) continue # Reject Windows reserved device basenames. On Windows, ``CON.yaml`` / diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index aa584168..9256c01a 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -130,6 +130,9 @@ class BaseSuccessCriterion(BaseModel, ABC): requires_agent: ClassVar[bool] = False """True if this criterion requires agent turn records to evaluate correctly.""" + supports_post_failure_evaluation: ClassVar[bool] = False + """True for deterministic, read-only artifact checks safe to run after agent failure.""" + @property def is_stop_armed(self) -> bool: """True when this criterion participates in the run's early-stop armed set. @@ -321,6 +324,7 @@ class FileExistsCriterion(BaseSuccessCriterion): Pure data model - checking logic in SuccessChecker._check_file_exists() """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["file_exists"] = "file_exists" path: str = Field( description="Path to the file that must exist; a glob pattern passes when it matches at least one file" @@ -333,6 +337,7 @@ class FileContainsCriterion(BaseSuccessCriterion): Pure data model - checking logic in SuccessChecker._check_file_contains() """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["file_contains"] = "file_contains" path: str = Field(description="Path to the file to check; may be a glob matching exactly one file") includes: list[str] = Field(description="List of strings that must be present in the file") @@ -406,6 +411,7 @@ class FileMatchesRegexCriterion(BaseSuccessCriterion): Pure data model - checking logic in SuccessChecker._check_file_matches_regex() """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["file_matches_regex"] = "file_matches_regex" path: str = Field(description="Path to the file to check; may be a glob matching exactly one file") pattern: str = Field(description="Regex pattern that must match somewhere in the file") @@ -842,6 +848,7 @@ class JsonCheckCriterion(BaseSuccessCriterion): Only active categories (schema, assertions) contribute to the average. """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["json_check"] = "json_check" path: str = Field( description="Path to the JSON file (relative to sandbox root); may be a glob matching exactly one file" @@ -879,6 +886,7 @@ class FileCheckCriterion(BaseSuccessCriterion): description: "main.py exists with correct imports and structure" """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["file_check"] = "file_check" path: str = Field( description="Path to the file to check (relative to sandbox root); may be a glob matching exactly one file" @@ -910,6 +918,7 @@ class ReferenceComparisonCriterion(BaseSuccessCriterion): """ requires_agent: ClassVar[bool] = True + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["reference_comparison"] = "reference_comparison" @@ -1107,6 +1116,7 @@ class ClassificationMatchCriterion(BaseSuccessCriterion): description: "Sentiment label matches ground truth" """ + supports_post_failure_evaluation: ClassVar[bool] = True type: Literal["classification_match"] = "classification_match" path: str = Field( description=( diff --git a/src/coder_eval/models/results.py b/src/coder_eval/models/results.py index 212ab667..8834710a 100644 --- a/src/coder_eval/models/results.py +++ b/src/coder_eval/models/results.py @@ -225,8 +225,9 @@ class JudgeCriterionResult(CriterionResult): transcript_path: str | None = Field( default=None, description=( - "Filename of the sibling JSON file holding this result's full transcript " - "(e.g. ``judge-0.yaml``), relative to the directory containing ``task.json``. " + "Filename of the sibling YAML file holding this result's full transcript " + "(``judge-N.yaml`` for canonical results or ``post-failure-judge-N.yaml`` " + "for diagnostic results), relative to the directory containing ``task.json``. " "Set by ``spill_judge_transcripts`` after the run; reloaded by " "``load_judge_transcripts`` for re-rendering. None when no transcript was captured." ), @@ -976,7 +977,11 @@ def judge_cost_usd(result: EvaluationResult) -> float | None: ``None`` when no criterion reported cost. """ criterion_results = result.success_criteria_results + result.post_failure_criteria_results - usages = [u for cr in criterion_results if (u := getattr(cr, "token_usage", None)) is not None] + usages = [ + cr.token_usage + for cr in criterion_results + if isinstance(cr, JudgeCriterionResult) and cr.token_usage is not None + ] return sum_costs(*(u.total_cost_usd for u in usages)) diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 28e63176..7e2a4583 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -22,8 +22,6 @@ from .errors import ( AgentCrashError, BudgetExceededError, - CheckerMisuseError, - JudgeInfrastructureError, TaskTimeoutError, TurnTimeoutError, ) @@ -675,11 +673,6 @@ async def _run_evaluation_with_failure_evidence( elapsed_seconds=time.time() - start_time, ) from None raise - except TaskTimeoutError: - self._record_post_failure_not_evaluated( - "the task_timeout budget was exhausted before post-failure grading could run" - ) - raise except (AgentCrashError, TurnTimeoutError, BudgetExceededError) as terminal_error: if ( isinstance(terminal_error, BudgetExceededError) @@ -694,18 +687,15 @@ async def _run_evaluation_with_failure_evidence( self._record_post_failure_not_evaluated( "the task_timeout budget expired during post-failure grading" ) - raise TaskTimeoutError( - task_timeout or 0, - task_id=self.task.task_id, - elapsed_seconds=time.time() - start_time, - ) from None - raise - except (JudgeInfrastructureError, CheckerMisuseError): - raise + logger.warning( + "[%s] Task timeout stopped post-failure grading; preserving %s", + self.task.task_id, + type(terminal_error).__name__, + ) + else: + raise except Exception as recovery_error: - self._record_post_failure_not_evaluated( - f"post-failure grading could not complete ({type(recovery_error).__name__})" - ) + self._record_post_failure_not_evaluated(self._post_failure_exception_reason(recovery_error)) logger.warning( "[%s] Post-failure criteria evaluation failed; preserving the original terminal error", self.task.task_id, @@ -713,6 +703,14 @@ async def _run_evaluation_with_failure_evidence( ) raise + @staticmethod + def _post_failure_exception_reason(error: Exception) -> str: + message = " ".join(str(error).split()) + if len(message) > 200: + message = message[:199] + "…" + suffix = f": {message}" if message else "" + return f"post-failure grading could not complete ({type(error).__name__}{suffix})" + @staticmethod def _not_evaluated_result(criterion: SuccessCriterion, reason: str) -> CriterionResult: return CriterionResult( @@ -736,9 +734,9 @@ def _record_post_failure_not_evaluated(self, reason: str) -> None: async def _evaluate_post_failure_criteria(self) -> None: """Evaluate diagnostic criteria before the live sandbox is torn down. - Results stay outside the canonical scored list. Agent-dependent checks - require at least one preserved turn; artifact-only checks can still run - when the agent failed before producing trajectory evidence. + Results stay outside the canonical scored list. Only criteria that + declare themselves deterministic and read-only run on this path. This + excludes judges and checks that execute sandbox commands. """ if self.result is None: return @@ -749,7 +747,7 @@ async def _evaluate_post_failure_criteria(self) -> None: runnable: list[SuccessCriterion] = [] unavailable_positions: set[int] = set() for position, criterion in enumerate(self.task.success_criteria): - if criterion.requires_agent and not self.result.iterations: + if not criterion.supports_post_failure_evaluation: unavailable_positions.add(position) else: runnable.append(criterion) @@ -780,7 +778,7 @@ async def _evaluate_post_failure_criteria(self) -> None: recovered.append( self._not_evaluated_result( criterion, - "no turn record survived for this agent-dependent criterion", + "the criterion is not a deterministic, read-only artifact check", ) ) else: @@ -936,7 +934,7 @@ def _finalize_result(self, start_time: float) -> None: # we dump task.json, so transcript_path is set on each judge result. # The inline `transcript` field stays in memory β€” HTML rendering below # uses it directly. We strip it from the JSON dump via `exclude=...`. - from .evaluation.judge_persistence import spill_judge_transcripts + from .evaluation.judge_persistence import TASK_JSON_TRANSCRIPT_EXCLUDE, spill_judge_transcripts spill_judge_transcripts(self.result, self.report_path.parent) @@ -954,10 +952,7 @@ def _finalize_result(self, start_time: float) -> None: # next to task.json, referenced by transcript_path. Excluding # `transcript` here avoids ~20-100 KB of bloat per judge result # in the row record without losing any data. - exclude={ - "success_criteria_results": {"__all__": {"transcript"}}, - "post_failure_criteria_results": {"__all__": {"transcript"}}, - }, + exclude=TASK_JSON_TRANSCRIPT_EXCLUDE, ), encoding="utf-8", ) diff --git a/src/coder_eval/reports_html.py b/src/coder_eval/reports_html.py index b582856f..def6591f 100644 --- a/src/coder_eval/reports_html.py +++ b/src/coder_eval/reports_html.py @@ -416,7 +416,7 @@ def _render_criteria_details(cr: CriterionResult) -> str: ) -def _render_judge_section(criteria: list[CriterionResult]) -> str: +def _render_judge_section(criteria: list[CriterionResult], heading: str = "Judge Verdicts") -> str: """Render the dedicated 'Judge Verdicts' section containing per-judge cards. Each judge result that carries verdict evidence (findings, transcript) gets @@ -443,7 +443,7 @@ def _render_judge_section(criteria: list[CriterionResult]) -> str: if not cards: return "" - return f'

Judge Verdicts ({len(cards)})

' + "".join(cards) + "
" + return f'

{_esc(heading)} ({len(cards)})

' + "".join(cards) + "
" def _extract_rationale(details: str | None) -> str: @@ -566,8 +566,16 @@ def _render_judge_transcript(transcript: Any) -> str: ) -def _render_criteria(results: list[CriterionResult], early_stop: EarlyStopInfo | None = None) -> str: +def _render_criteria( + results: list[CriterionResult], + early_stop: EarlyStopInfo | None = None, + *, + heading: str = "Success Criteria", + diagnostic: bool = False, +) -> str: if not results: + if diagnostic: + return "" return """

Success Criteria

@@ -579,31 +587,52 @@ def _render_criteria(results: list[CriterionResult], early_stop: EarlyStopInfo | rows: list[str] = [] for cr in results: advisory = "" - if not cr.gating: + evaluated = cr.evaluation_status == "evaluated" + if not evaluated: + advisory = '
no verdict β€” the check did not run
' + elif not cr.gating and not diagnostic: # weight: 0 β€” measured but excluded from the gate. Saying so here keeps # the row from reading as a failure that the header/status contradicts. advisory = '
informational β€” not gated (weight: 0)
' - elif early_stop is not None and f"{cr.criterion_type}: {cr.description}" not in armed: + elif not diagnostic and early_stop is not None and f"{cr.criterion_type}: {cr.description}" not in armed: advisory = '
advisory β€” not gated (run stopped early)
' + score = _score_pill(cr.score) if evaluated else 'NOT EVALUATED' rows.append( f""" {_esc(cr.criterion_type)} {_esc(cr.description)}{advisory} - {_score_pill(cr.score)} + {score} {_render_criteria_details(cr)} """ ) - # Count over gating criteria only, so this header agrees with final_status - # and the CLI exit code (both of which ignore weight: 0 criteria). - gating_results = [r for r in results if r.gating] - passed = sum(1 for r in gating_results if r.score >= r.pass_threshold) - total = len(gating_results) - informational = len(results) - total - info_note = f' + {informational} informational' if informational else "" + evaluated_results = [r for r in results if r.evaluation_status == "evaluated"] + not_evaluated = len(results) - len(evaluated_results) + if diagnostic: + passed = sum(1 for r in evaluated_results if r.score >= r.pass_threshold) + summary = f"{passed}/{len(evaluated_results)} evaluated checks passed" + if not_evaluated: + summary += f" Β· {not_evaluated} not evaluated" + note = ( + '

Diagnostic evidence collected after the agent failed. ' + "It does not change the terminal status, canonical score, or pass/fail gate.

" + ) + else: + # Count over evaluated gating criteria only, so this header agrees with + # final_status and the CLI exit code. + gating_results = [r for r in evaluated_results if r.gating] + passed = sum(1 for r in gating_results if r.score >= r.pass_threshold) + summary = f"{passed}/{len(gating_results)} passed" + informational = len(evaluated_results) - len(gating_results) + if informational: + summary += f" + {informational} informational" + if not_evaluated: + summary += f" + {not_evaluated} not evaluated" + note = "" return f""" -

Success Criteria ({passed}/{total} passed{info_note})

+

{_esc(heading)} ({_esc(summary)})

+{note}
@@ -1495,6 +1524,15 @@ def generate_task_html(result: EvaluationResult) -> str: + _render_simulation(result) + _render_criteria(result.success_criteria_results or [], result.early_stop) + _render_judge_section(result.success_criteria_results or []) + + _render_criteria( + result.post_failure_criteria_results or [], + heading="Post-failure Artifact Evidence", + diagnostic=True, + ) + + _render_judge_section( + result.post_failure_criteria_results or [], + heading="Post-failure Judge Evidence", + ) + _render_error_details(result) + f"

Conversation Trace ({trace_count})

" + turns_html diff --git a/src/coder_eval/reports_junit.py b/src/coder_eval/reports_junit.py index 569f7217..133f9187 100644 --- a/src/coder_eval/reports_junit.py +++ b/src/coder_eval/reports_junit.py @@ -189,6 +189,49 @@ def _load_task_json(run_dir: Path, row: dict[str, Any], variant: str) -> dict[st return None +def _criterion_lines(criteria: list[Any], *, diagnostic: bool = False) -> list[str]: + """Render criterion rows without treating unavailable evidence as a failed check.""" + lines: list[str] = [] + for crit in criteria: + if not isinstance(crit, dict): + continue + ctype = str(crit.get("criterion_type", "unknown")) + description = str(crit.get("description", "")) + evaluation_status = crit.get("evaluation_status", "evaluated") + detail = crit.get("details") or crit.get("error") + if evaluation_status == "not_evaluated": + lines.append(f"[NOT EVALUATED] {ctype}: {description}") + if detail: + lines.append(str(detail)) + continue + + score = crit.get("score") + threshold = crit.get("pass_threshold") + score_str = f"{score:.2f}" if isinstance(score, int | float) else str(score) + passed = isinstance(score, int | float) and isinstance(threshold, int | float) and score >= threshold + if diagnostic: + label = "DIAGNOSTIC PASS" if passed else "DIAGNOSTIC FAIL" + lines.append(f"[{label}] {ctype}: score {score_str} β€” {description}") + if detail: + lines.append(str(detail)) + continue + + # Only an explicit JSON ``false`` marks an informational criterion; a + # missing key or a schema-skewed value fails safe to gating. + informational = crit.get("gating", True) is False + if informational: + lines.append(f"[INFO] {ctype}: score {score_str} β€” {description}") + continue + if passed: + lines.append(f"[PASS] {ctype}: {description}") + continue + threshold_str = f"{threshold:.2f}" if isinstance(threshold, int | float) else str(threshold) + lines.append(f"[FAIL] {ctype}: score {score_str} < threshold {threshold_str} β€” {description}") + if detail: + lines.append(str(detail)) + return lines + + def _criteria_body(row: dict[str, Any], run_dir: Path, variant: str) -> str: """Build the failure/error body for a non-succeeded row. @@ -199,43 +242,25 @@ def _criteria_body(row: dict[str, Any], run_dir: Path, variant: str) -> str: status = _status_of(row) data = _load_task_json(run_dir, row, variant) criteria = data.get("success_criteria_results") if isinstance(data, dict) else None + post_failure = data.get("post_failure_criteria_results") if isinstance(data, dict) else None lines: list[str] = [] if isinstance(criteria, list) and criteria: - for crit in criteria: - if not isinstance(crit, dict): - continue - ctype = str(crit.get("criterion_type", "unknown")) - description = str(crit.get("description", "")) - score = crit.get("score") - threshold = crit.get("pass_threshold") - # Only an explicit JSON ``false`` marks an informational criterion; a - # missing key or a schema-skewed value (null / non-bool) fails safe - # to gating β€” matching CriterionResult.gating's default and this - # module's isinstance-guarded, degrade-don't-crash reads of untyped - # rows. Informational criteria are excluded from the score/gate, so - # they are labelled [INFO] regardless of pass/fail and never rendered - # as the failure cause (mirrors reports.py `_compute_suite_rollup`'s - # `if not cr.gating: continue`). - informational = crit.get("gating", True) is False - score_str = f"{score:.2f}" if isinstance(score, int | float) else str(score) - if informational: - lines.append(f"[INFO] {ctype}: score {score_str} β€” {description}") - continue - passed = isinstance(score, int | float) and isinstance(threshold, int | float) and score >= threshold - if passed: - lines.append(f"[PASS] {ctype}: {description}") - continue - thr_str = f"{threshold:.2f}" if isinstance(threshold, int | float) else str(threshold) - lines.append(f"[FAIL] {ctype}: score {score_str} < threshold {thr_str} β€” {description}") - detail = crit.get("details") or crit.get("error") - if detail: - lines.append(str(detail)) + lines.extend(_criterion_lines(criteria)) else: weighted = row.get("weighted_score") weighted_str = f"{weighted:.2f}" if isinstance(weighted, int | float) else str(weighted) lines.append(f"status={status} weighted_score={weighted_str}") + if isinstance(post_failure, list) and post_failure: + lines.extend( + [ + "", + "Post-failure artifact evidence (diagnostic only; does not affect status or weighted score):", + *_criterion_lines(post_failure, diagnostic=True), + ] + ) + return _xml_safe(truncate("\n".join(lines), _BODY_LIMIT)) diff --git a/tests/test_judge_persistence.py b/tests/test_judge_persistence.py index 7eb183cd..e5bf0da5 100644 --- a/tests/test_judge_persistence.py +++ b/tests/test_judge_persistence.py @@ -14,6 +14,7 @@ import pytest from coder_eval.evaluation.judge_persistence import ( + TASK_JSON_TRANSCRIPT_EXCLUDE, load_judge_transcripts, spill_judge_transcripts, ) @@ -134,28 +135,28 @@ def test_spill_preserves_index_for_multiple_judges(tmp_path: Path) -> None: def test_post_failure_judge_uses_distinct_sibling_and_round_trips(tmp_path: Path) -> None: - judge = _make_judge_result(transcript=_make_transcript()) - result = _make_evaluation_result(criteria=[]) + canonical = _make_judge_result(score=0.6, transcript=_make_transcript()) + diagnostic = _make_judge_result(score=0.9, transcript=_make_transcript()) + result = _make_evaluation_result(criteria=[canonical]) result.final_status = FinalStatus.ERROR - result.post_failure_criteria_results = [judge] + result.post_failure_criteria_results = [diagnostic] - assert spill_judge_transcripts(result, tmp_path) == 1 - assert judge.transcript_path == "post-failure-judge-0.yaml" + assert spill_judge_transcripts(result, tmp_path) == 2 + assert canonical.transcript_path == "judge-0.yaml" + assert diagnostic.transcript_path == "post-failure-judge-0.yaml" - raw = result.model_dump_json( - exclude={ - "success_criteria_results": {"__all__": {"transcript"}}, - "post_failure_criteria_results": {"__all__": {"transcript"}}, - } - ) + raw = result.model_dump_json(exclude=TASK_JSON_TRANSCRIPT_EXCLUDE) assert "raw_verdict" not in raw reloaded = EvaluationResult.model_validate_json(raw) - assert load_judge_transcripts(reloaded, tmp_path) == 1 - recovered = reloaded.post_failure_criteria_results[0] - assert isinstance(recovered, JudgeCriterionResult) - assert recovered.transcript is not None - assert recovered.transcript.raw_verdict == '{"score":0.75,"rationale":"ok"}' + assert load_judge_transcripts(reloaded, tmp_path) == 2 + for recovered in ( + reloaded.success_criteria_results[0], + reloaded.post_failure_criteria_results[0], + ): + assert isinstance(recovered, JudgeCriterionResult) + assert recovered.transcript is not None + assert recovered.transcript.raw_verdict == '{"score":0.75,"rationale":"ok"}' def test_spill_skips_non_judge_results(tmp_path: Path) -> None: @@ -342,6 +343,16 @@ def test_load_rejects_subdir_path(tmp_path: Path) -> None: assert n == 0 +@pytest.mark.parametrize("name", [".", ".."]) +def test_load_rejects_dot_path_components(tmp_path: Path, name: str) -> None: + judge = _make_judge_result(transcript=None) + judge.transcript_path = name + result = _make_evaluation_result(criteria=[judge]) + + assert load_judge_transcripts(result, tmp_path) == 0 + assert judge.transcript is None + + @pytest.mark.parametrize( "name", [ diff --git a/tests/test_reports_html.py b/tests/test_reports_html.py index bdfba2cc..77e5886a 100644 --- a/tests/test_reports_html.py +++ b/tests/test_reports_html.py @@ -269,6 +269,36 @@ def test_task_html_error_case_with_empty_turns(): assert "Logs" not in html +def test_task_html_renders_post_failure_evidence_as_diagnostic() -> None: + result = _make_result( + final_status=FinalStatus.ERROR, + error_message="Agent turn timed out", + ) + result.weighted_score = 0.0 + result.post_failure_criteria_results = [ + CriterionResult( + criterion_type="file_exists", + description="artifact exists", + score=1.0, + ), + CriterionResult( + criterion_type="run_command", + description="validator runs", + score=0.0, + evaluation_status="not_evaluated", + details="Not evaluated after terminal agent failure: unsafe recovery check.", + ), + ] + + html = HTMLReportGenerator.generate_task_html(result) + + assert "Post-failure Artifact Evidence" in html + assert "1/1 evaluated checks passed Β· 1 not evaluated" in html + assert "NOT EVALUATED" in html + assert "does not change the terminal status, canonical score, or pass/fail gate" in html + assert "ERROR" in html + + def test_task_html_error_renders_error_log_tail(): """``error_log_tail`` on the result drives the Logs disclosure.""" result = _make_result( diff --git a/tests/test_reports_junit.py b/tests/test_reports_junit.py index 54d44b78..0785f1cb 100644 --- a/tests/test_reports_junit.py +++ b/tests/test_reports_junit.py @@ -32,11 +32,15 @@ def _write_task_json( task_id: str, replicate_index: int, criteria: list[dict[str, Any]], + post_failure: list[dict[str, Any]] | None = None, ) -> None: """Write a minimal task.json (plain dict) at the run-layout location.""" task_dir = run_dir / variant / task_id / f"{replicate_index:02d}" task_dir.mkdir(parents=True, exist_ok=True) - (task_dir / "task.json").write_text(json.dumps({"success_criteria_results": criteria}), encoding="utf-8") + payload: dict[str, Any] = {"success_criteria_results": criteria} + if post_failure is not None: + payload["post_failure_criteria_results"] = post_failure + (task_dir / "task.json").write_text(json.dumps(payload), encoding="utf-8") def _row( @@ -853,6 +857,43 @@ def test_passing_informational_criterion_is_info_not_pass(write_run_json: Callab assert "[PASS]" not in body +def test_error_body_labels_post_failure_evidence_as_diagnostic( + write_run_json: Callable[..., Path], tmp_path: Path +) -> None: + run_dir = tmp_path / "run" + rows = [_row("t_error", "ERROR", variant_id="v1", replicate_index=0, weighted_score=0.0)] + write_run_json(run_dir, rows) + _write_task_json( + run_dir, + "v1", + "t_error", + 0, + [], + post_failure=[ + { + "criterion_type": "file_exists", + "description": "artifact exists", + "score": 1.0, + "pass_threshold": 0.9, + "evaluation_status": "evaluated", + }, + { + "criterion_type": "run_command", + "description": "validator runs", + "score": 0.0, + "pass_threshold": 0.9, + "evaluation_status": "not_evaluated", + }, + ], + ) + + body = _find_testsuite(fromstring(generate_junit_xml(run_dir)), "v1").find("testcase").find("error").text or "" + assert "status=ERROR weighted_score=0.00" in body + assert "diagnostic only; does not affect status or weighted score" in body + assert "[DIAGNOSTIC PASS] file_exists" in body + assert "[NOT EVALUATED] run_command" in body + + def test_windows_drive_task_id_degrades_to_status_body(write_run_json: Callable[..., Path], tmp_path: Path) -> None: """A Windows drive-qualified task_id (``C:/x``) passes the nested-relpath shape on POSIX but is rejected by the drive guard, so it degrades to a status body diff --git a/tests/test_run_limits_orchestrator.py b/tests/test_run_limits_orchestrator.py index 2245d25f..adfe7356 100644 --- a/tests/test_run_limits_orchestrator.py +++ b/tests/test_run_limits_orchestrator.py @@ -252,6 +252,30 @@ async def test_input_budget_trip_records_criteria(self, tmp_path): # Criteria still ran before budget check (single-shot order). assert len(result.success_criteria_results) == 1 + async def test_complete_canonical_results_skip_post_failure_regrade(self, tmp_path): + task = _make_task(run_limits=RunLimits(max_input_tokens=10)) + run_dir = tmp_path / "run" / "complete_budget_result" + run_dir.mkdir(parents=True) + orch = Orchestrator(task=task, run_dir=run_dir, variant_id="v") + orch._setup = AsyncMock() # type: ignore[method-assign] + orch._cleanup = AsyncMock() # type: ignore[method-assign] + orch._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] + err = BudgetExceededError("input_tokens", actual=100, limit=10, task_id=task.task_id, iteration=1) + + async def loop() -> bool: + assert orch.result is not None + orch.result.success_criteria_results = [ + CriterionResult(criterion_type="file_exists", description="x", score=1.0) + ] + raise err + + orch._evaluation_loop = loop # type: ignore[method-assign] + result = await orch.run() + + assert result.final_status == FinalStatus.TOKEN_BUDGET_EXCEEDED + assert len(result.success_criteria_results) == 1 + assert result.post_failure_criteria_results == [] + @pytest.mark.parametrize( "budget_name,expected_status,expected_component", [ diff --git a/tests/test_timeout_orchestrator.py b/tests/test_timeout_orchestrator.py index de36073f..593c675e 100644 --- a/tests/test_timeout_orchestrator.py +++ b/tests/test_timeout_orchestrator.py @@ -7,8 +7,9 @@ import pytest -from coder_eval.errors import JudgeInfrastructureError +from coder_eval.errors import AgentCrashError, CheckerMisuseError, JudgeInfrastructureError from coder_eval.errors.timeout import TaskTimeoutError, TurnTimeoutError +from coder_eval.evaluation.checker import SuccessChecker from coder_eval.models import ( AgentKind, ClaudeCodeAgentConfig, @@ -16,12 +17,15 @@ CriterionResult, EvaluationResult, FileExistsCriterion, + LLMJudgeCriterion, + RunCommandCriterion, SandboxConfig, TaskDefinition, TokenUsage, TurnRecord, ) from coder_eval.orchestrator import Orchestrator +from coder_eval.sandbox import Sandbox def _make_task(*, turn_timeout: float | None = None, task_timeout: float | None = None): @@ -335,44 +339,60 @@ async def turn_out_communicate(_prompt, **kwargs): await orchestrator._evaluation_loop() +@pytest.mark.parametrize( + "terminal_error", + [ + pytest.param( + TurnTimeoutError(1200, task_id="timeout_test", iteration=1), + id="turn-timeout", + ), + pytest.param(AgentCrashError("agent subprocess crashed"), id="agent-crash"), + ], +) @pytest.mark.asyncio -async def test_turn_timeout_records_post_failure_evidence_without_rescoring(tmp_path) -> None: - """A terminal turn timeout preserves artifact truth without changing the ERROR score.""" +async def test_terminal_agent_error_records_safe_artifact_evidence_without_rescoring( + tmp_path, terminal_error: Exception +) -> None: + """Agent failures preserve artifact truth before the live sandbox is removed.""" task = _make_task(turn_timeout=1200, task_timeout=1500) task.success_criteria = [ - FileExistsCriterion(type="file_exists", path="artifact.txt", description="artifact exists"), CommandExecutedCriterion( type="command_executed", tool_name="Bash", description="agent ran validator", ), + FileExistsCriterion(type="file_exists", path="artifact.txt", description="artifact exists"), + RunCommandCriterion( + type="run_command", + command="touch should-not-run", + description="sandbox command", + ), + LLMJudgeCriterion( + type="llm_judge", + prompt="Grade the artifact.", + description="paid judge", + ), ] run_dir = tmp_path / "run" / "post_failure_evidence" run_dir.mkdir(parents=True) orchestrator = Orchestrator(task=task, run_dir=run_dir, variant_id="test-variant") orchestrator._setup = AsyncMock() # type: ignore[method-assign] - orchestrator._cleanup = AsyncMock() # type: ignore[method-assign] orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] - orchestrator._evaluation_loop = AsyncMock( # type: ignore[method-assign] - side_effect=TurnTimeoutError(1200, task_id=task.task_id, iteration=1) - ) + orchestrator._evaluation_loop = AsyncMock(side_effect=terminal_error) # type: ignore[method-assign] - mock_sandbox = MagicMock() - mock_sandbox.sandbox_dir = tmp_path / "sandbox" - mock_sandbox.sandbox_dir.mkdir() - orchestrator.sandbox = mock_sandbox + sandbox = Sandbox(SandboxConfig(driver="tempdir"), task_id=task.task_id) + sandbox_dir = sandbox.setup() + (sandbox_dir / "artifact.txt").write_text("finished", encoding="utf-8") + orchestrator.sandbox = sandbox - mock_checker = MagicMock() - mock_checker.check_all_async = AsyncMock( - return_value=[ - CriterionResult( - criterion_type="file_exists", - description="artifact exists", - score=1.0, - ) - ] - ) - orchestrator.success_checker = mock_checker + checker = SuccessChecker(sandbox) + checker.check_all_async = AsyncMock(wraps=checker.check_all_async) # type: ignore[method-assign] + orchestrator.success_checker = checker + + async def cleanup() -> None: + sandbox.cleanup() + + orchestrator._cleanup = cleanup # type: ignore[method-assign] mock_agent = MagicMock() mock_agent.kill_sync = MagicMock() @@ -383,30 +403,45 @@ async def test_turn_timeout_records_post_failure_evidence_without_rescoring(tmp_ result = await orchestrator.run() assert result.final_status == "ERROR" + assert result.error_message == str(terminal_error) assert result.weighted_score == 0.0 assert result.success_criteria_results == [] - assert len(result.post_failure_criteria_results) == 2 - artifact, agent_dependent = result.post_failure_criteria_results + assert len(result.post_failure_criteria_results) == 4 + agent_dependent, artifact, command, judge = result.post_failure_criteria_results assert artifact.score == 1.0 assert artifact.evaluation_status == "evaluated" - assert agent_dependent.score == 0.0 - assert agent_dependent.evaluation_status == "not_evaluated" - assert "no turn record survived" in (agent_dependent.details or "") + for unavailable in (agent_dependent, command, judge): + assert unavailable.score == 0.0 + assert unavailable.evaluation_status == "not_evaluated" + assert "not a deterministic, read-only artifact check" in (unavailable.details or "") - checked_criteria = mock_checker.check_all_async.await_args.args[0] + checked_criteria = checker.check_all_async.await_args.args[0] assert [criterion.type for criterion in checked_criteria] == ["file_exists"] + assert not sandbox_dir.exists(), "cleanup must run after diagnostic grading" persisted = EvaluationResult.model_validate_json((run_dir / "task.json").read_text()) assert persisted.final_status == "ERROR" assert persisted.weighted_score == 0.0 assert [r.evaluation_status for r in persisted.post_failure_criteria_results] == [ + "not_evaluated", "evaluated", "not_evaluated", + "not_evaluated", ] +@pytest.mark.parametrize( + "recovery_error", + [ + pytest.param(JudgeInfrastructureError("judge unavailable"), id="judge-infrastructure"), + pytest.param(CheckerMisuseError("checker contract violated"), id="checker-misuse"), + pytest.param(None, id="result-count-mismatch"), + ], +) @pytest.mark.asyncio -async def test_post_failure_judge_infrastructure_error_still_escalates(tmp_path) -> None: +async def test_post_failure_checker_error_preserves_terminal_agent_error( + tmp_path, recovery_error: Exception | None +) -> None: task = _make_task(turn_timeout=1200, task_timeout=1500) task.success_criteria = [ FileExistsCriterion(type="file_exists", path="artifact.txt", description="artifact exists") @@ -415,12 +450,18 @@ async def test_post_failure_judge_infrastructure_error_still_escalates(tmp_path) orchestrator._setup = AsyncMock() # type: ignore[method-assign] orchestrator._cleanup = AsyncMock() # type: ignore[method-assign] orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] - orchestrator._evaluation_loop = AsyncMock( # type: ignore[method-assign] - side_effect=TurnTimeoutError(1200, task_id=task.task_id, iteration=1) - ) + terminal_error = TurnTimeoutError(1200, task_id=task.task_id, iteration=1) + orchestrator._evaluation_loop = AsyncMock(side_effect=terminal_error) # type: ignore[method-assign] orchestrator.sandbox = MagicMock() orchestrator.success_checker = MagicMock() - orchestrator.success_checker.check_all_async = AsyncMock(side_effect=JudgeInfrastructureError("judge unavailable")) + if recovery_error is None: + orchestrator.success_checker.check_all_async = AsyncMock(return_value=[]) + expected_type = "ValueError" + expected_message = "Post-failure checker returned 0 results for 1 runnable criteria" + else: + orchestrator.success_checker.check_all_async = AsyncMock(side_effect=recovery_error) + expected_type = type(recovery_error).__name__ + expected_message = str(recovery_error) orchestrator.agent = MagicMock() orchestrator.agent.get_sdk_options.return_value = None @@ -428,8 +469,47 @@ async def test_post_failure_judge_infrastructure_error_still_escalates(tmp_path) result = await orchestrator.run() assert result.final_status == "ERROR" - assert result.error_message == "judge unavailable" + assert result.error_message == str(terminal_error) assert result.weighted_score == 0.0 + assert len(result.post_failure_criteria_results) == 1 + unavailable = result.post_failure_criteria_results[0] + assert unavailable.evaluation_status == "not_evaluated" + assert expected_type in (unavailable.details or "") + assert expected_message in (unavailable.details or "") + + +@pytest.mark.asyncio +async def test_task_timeout_during_post_failure_grading_preserves_agent_error(tmp_path) -> None: + task = _make_task(turn_timeout=1200, task_timeout=0.1) + run_dir = tmp_path / "run" / "diagnostic_timeout" + run_dir.mkdir(parents=True) + orchestrator = Orchestrator(task=task, run_dir=run_dir, variant_id="test-variant") + orchestrator._setup = AsyncMock() # type: ignore[method-assign] + orchestrator._cleanup = AsyncMock() # type: ignore[method-assign] + orchestrator._refresh_runtime_tool_versions = MagicMock() # type: ignore[method-assign] + terminal_error = TurnTimeoutError(1200, task_id=task.task_id, iteration=1) + orchestrator._evaluation_loop = AsyncMock(side_effect=terminal_error) # type: ignore[method-assign] + orchestrator.sandbox = MagicMock() + orchestrator.success_checker = MagicMock() + + async def slow_check(*_args, **_kwargs): + await asyncio.sleep(10) + + orchestrator.success_checker.check_all_async = slow_check + orchestrator.agent = MagicMock() + orchestrator.agent.kill_sync = MagicMock() + orchestrator.agent.get_sdk_options.return_value = None + + with patch("coder_eval.orchestrator.load_reference", return_value=(None, None, None)): + result = await orchestrator.run() + + assert result.final_status == "ERROR" + assert result.error_message == str(terminal_error) + assert result.weighted_score == 0.0 + assert len(result.post_failure_criteria_results) == 1 + unavailable = result.post_failure_criteria_results[0] + assert unavailable.evaluation_status == "not_evaluated" + assert "task_timeout budget expired during post-failure grading" in (unavailable.details or "") def test_runtime_timeout_warning_is_emitted_once(tmp_path, caplog) -> None: