fix(evals): StepEfficiency negative score on redundant calls within a step - #62
Conversation
… step Redundancy was counted per tool call but normalized by step count, so a single step with multiple redundant calls could push redundancy_ratio below 0 and produce a negative score, out of the documented 0 to 1 range. Divide by total tool calls instead, matching the failure calculation in the same method. The per-call denominator keeps the ratio bounded by construction, so no clamp is needed. Adds a test covering multiple redundant calls within a single step. Fixes future-agi#60
|
@abhiramArise this implements option 1 as discussed in #60 — redundancy now divides by total_calls, no clamp. Added a test for the multiple-calls-per-step case alongside the existing single-call test. Ready for review whenever you get a chance. |
|
Hi @abhiramArise, gentle nudge whenever you have a moment. Could you approve the workflow run so CI can execute on this? Happy to address anything that comes up in review. |
abhiramArise
left a comment
abhiramArise
left a comment
There was a problem hiding this comment.
Pulled both branches and verified locally. The fix is correct: redundant_count can never reach total_calls (the first occurrence of any signature is never flagged redundant), so the ratio is bounded to (0, 1] by construction — the "no clamp needed" reasoning holds. Ran the full suite (29 passed) and specifically test_redundant_calls_within_single_step, both green. LGTM.
Thanks for the careful review and for verifying it locally. Glad the bounded-ratio reasoning held up. Ready to merge whenever CI clears on your side. |
2 participants
Fixes #60
What does this PR do?
Fixes StepEfficiency returning a negative score when a single trajectory step contains more than one redundant tool call.
Why?
Redundancy was counted per tool call (the inner loop runs over step.tool_calls) but the ratio divided that count by total_steps, the number of steps. When a step makes several redundant calls, redundant_count exceeds total_steps, so redundancy_ratio goes below 0 and the final score drops out of the documented 0 to 1 range. On the reproduction in #60 (one step, five identical calls) the metric returned -0.2.
What changed?
The redundancy ratio now divides by total_calls instead of total_steps, matching the failure calculation in the same method. total_calls is moved up so it is defined once before both uses. As discussed in #60, the per-call denominator keeps the ratio bounded by construction, so no clamp is added.
Tests
Adds test_redundant_calls_within_single_step, covering multiple redundant calls within a single step and asserting the score stays within 0 to 1. The existing single-call-per-step test is unchanged. Full agent suite passes locally (29 passed).