Skip to content

fix: guard _classify_completed_outcomes against non-dict rewards - #1054

Open
VaisakhiMishra wants to merge 3 commits into
benchflow-ai:mainfrom
VaisakhiMishra:fix/evaluation-malformed-rewards
Open

fix: guard _classify_completed_outcomes against non-dict rewards#1054
VaisakhiMishra wants to merge 3 commits into
benchflow-ai:mainfrom
VaisakhiMishra:fix/evaluation-malformed-rewards

Conversation

@VaisakhiMishra

@VaisakhiMishra VaisakhiMishra commented Aug 25, 2026

Copy link
Copy Markdown

Previously, a truthy non-dict value in the 'rewards' field of result.json (e.g. bare float 1.0, int, bool, list) caused an uncaught AttributeError in _classify_completed_outcomes, crashing the entire evaluation run before any new tasks could execute.


How to reproduce the crash (pre-fix)

# 1. Create a jobs directory with a result.json whose rewards is a bare float
mkdir -p /tmp/bf_repro/my-job/my-task__abc123
cat > /tmp/bf_repro/my-job/my-task__abc123/result.json <<'EOF'
{
  "task_name": "my-task",
  "rollout_name": "my-task__abc123",
  "rewards": 1.0,
  "agent": "oracle",
  "model": null,
  "n_tool_calls": 5
}
EOF

# 2. Resume — crashes before any tasks run
bench eval run --tasks-dir tasks/my-task --jobs-dir /tmp/bf_repro
# AttributeError: 'float' object has no attribute 'get'

Can we make this bug fix as part of https://github.com/benchflow-ai/FrontierPhysics/issues/125 or should I open a separate issue to track this fix?

Root cause:

    rewards = r.get('rewards') if isinstance(r, dict) else None
    reward = rewards.get('reward') if rewards else None

only guards against falsy values; a truthy non-dict (e.g. 1.0) passes the if rewards check and then fails on .get().

Fixes:

  1. Import extract_reward from benchflow._utils.scoring — it already handles None, dict, and non-dict values safely.
  2. Replace the two-liner in _classify_completed_outcomes with:
reward = extract_reward(r) 
if isinstance(r, dict) else None
  1. Add a logger.warning in _get_completed_tasks when rewards is a non-dict truthy value, so operators can identify malformed result files without a crash. The fix is backwards-compatible: valid dict-shaped rewards are unchanged; falsy non-dict values were already silently treated as errored and still are (with a warning now); truthy non-dict values no longer crash.
  1. Minimal fix for uvx ruff check . failure for src/benchflow/contracts/user.py

Open in Devin Review

Previously, a truthy non-dict value in the 'rewards' field of result.json
(e.g. bare float 1.0, int, bool, list) caused an uncaught AttributeError
in _classify_completed_outcomes, crashing the entire evaluation run before
any new tasks could execute.

Root cause: the two-step pattern
    rewards = r.get('rewards') if isinstance(r, dict) else None
    reward = rewards.get('reward') if rewards else None
only guards against falsy values; a truthy non-dict (e.g. 1.0) passes the
'if rewards' check and then blows up on .get().

Fixes:
1. Import extract_reward from benchflow._utils.scoring — it already
   handles None, dict, and non-dict values safely.
2. Replace the two-liner in _classify_completed_outcomes with:
       reward = extract_reward(r) if isinstance(r, dict) else None
3. Add a logger.warning in _get_completed_tasks when rewards is a
   non-dict truthy value, so operators can identify malformed result
   files without a crash.

The fix is backwards-compatible: valid dict-shaped rewards are unchanged;
falsy non-dict values were already silently treated as errored and still
are (with a warning now); truthy non-dict values no longer crash.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/benchflow/evaluation.py Outdated
@VaisakhiMishra

Copy link
Copy Markdown
Author

/devin review

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