Skip to content

feat(cli): surface fractional rewards in console summaries - #960

Merged
xdotli merged 2 commits into
mainfrom
feat/console-mean-reward
Aug 9, 2026
Merged

feat(cli): surface fractional rewards in console summaries#960
xdotli merged 2 commits into
mainfrom
feat/console-mean-reward

Conversation

@xdotli

@xdotli xdotli commented Aug 9, 2026

Copy link
Copy Markdown
Member

The console binarizes rewards (pass = reward==1), so a task scoring 0.3 partial credit printed identically to a flat 0 — observed on a real env0 run: [FAIL] gdoc-extract-content (tools=47), Job complete: 0/1 (0.0%), ✗ Score: 0/1 (0.0%) for a rollout whose verifier returned 0.3 with rich metrics. Rubric-style verifiers make 0<r<1 the common case; the fractional signal was console-invisible.

What changed

Binarized counts stay (thresholding is intentional); the fractional view renders alongside:

  • Per-task lines carry the scored reward — [FAIL] gdoc-extract-content (reward=0.30, tools=47); error lines (reward None) unchanged.
  • EvaluationResult.mean_reward: mean over scored rollouts via a new canonical mean_scored_reward() in _utils/scoring.py (built on extract_reward; excludes bools — classify_result treats a persisted true as a pass but a bool is not a magnitude — and non-finite values, since the resume path feeds unvalidated json.loads payloads). None when nothing scored — errors are excluded, not zeroed, so an all-error run shows no fabricated 0.00.
  • Job-complete log line: Job complete: 0/1 (0.0%), mean_reward=0.30, errors=0, … (key=value, matching its neighbors).
  • CLI Score line: ✗ Score: 0/1 (0.0%), mean reward 0.30, errors=0 (prose, matching the failure-reason lines below it). getattr-guarded: sharded aggregation omits the segment.
  • summary.json gains the same mean_reward field (additive), pinned end-to-end by test.

Review

Structural review before opening (REQUEST-CHANGES → fixed): the initial construction-site extraction was a fourth ad-hoc read of rewards.reward one import away from the canonical helper — and the only read in the aggregation flow that could crash on a malformed resumed payload, plus a NaN hole. The shipped version extracts mean_scored_reward() beside pass_rate/count_score_outcomes with unit tests for the bool/NaN/malformed-shape edges. Known follow-ups deliberately not in scope: sharded workers don't report per-shard means (aggregate mean_reward stays None — needs worker payload + weighted merge), and bench eval metrics' post-hoc summary doesn't yet carry the field.

Gates: ruff format/check, ty; test_scoring + test_job + test_cli_live_progress 135 passed; -k "cli" 366 passed; oracle-chokepoint/loop-strategies substring assertions verified unaffected (110 passed).

xdotli added 2 commits August 9, 2026 10:24
The console binarizes rewards (pass = reward==1), so a task scoring 0.3
partial credit printed identically to a flat 0: [FAIL] task (tools=47),
Job complete: 0/1 (0.0%), Score: 0/1 (0.0%). Rubric-style verifiers make
0<r<1 the common case, and the fractional signal was console-invisible.

Keep the binarized counts (thresholding is intentional) and add the
fractional view alongside:

- per-task lines carry the scored reward: [FAIL] task (reward=0.30,
  tools=47); error lines (reward None) are unchanged
- EvaluationResult gains mean_reward (mean over scored rollouts, None
  when nothing scored — errors are excluded, not zeroed)
- the job-complete log line carries mean_reward=0.30 and the CLI Score
  line renders ', mean reward 0.30' when available (getattr-guarded:
  sharded aggregation doesn't carry it)
- summary.json gains the same mean_reward field for machine consumers
…-up)

The construction-site extraction was a fourth ad-hoc read of
rewards.reward, written one import away from the canonical
extract_reward — and the only read in the aggregation flow that could
crash on malformed resume data ((rewards or {}).get raises on a truthy
non-dict rewards, and the resume path feeds raw json.loads payloads
with no shape validation). It also admitted NaN into the mean.

Extract mean_scored_reward() into _utils/scoring.py built on
extract_reward with the bool + isfinite guard, use it at the
EvaluationResult construction site, unit-test the helper (bool
exclusion, NaN exclusion, malformed-shape tolerance), and pin the
summary.json mean_reward field end-to-end.
@xdotli
xdotli temporarily deployed to pypi-internal-preview August 9, 2026 17:33 — with GitHub Actions Inactive

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a193d388b9

ℹ️ 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".

for r in results
if isinstance(rw := extract_reward(r), (int, float))
and not isinstance(rw, bool)
and math.isfinite(rw)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject oversized integers before calling isfinite

When resuming a job whose raw result.json has an integer reward too large to convert to a C double (for example, a 1,000-digit JSON integer), this predicate accepts it as an int and math.isfinite(rw) raises OverflowError; _get_completed_tasks deliberately passes such JSON rewards through without validation, so Evaluation.run() now aborts during aggregation rather than producing the summary. Validate or bound integers, or handle the conversion failure before treating the value as scored.

Useful? React with 👍 / 👎.

Comment thread tests/test_job.py
Comment on lines +663 to +667
"""Partial credit must survive the binarized pass/fail view: run()
computes mean_reward over scored rollouts (errors excluded, not
zeroed), the per-task lines carry reward=, error lines don't, and the
job-complete line carries mean_reward=.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Name the guarded change in regression test docstrings

These newly added tests guard the observed regression where fractional rewards disappeared from console and summary output, but this docstring—and the other new regression cases—only restates semantics and never identifies a PR or commit. Add the guarded change identifier to each regression-test docstring so the coverage remains traceable under the repository convention.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

@xdotli
xdotli merged commit 49d5f7e into main Aug 9, 2026
10 checks passed
@xdotli
xdotli deleted the feat/console-mean-reward branch August 9, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant