feat(cli): env0-shaped failure breakdowns; details pointer keyed off on-disk artifacts

docs/reference/cli.md

@@ -301,6 +301,11 @@ alike (so it also wins over an exported `on`). Note that on a TTY,
 mutes INFO logging while it owns the screen; pair it with
 `BENCHFLOW_NO_PROGRESS=1` to get plain heartbeat lines on a TTY.
 
+The final `Score: P/T (…%)` line is pass-threshold aggregation — a task counts
+as passed only at reward 1.0 — while `mean reward` beside it is the average raw
+verifier reward, so `0/1 (0.0%)` next to `mean reward 0.80` means partial
+credit below the pass threshold, not a flat zero.
+
 Set `BENCHFLOW_ACP_HANDSHAKE_TIMEOUT` to a number of seconds (default 60) to
 give slow-starting agents more time to answer the pre-prompt ACP handshake
 (`initialize`/`session_new`) — heavyweight task images can push agent startup

src/benchflow/cli/_failure_evidence.py

@@ -4,34 +4,41 @@
 task's reason would otherwise be the bare ``reward X`` fallback, mine the
 rollout's verifier artifacts for a one-line explanation. Lives in its own
 module so ``cli/_shared.py`` stays the side-effect-free display helpers it
-advertises.
+advertises. Also home of :func:`metric_breakdown`, the one canonical
+rewards-dict flattening — shared by ``_shared.py``'s in-memory metric tier and
+the ``reward.json`` probe here, so the two render identically.
 
 Contract (the engine stays file-free — these reads are CLI-side, report-time
 only):
 
 - The rollout dir is resolved exactly, from the ``rollout_name`` the engine
   records on every result — never guessed from globs.
-- Evidence sources are tried in order (CTRF report, then test-stdout tail);
-  the first that yields a line wins.
+- Evidence sources are tried in order (CTRF report, then reward.json metric
+  breakdown, then test-stdout tail); the first that yields a line wins.
 - Every read is bounded (``_ARTIFACT_READ_BYTES`` per file) and nothing here
   raises — any surprise degrades to ``None``, i.e. the bare reward reason.
+- The report block's ``(details: …)`` pointer is decided separately, by
+  :func:`verifier_dir_for` from dir existence alone — every failure block
+  with artifacts on disk gets one pointer, evidence or not.
 """
 
 from __future__ import annotations
 
 import json
 import re
-from typing import TYPE_CHECKING, NamedTuple
+from typing import TYPE_CHECKING, Any, NamedTuple
 
 if TYPE_CHECKING:
-    from collections.abc import Callable
+    from collections.abc import Callable, Mapping
     from pathlib import Path
 
 # Never read more than this many bytes per file, and evidence is only mined
 # for the <= _MAX_FAILURE_LINES tasks the report block displays.
 _ARTIFACT_READ_BYTES = 64 * 1024
 # Pytest final summary, decoration stripped: "1 failed, 2 passed in 40.86s".
 _PYTEST_SUMMARY_RE = re.compile(r"\d+ failed\b")
+# Metric-breakdown cap: one metric-happy verifier can't flood the line.
+_BREAKDOWN_METRICS = 3
 
 
 class FailureLine(NamedTuple):
@@ -49,6 +56,88 @@ class FailureLine(NamedTuple):
     suffix: str = ""
 
 
+def _numeric_items(mapping: Mapping[str, Any]) -> list[tuple[str, Any]]:
+    """The named numeric metrics of one mapping level, insertion-ordered.
+
+    ``reward`` is excluded at every level: at the top it is the aggregate the
+    line already leads with, and a nested ``reward`` would render a confusing
+    ``reward 0.8 — reward 0.8``.
+    """
+    return [
+        (name, value)
+        for name, value in mapping.items()
+        if name != "reward" and isinstance(value, (bool, int, float))
+    ]
+
+
+def metric_breakdown(rewards: Mapping[str, Any]) -> str | None:
+    """Compact ``name value`` metric breakdown of a rewards mapping, capped at
+    ``_BREAKDOWN_METRICS`` — or ``None`` when it holds no named metrics.
+
+    The numeric evidence may sit flat in the mapping, or nested one level under
+    a ``metrics`` sub-dict (env0-style verifiers: ``{"reward": …, "metrics":
+    {…}, "details": {…}}``), or under ``details`` when ``metrics`` yields
+    nothing. Flat keys win outright — a flat rewards dict renders exactly as
+    before.
+
+    Ordering leads with the lowest-signal metrics — they explain the miss:
+
+    - A ``<name>_found`` metric with a matching positive numeric
+      ``<name>_total`` anywhere in the mapping renders as ``<name> found/total``
+      and sorts by that fraction, so ``deadlines 1/5`` leads even though 1 is
+      not 0. A total consumed by a pair is dropped from the shown list (the
+      fraction already carries it).
+    - Unpaired metrics keep the zero-first rule: zero/failed first, stable
+      within each group.
+    """
+    shown = _numeric_items(rewards)
+    if not shown:
+        for key in ("metrics", "details"):
+            sub = rewards.get(key)
+            if isinstance(sub, dict):
+                shown = _numeric_items(sub)
+                if shown:
+                    break
+    if not shown:
+        return None
+    # found/total pairing looks across ALL levels: env0 keeps the found counts
+    # under "metrics" and the totals under "details". First-seen wins.
+    totals: dict[str, Any] = {}
+    for source in (rewards, rewards.get("metrics"), rewards.get("details")):
+        if isinstance(source, dict):
+            for name, value in _numeric_items(source):
+                totals.setdefault(name, value)
+
+    def _pair_total(name: str, value: Any) -> Any | None:
+        """The positive numeric ``<base>_total`` for a ``<base>_found`` metric,
+        else ``None``. Bools (either side) and non-positive totals never pair."""
+        base = name.removesuffix("_found")
+        if base == name or isinstance(value, bool):
+            return None
+        total = totals.get(base + "_total")
+        if isinstance(total, bool) or not isinstance(total, (int, float)):
+            return None
+        return total if total > 0 else None
+
+    consumed = {
+        name.removesuffix("_found") + "_total"
+        for name, value in shown
+        if _pair_total(name, value) is not None
+    }
+    entries: list[tuple[float, str]] = []
+    for name, value in shown:
+        if name in consumed:
+            continue
+        total = _pair_total(name, value)
+        if total is not None:
+            base = name.removesuffix("_found")
+            entries.append((value / total, f"{base} {value}/{total}"))
+        else:
+            entries.append((0.0 if value == 0 else 1.0, f"{name} {value}"))
+    entries.sort(key=lambda entry: entry[0])  # stable: ties keep dict order
+    return ", ".join(fragment for _, fragment in entries[:_BREAKDOWN_METRICS])
+
+
 def _display_test_name(raw_name: str) -> str:
     """Test name for display: node-id path segments dropped, param id kept.
 
