feat(cli): full failure count on per-task console lines; preserve param ids

src/benchflow/cli/_failure_evidence.py

@@ -21,7 +21,7 @@
 
 import json
 import re
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, NamedTuple
 
 if TYPE_CHECKING:
     from collections.abc import Callable
@@ -34,46 +34,91 @@
 _PYTEST_SUMMARY_RE = re.compile(r"\d+ failed\b")
 
 
-def _ctrf_failure_line(ctrf_path: Path) -> str | None:
+class FailureLine(NamedTuple):
+    """A failure one-liner in two parts with distinct truncation contracts.
+
+    ``body`` is the free-text evidence (test name plus assertion) and competes
+    for the display line's char budget; ``suffix`` is the compact multi-failure
+    roll-up count (empty when the evidence covers everything) and must survive
+    display truncation. Renderers give ``body`` the full line budget and append
+    ``suffix`` whole past it — never truncating the concatenation, or a long
+    assertion would silently eat the "there is more broken than this" signal.
+    """
+
+    body: str
+    suffix: str = ""
+
+
+def _display_test_name(raw_name: str) -> str:
+    """Test name for display: node-id path segments dropped, param id kept.
+
+    Full pytest node ids ("tests/test_x.py::test_build[param]") waste the
+    100-char line budget; keep the function name plus any ``[param]`` id.
+    The split must not touch the bracket part — a param id may itself contain
+    ``::`` — so only the text before the first ``[`` is segment-split. (When
+    the report was written by pytest-json-ctrf — as of 0.5.x — the param id
+    is already gone — the plugin stores ``nodeid.split('[')[0]`` as ``name``
+    — but other CTRF producers keep the full name, and we must not re-trim
+    it.)
+    """
+    head, bracket, param = raw_name.partition("[")
+    return head.rsplit("::", 1)[-1] + bracket + param
+
+
+def _ctrf_failure_line(ctrf_path: Path) -> FailureLine | None:
     """``<test> failed[: <assertion>]`` from the first failed CTRF test.
 
     The verifier's pytest run writes a CTRF report (``pytest --ctrf``, see the
     standard path in ``task_authoring/structural_checks.py``). Per test the
     useful failure evidence is the ``E …`` assertion line inside ``trace``;
     ``message`` is only a generic phase description, so it is the fallback.
+    When the report holds more than one failed test, the first failure's line
+    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.
         return None
     data = json.loads(ctrf_path.read_text(encoding="utf-8", errors="replace"))
-    tests = (data.get("results") or {}).get("tests") or []
-    for test in tests:
-        if not isinstance(test, dict) or test.get("status") != "failed":
-            continue
-        # Full pytest node ids ("tests/test_x.py::test_build") waste the
-        # 100-char line budget; keep the test function name.
-        name = str(test.get("name") or "test").rsplit("::", 1)[-1]
-        trace = test.get("trace")
-        if isinstance(trace, str):
-            for line in trace.splitlines():
-                stripped = line.strip()
-                if stripped.startswith("E "):
-                    assertion = stripped[2:].strip()
-                    # "<test> failed:" already says it's an assertion — drop
-                    # the prefix to reclaim line budget for the message.
-                    assertion = assertion.removeprefix("AssertionError: ")
-                    return f"{name} failed: {assertion}"
-        message = test.get("message")
-        if isinstance(message, str) and message.strip():
-            return f"{name} failed: {' '.join(message.split())}"
-        return f"{name} failed"
-    return None
-
-
-def _stdout_tail_failure_line(stdout_path: Path) -> str | None:
+    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"]
+    if not failed:
+        return None
+    suffix = ""
+    if len(failed) > 1:
+        # Counts come from the same rolled-up test list the first-failure pick
+        # uses, so the suffix can never disagree with the shown evidence.
+        # T is all entries — skips count against the denominator, not as passes.
+        extra = len(failed) - 1
+        passed = sum(1 for test in tests if test.get("status") == "passed")
+        plural = "" if extra == 1 else "s"
+        suffix = (
+            f" (+{extra} more failure{plural}; {passed}/{len(tests)} checks passed)"
+        )
+    test = failed[0]
+    name = _display_test_name(str(test.get("name") or "test"))
+    trace = test.get("trace")
+    if isinstance(trace, str):
+        for line in trace.splitlines():
+            stripped = line.strip()
+            if stripped.startswith("E "):
+                assertion = stripped[2:].strip()
+                # "<test> failed:" already says it's an assertion — drop
+                # the prefix to reclaim line budget for the message.
+                assertion = assertion.removeprefix("AssertionError: ")
+                return FailureLine(f"{name} failed: {assertion}", suffix)
+    message = test.get("message")
+    if isinstance(message, str) and message.strip():
+        return FailureLine(f"{name} failed: {' '.join(message.split())}", suffix)
+    return FailureLine(f"{name} failed", suffix)
+
+
+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."""
+    line — from a bounded tail of the verifier's test-stdout capture. No count
+    suffix: the summary line already carries the full counts itself."""
     with stdout_path.open("rb") as fh:
         fh.seek(0, 2)  # SEEK_END
         fh.seek(max(0, fh.tell() - _ARTIFACT_READ_BYTES))
@@ -82,27 +127,27 @@ def _stdout_tail_failure_line(stdout_path: Path) -> str | None:
     for line in reversed(tail.splitlines()):
         bare = line.strip().strip("=").strip()
         if _PYTEST_SUMMARY_RE.match(bare):
-            return bare
+            return FailureLine(bare)
         if last_failed is None and line.strip().startswith("FAILED "):
             last_failed = line.strip()
-    return last_failed
+    return FailureLine(last_failed) if last_failed is not None else None
 
 
 def artifact_failure_evidence(
     job_dir: Path, rollout_name: str
-) -> tuple[str, Path] | None:
-    """One-liner mined from the rollout's verifier artifacts, or ``None``.
+) -> tuple[FailureLine, Path] | None:
+    """``FailureLine`` mined from the rollout's verifier artifacts, or ``None``.
 
-    Returns the one-liner plus the verifier dir it came from (for the report
-    block's single ``(details: …)`` pointer line). Never raises.
+    Returns the structured one-liner plus the verifier dir it came from (for
+    the report block's single ``(details: …)`` pointer line). Never raises.
     """
     # Local import: RolloutPaths pulls in the task package, which the CLI
     # shouldn't pay for until a failure actually needs artifact evidence.
     from benchflow.task.paths import RolloutPaths
 
     rollout_paths = RolloutPaths(rollout_dir=job_dir / rollout_name)
     verifier_dir = rollout_paths.verifier_dir
