feat(cli): env0-shaped failure breakdowns; details pointer keyed off on-disk artifacts - #964
Merged
Merged
Conversation
…nce-less details pointer Observed on a real env0 run (gdoc-extract-content, reward 0.8): the per-failed- task console line printed the bare 'reward 0.8' fallback while the decisive fact (deadlines 1/5 found) sat in memory and on disk. Three stacked misses, three fixes: - The metric-breakdown tier read only flat numeric values in the rewards dict; env0-style verifiers nest them one level under 'metrics' (totals under 'details'). metric_breakdown() — the one canonical flattening, now in _failure_evidence.py so both the in-memory tier and the artifact probe share it — flattens one level (metrics, else details; flat keys still win), pairs <name>_found with a positive <name>_total into a 'deadlines 1/5' fraction, and sorts lowest-signal first (fraction when paired, zero-first otherwise), so the line leads with the metric that explains the miss. - The artifact tier probed only ctrf.json and a pytest-shaped stdout tail; env0 writes verifier/reward.json and non-pytest stdout. A reward.json probe now sits between them (bounded read, same never-raise contract) and mines the same flattened breakdown from disk. - The '(details: …)' pointer printed only when the artifact tier yielded evidence — exactly when extraction failed, the user also lost the place to look. artifact_failure_evidence() now returns the verifier dir whenever it exists, evidence or not, keeping the once-per-block + first-failure semantics. Also documents the Score line's pass threshold in docs/reference/cli.md: '0/1 (0.0%)' beside 'mean reward 0.80' is partial credit below the reward-1.0 threshold, not a flat zero. Console strings unchanged except the failure lines.
…ded-JSON read Review follow-ups to the nested-metrics commit: - The details-pointer rule stopped halfway: it printed iff the reason fell through to the artifact tier and the dir existed, so an in-memory env0 breakdown and the byte-identical reward.json-mined one differed on a provenance the console can't show. New one-sentence rule: every failure block with artifacts on disk gets one (details:) pointer — computed in _report_eval_result from the first displayed failure whose rollout_name resolves to an existing verifier dir, via the new verifier_dir_for(). This dissolves the widened tuple: _failure_reason returns FailureLine again and artifact_failure_evidence returns FailureLine | None. The yields-to-metric-breakdown test flips its no-pointer assertion (it pinned the abandoned rationale). - New both-present ordering test: reward.json beats a pytest-shaped stdout tail (reordering those two probes previously passed the suite). - _bounded_json() extracts the duplicated size-guard + whole-file JSON parse shared by the CTRF and reward.json probes (comment included). - Test factories _bare_failure/_metric_failure collapse into one _failure() with a rewards keyword defaulting to the bare reward-0.0 shape.
xdotli
temporarily deployed
to
pypi-internal-preview
August 9, 2026 18:51 — with 
GitHub Actions
Inactive
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: a25d09457c
ℹ️ 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".
| ) | ||
|
|
||
|
|
||
| def test_failure_reason_flattens_nested_metrics(): |
There was a problem hiding this comment.
The newly added regression tests, beginning with test_failure_reason_flattens_nested_metrics and continuing through the reward-artifact and pointer cases, describe regressions only in comments and have no docstrings naming the originating PR or commit. Add a traceability docstring to each regression test naming this commit or its PR, as required by the repository convention.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
This was referenced Aug 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round-7 dogfood follow-up: an env0 partial-credit run (gdoc-extract-content, reward 0.8) printed the bare fallback
✗ gdoc-extract-content: reward 0.8— no breakdown, no pointer — while the decisive fact (deadlines 1/5) sat both in memory and on disk. Three stacked misses: env0 verifiers nest their numbers underrewards['metrics'](the breakdown tier only read flat keys), the artifact tier probed only CTRF/pytest shapes (env0 writesverifier/reward.json), and the(details:)pointer only printed when the artifact tier fired — losing the pointer exactly when evidence extraction failed.What changed
metric_breakdown()(cli/_failure_evidence.py): flat numeric keys win unchanged; else flatten one level frommetrics, elsedetails(non-numerics filtered).<name>_found/<name>_totalpairs — totals resolved deterministically top-level → metrics → details, since env0 splits them across sub-dicts — render asdeadlines 1/5and sort by fraction; consumed totals dropped; zero-first + cap-3 unchanged (a pair costs one slot). Both the in-memory tier and the artifact tier consume the same helper, so they render identically.reward 0.8→reward 0.8 — deadlines 1/5, summary_doc_exists 1, decisions 5/5(asserted verbatim).verifier/reward.jsonprobe between ctrf.json and the stdout tail (viaRolloutPaths, shared_bounded_jsonreader, never-raise; metric-less files yield to stdout).(details:)pointer is now computed from the first displayed failure whose rollout has a verifier dir on disk — one sentence: every failure block with artifacts on disk gets one pointer, evidence mined or not. The initial cut printed it only when the artifact tier fired, which made byte-identical breakdown lines carry or omit the pointer based on invisible provenance; the review caught that halfway rule and the fix also deleted a widened return-tuple. Behavior note: verifier-error failures with on-disk artifacts now also claim the pointer.0/1besidemean reward 0.80is partial credit).Review
Structural review before opening (REQUEST-CHANGES → fixed): mechanics praised (canonical helper, net-zero _shared.py, mutation-killed tests incl. the verbatim dogfood case); the required change was finishing the pointer rule, which strictly simplified the code. Ordering fully pinned: CTRF > reward.json > stdout, each by a both-present test.
Gates: ruff format/check, ty (5 pre-existing diagnostics, byte-identical to base); evidence/job/docs-drift 113 passed;
-k "cli"388 passed.