@@ -65,6 +154,19 @@ def _display_test_name(raw_name: str) -> str:
     return head.rsplit("::", 1)[-1] + bracket + param
 
 
+def _bounded_json(path: Path) -> dict[str, Any] | None:
+    """The file parsed as a JSON object, or ``None`` when it can't serve.
+
+    JSON only parses whole — an oversized (> ``_ARTIFACT_READ_BYTES``) file is
+    skipped, not truncated, and a non-object payload is skipped the same way;
+    either way the next evidence source gets its turn.
+    """
+    if path.stat().st_size > _ARTIFACT_READ_BYTES:
+        return None
+    data = json.loads(path.read_text(encoding="utf-8", errors="replace"))
+    return data if isinstance(data, dict) else None
+
+
 def _ctrf_failure_line(ctrf_path: Path) -> FailureLine | None:
     """``<test> failed[: <assertion>]`` from the first failed CTRF test.
 
@@ -76,11 +178,9 @@ def _ctrf_failure_line(ctrf_path: Path) -> FailureLine | None:
     carries a count suffix (``(+N more failures; P/T checks passed)``) so the
     console never under-reports how much is broken.
     """
-    if ctrf_path.stat().st_size > _ARTIFACT_READ_BYTES:
-        # JSON only parses whole — an oversized report is skipped (not
-        # truncated) and the next evidence source gets its turn.
+    data = _bounded_json(ctrf_path)
+    if data is None:
         return None
-    data = json.loads(ctrf_path.read_text(encoding="utf-8", errors="replace"))
     raw_tests = (data.get("results") or {}).get("tests") or []
     tests = [test for test in raw_tests if isinstance(test, dict)]
     failed = [test for test in tests if test.get("status") == "failed"]
@@ -115,6 +215,25 @@ def _ctrf_failure_line(ctrf_path: Path) -> FailureLine | None:
     return FailureLine(f"{name} failed", suffix)
 
 
