feat(cli): full failure count on per-task console lines; preserve param ids - #962
Conversation
…am ids
The per-failed-task console line mined from a verifier CTRF report named
only the first failed test — a run with 2 failed / 4 passed checks read as
a single failure (observed on skillsbench dialogue-parser, reward 0.667).
When the report holds more than one failed test, the line now carries a
compact roll-up suffix: ' (+N more failure(s); P/T checks passed)', counted
from the same test list the first-failure pick uses.
Evidence is now a typed FailureLine(body, suffix) NamedTuple end-to-end:
the 100-char budget truncates only the free-text body, then the suffix is
appended whole, so a long assertion can never swallow the count signal
(worst case ~40 chars past the budget). The stdout-tail tier never sets a
suffix — its pytest summary line already carries the counts.
Test names also keep their parametrize id: the node-id segment split now
only touches the text before the first '[', so names like test_foo[a::b]
no longer collapse to 'b]' and ids survive whenever the report carries
them. (pytest-json-ctrf <= 0.5.3 strips ids at generation time via
nodeid.split('[')[0] — unrecoverable report-side — but other CTRF
producers keep full names.)
- FailureLine docstring states the implemented policy: body gets the full line budget, suffix is appended past it (not a reserved-suffix budget). - _FAILURE_LINE_LIMIT comment notes the deliberate ~40-char overrun for suffixed lines. - Record the T-denominator decision at the counting site (skips count against the denominator, not as passes). - Soften the pytest-json-ctrf version claim to 'as of 0.5.x'.
GitHub Actions
Inactive
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fb8827fda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert "(+" not in out | ||
|
|
||
|
|
||
| def test_ctrf_multi_failure_appends_count_suffix(tmp_path): |
There was a problem hiding this comment.
The newly added regression cases beginning here use comments instead of docstrings and do not identify the PR or commit they protect. Add a docstring naming 45a547f or the associated PR to each new regression test so future maintainers can trace why these cases must remain.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
1 participant
Dogfood follow-up to #959: on a real run (skillsbench dialogue-parser, reward 0.667) the CTRF held 2 failed / 4 passed tests, but the console line named only the first failure — a user stopping at the console under-counts what's broken.
What changed
(+N more failure(s); P/T checks passed). Dogfood case, verified against the actual run's ctrf.json:✗ dialogue-parser: reward 0.667 — test_graph_logic failed: Unreachable nodes found: ['End']... (+1 more failure; 4/6 checks passed)T counts all entries (skips count against the denominator, not as passes — the least-misleading option; documented at the counting site).
FailureLine(body, suffix)— the body takes the full 100-char budget, the ~40-char suffix deliberately rides past it (reserving budget for the suffix would truncate away the test name itself; consistent with feat(cli): failure reasons from verifier artifacts (CTRF / test-stdout) #959's untruncated pointer precedent, documented at both the constant and the render site).::-split now splits only before the first[, fixing a latent mangling of ids containing::(test_foo[a::b]previously displayed asb]). Investigation finding, verified against the installed plugin source: pytest-json-ctrf itself strips the parametrize id at generation time (nodeid.split('[')[0], as of 0.5.x), so ids are unrecoverable for that producer — no heuristic recovery added; spec-conformant producers now display intact.Review
Structural review before opening: APPROVE-WITH-MINORS — the
FailureLineseam, the rejected-alternative arithmetic, and the revert-kill test matrix (all end-to-end through_report_eval_result) validated; the three minors were docstring/comment accuracy on the truncation contract, all applied. Includes a clean merge of main (#960's mean-reward line renders directly above the failure block; both verified intact).Gates: ruff format/check, ty; test_cli_live_progress 34 passed;
-k "cli"369 passed.