feat(evaluation): preserve criteria after agent failures

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 ""

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-<idx>.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-<idx>.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.

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.

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))
 
 

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,
+    )