-    attempts: list[tuple[Path, Callable[[Path], str | None]]] = [
+    attempts: list[tuple[Path, Callable[[Path], FailureLine | None]]] = [
         # 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),

src/benchflow/cli/_shared.py

@@ -21,7 +21,7 @@
 from rich.markup import escape
 
 from benchflow._utils.text import truncate_end
-from benchflow.cli._failure_evidence import artifact_failure_evidence
+from benchflow.cli._failure_evidence import FailureLine, artifact_failure_evidence
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -106,15 +106,16 @@ def _exit_if_evaluation_had_errors(result: object) -> None:
 
 
 # Final-block failure lines: keep the block skimmable on big jobs and each
-# line inside a typical terminal width.
+# line inside a typical terminal width — except lines carrying a multi-failure
+# 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[str, Path | 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.
 
@@ -123,13 +124,15 @@ def _failure_reason(
     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 returned path is non-``None`` only for the
-    artifact tier, so the caller can print one ``(details: …)`` pointer per
-    report block.
+    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.
     """
     if failure.verifier_error:
         # Collapse whitespace: verifier errors are routinely multi-line.
-        return " ".join(failure.verifier_error.split()), None
+        return FailureLine(" ".join(failure.verifier_error.split())), None
     rewards = failure.rewards or {}
     reward = rewards.get("reward")
     metrics = [
@@ -144,15 +147,18 @@ def _failure_reason(
         shown = ", ".join(
             f"{name} {value}" for name, value in metrics[:_FAILURE_REASON_METRICS]
         )
-        return f"reward {reward} — {shown}", None
+        return FailureLine(f"reward {reward} — {shown}"), None
     # 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 f"reward {reward} — {detail}", verifier_dir
-    return f"reward {reward}", None
+            return (
+                FailureLine(f"reward {reward} — {detail.body}", detail.suffix),
+                verifier_dir,
+            )
+    return FailureLine(f"reward {reward}"), None
 
 
 def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -> None:
@@ -207,11 +213,16 @@ def _report_eval_result(result: EvaluationResult, job_dir: Path | None = None) -
         reason, verifier_dir = _failure_reason(failure, job_dir)
         if artifact_pointer is None and verifier_dir is not None:
             artifact_pointer = verifier_dir
-        line = truncate_end(
-            f"  ✗ {failure.task_name}: {reason}",
+        # 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
+        # line runs ~40 chars past the budget — completeness beats strict
+        # width for this one signal.
+        body = truncate_end(
+            f"  ✗ {failure.task_name}: {reason.body}",
             _FAILURE_LINE_LIMIT,
         )
-        console.print(f"[dim]{escape(line)}[/dim]")
+        console.print(f"[dim]{escape(body + reason.suffix)}[/dim]")
     extra = len(failures) - _MAX_FAILURE_LINES
     if extra > 0:
         console.print(f"[dim]  … and {extra} more[/dim]")