+def _reward_json_failure_line(reward_path: Path) -> FailureLine | None:
+    """Metric breakdown mined from the verifier's ``reward.json``.
+
+    env0-style verifiers write ``{"reward": …, "metrics": {…}, "details":
+    {…}}`` (and non-pytest stdout, no CTRF report) — when the result's rewards
+    dict was stripped to the bare aggregate (older persisted results, sharded
+    aggregation), the numeric evidence still sits on disk. Reuses the same
+    one-level flattening as the in-memory reward-dict tier so the two render
+    identically. A file with no named metrics repeats nothing the line does
+    not already say — it yields to the stdout tail. No count suffix: the
+    breakdown is a metric summary, not a first-of-N failure pick.
+    """
+    data = _bounded_json(reward_path)
+    if data is None:
+        return None
+    shown = metric_breakdown(data)
+    return FailureLine(shown) if shown is not None else None
+
+
 def _stdout_tail_failure_line(stdout_path: Path) -> FailureLine | None:
     """Last pytest summary ("N failed, M passed …") — else last ``FAILED …``
     line — from a bounded tail of the verifier's test-stdout capture. No count
@@ -133,13 +252,28 @@ def _stdout_tail_failure_line(stdout_path: Path) -> FailureLine | None:
     return FailureLine(last_failed) if last_failed is not None else None
 
 
-def artifact_failure_evidence(
-    job_dir: Path, rollout_name: str
-) -> tuple[FailureLine, Path] | None:
-    """``FailureLine`` mined from the rollout's verifier artifacts, or ``None``.
+def verifier_dir_for(job_dir: Path, rollout_name: str) -> Path | None:
+    """The rollout's verifier dir when it exists on disk, else ``None``.
+
+    Powers the report block's ``(details: …)`` pointer rule: every failure
+    block with artifacts on disk gets one pointer — from dir existence alone,
+    independent of which tier supplied the reason line (the pointer is most
+    valuable exactly when evidence extraction fails). Never raises.
+    """
+    # Local import: RolloutPaths pulls in the task package (see
+    # artifact_failure_evidence).
+    from benchflow.task.paths import RolloutPaths
+
+    try:
+        verifier_dir = RolloutPaths(rollout_dir=job_dir / rollout_name).verifier_dir
+        return verifier_dir if verifier_dir.is_dir() else None
+    except Exception:
+        return None
+
 
-    Returns the structured one-liner plus the verifier dir it came from (for
-    the report block's single ``(details: …)`` pointer line). Never raises.
+def artifact_failure_evidence(job_dir: Path, rollout_name: str) -> FailureLine | None:
+    """``FailureLine`` mined from the rollout's verifier artifacts, or ``None``
+    when every probe misses. Never raises.
     """
     # Local import: RolloutPaths pulls in the task package, which the CLI
     # shouldn't pay for until a failure actually needs artifact evidence.
@@ -151,6 +285,7 @@ def artifact_failure_evidence(
         # ctrf.json has no RolloutPaths property — the verifier recovers it by
         # literal name (verifier_core.py, `_recover_main_verifier_outputs`).
         (verifier_dir / "ctrf.json", _ctrf_failure_line),
+        (rollout_paths.reward_json_path, _reward_json_failure_line),
         (rollout_paths.test_stdout_path, _stdout_tail_failure_line),
     ]
     for path, extract in attempts:
@@ -163,5 +298,5 @@ def artifact_failure_evidence(
             # the report must never crash over evidence mining.
             continue
         if detail is not None:
-            return detail, verifier_dir
+            return detail
     return None

src/benchflow/cli/_shared.py

@@ -21,7 +21,12 @@
 from rich.markup import escape
 
 from benchflow._utils.text import truncate_end
-from benchflow.cli._failure_evidence import FailureLine, artifact_failure_evidence
+from benchflow.cli._failure_evidence import (
+    FailureLine,
+    artifact_failure_evidence,
+    metric_breakdown,
+    verifier_dir_for,
+)
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -110,55 +115,38 @@ def _exit_if_evaluation_had_errors(result: object) -> None:
 # count suffix, which deliberately run ~40 chars past the budget.
 _MAX_FAILURE_LINES = 5
 _FAILURE_LINE_LIMIT = 100
-_FAILURE_REASON_METRICS = 3
 
 
-def _failure_reason(
-    failure: TaskFailure, job_dir: Path | None = None
-) -> tuple[FailureLine, Path | None]:
-    """One cheap line explaining why a FAILED (scored, reward != 1) task
-    failed, plus the verifier dir when artifacts supplied the evidence.
+def _failure_reason(failure: TaskFailure, job_dir: Path | None = None) -> FailureLine:
+    """One cheap line explaining why a FAILED (scored, reward != 1) task failed.
 
     Priority: the verifier's own error if set; else the reward plus a compact
-    breakdown of the named metrics in the reward dict (zero/failed metrics
-    first — they explain the miss); else the reward plus a one-liner mined
-    from the rollout's verifier artifacts (bounded CLI-side reads resolved via
-    the ``rollout_name`` the engine records — the engine stays file-free);
-    else just the reward. The reason is a ``FailureLine`` so the artifact
-    tier's multi-failure count suffix stays separable from the truncatable
-    body (only the artifact tier ever sets it). The returned path is
-    non-``None`` only for the artifact tier, so the caller can print one
-    ``(details: …)`` pointer per report block.
+    breakdown of the named metrics in the reward dict — flat, or flattened one
+    level from an env0-style ``metrics``/``details`` sub-dict, lowest-signal
+    metrics first (see :func:`metric_breakdown`); else the reward plus a
+    one-liner mined from the rollout's verifier artifacts (bounded CLI-side
+    reads resolved via the ``rollout_name`` the engine records — the engine
+    stays file-free); else just the reward. The reason is a ``FailureLine`` so
+    the artifact tier's multi-failure count suffix stays separable from the
+    truncatable body (only the artifact tier ever sets it). The block's
+    ``(details: …)`` pointer is NOT decided here — the caller keys it off
+    on-disk artifacts alone, independent of which tier supplied the reason.
     """
     if failure.verifier_error:
         # Collapse whitespace: verifier errors are routinely multi-line.
-        return FailureLine(" ".join(failure.verifier_error.split())), None
+        return FailureLine(" ".join(failure.verifier_error.split()))
     rewards = failure.rewards or {}
     reward = rewards.get("reward")
-    metrics = [
-        (name, value)
-        for name, value in rewards.items()
-        if name != "reward" and isinstance(value, (bool, int, float))
-    ]
-    if metrics:
-        # Zero/failed metrics first (stable within each group), capped so one
-        # metric-happy verifier can't flood the line.
-        metrics.sort(key=lambda kv: kv[1] != 0)
-        shown = ", ".join(
-            f"{name} {value}" for name, value in metrics[:_FAILURE_REASON_METRICS]
-        )
-        return FailureLine(f"reward {reward} — {shown}"), None
+    shown = metric_breakdown(rewards)
+    if shown is not None:
+        return FailureLine(f"reward {reward} — {shown}")
     # No rollout_name (pre-#957-follow-up result.json, sharded aggregation):
     # nothing to resolve — the bare reward is as honest as it gets.
     if job_dir is not None and failure.rollout_name:
-        evidence = artifact_failure_evidence(job_dir, failure.rollout_name)
-        if evidence is not None:
-            detail, verifier_dir = evidence
-            return (
-                FailureLine(f"reward {reward} — {detail.body}", detail.suffix),
-                verifier_dir,
-            )
-    return FailureLine(f"reward {reward}"), None
+        detail = artifact_failure_evidence(job_dir, failure.rollout_name)
+        if detail is not None:
+            return FailureLine(f"reward {reward} — {detail.body}", detail.suffix)
+    return FailureLine(f"reward {reward}")
 
 
 def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -> None:
@@ -170,8 +158,9 @@ def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -
     dim ``✗ task: reason`` line (capped at ``_MAX_FAILURE_LINES``) so the "why"
     doesn't require opening summary.json; when ``job_dir`` is given, a reason
     that would otherwise be a bare ``reward X`` is upgraded from the rollout's
-    verifier artifacts (bounded reads, displayed failures only), with a single
-    ``(details: …/verifier)`` pointer line for the block. When ``job_dir`` is
+    verifier artifacts (bounded reads, displayed failures only), and every
+    failure block with artifacts on disk gets one ``(details: …/verifier)``
+    pointer line — evidence mined or not. When ``job_dir`` is
     given, the result/summary paths are printed so testers know where to look
     (the guide repeatedly says "read summary.json" but the CLI never said
     where).
@@ -210,9 +199,14 @@ def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -
     failures = getattr(result, "task_failures", None) or []
     artifact_pointer: Path | None = None
     for failure in failures[:_MAX_FAILURE_LINES]:
-        reason, verifier_dir = _failure_reason(failure, job_dir)
-        if artifact_pointer is None and verifier_dir is not None:
-            artifact_pointer = verifier_dir
+        reason = _failure_reason(failure, job_dir)
+        # Pointer rule: every failure block with artifacts on disk gets one
+        # (details:) pointer — the first displayed failure whose verifier dir
+        # exists, independent of which tier supplied its reason line (byte-
+        # identical reasons must not differ on provenance the console can't
+        # show, and the pointer matters most when every probe missed).
+        if artifact_pointer is None and job_dir is not None and failure.rollout_name:
+            artifact_pointer = verifier_dir_for(job_dir, failure.rollout_name)
         # The char budget governs only the free-text body; the multi-failure
         # count suffix is appended AFTER truncation so a long assertion can
         # never swallow the "more than this is broken" signal. Worst case the
@@ -226,8 +220,9 @@ def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -
     extra = len(failures) - _MAX_FAILURE_LINES
     if extra > 0:
         console.print(f"[dim]  … and {extra} more[/dim]")
-    # One pointer per block (not per line): the first artifact-backed reason's
-    # verifier dir, so "where do I look next" needs no summary.json dig either.
+    # One pointer per block (not per line): the first displayed failure whose
+    # verifier dir exists — artifact-backed or not — so "where do I look next"
+    # needs no summary.json dig even when evidence mining came up empty.
     if artifact_pointer is not None:
         console.print(f"[dim]  (details: {escape(str(artifact_pointer))})[/dim]")
     if job_dir is not None: