From 5b483059200d40f1fc3614c5ee9d292a3a9bc01f Mon Sep 17 00:00:00 2001 From: jjcav84 Date: Tue, 6 Oct 2026 22:13:55 -0500 Subject: [PATCH] fix(evals): bound StepEfficiency redundancy by tool calls, not steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StepEfficiency counted redundant *tool calls* but divided by the number of *steps*. A step holding several redundant calls drove the ratio below zero and the documented 0.0-1.0 score went negative (#60), which also pulled the composed TrajectoryScore down. Divide by total_calls instead — the same denominator the failure term uses — so the ratio is bounded by construction. Rename the diagnostic details key to redundant_calls to match what is counted; nothing in the repo consumes the old key. Generated with Devin Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/fi/evals/metrics/agents/metrics.py | 9 ++- tests/test_agent_step_efficiency.py | 103 +++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 tests/test_agent_step_efficiency.py diff --git a/src/fi/evals/metrics/agents/metrics.py b/src/fi/evals/metrics/agents/metrics.py index b52596d0d..7c6f59e70 100644 --- a/src/fi/evals/metrics/agents/metrics.py +++ b/src/fi/evals/metrics/agents/metrics.py @@ -232,7 +232,9 @@ def compute_one(self, inputs: AgentTrajectoryInput) -> Dict[str, Any]: # No baseline - give partial credit if reasonable number of steps step_score = self.expected_step_weight * (1.0 if total_steps <= 10 else 10 / total_steps) - # Detect redundant steps (same tool called with same arguments) + total_calls = sum(len(step.tool_calls) for step in inputs.trajectory) + + # Detect redundant tool calls (same tool called with same arguments) seen_signatures: Set[str] = set() redundant_count = 0 for step in inputs.trajectory: @@ -243,9 +245,9 @@ def compute_one(self, inputs: AgentTrajectoryInput) -> Dict[str, Any]: else: seen_signatures.add(call_sig) - redundancy_ratio = 1.0 - (redundant_count / total_steps) if total_steps > 0 else 1.0 + redundancy_ratio = 1.0 - (redundant_count / total_calls) if total_calls > 0 else 1.0 redundancy_score = redundancy_ratio * self.redundancy_weight - details["redundant_steps"] = redundant_count + details["redundant_calls"] = redundant_count # Count failures failed_calls = sum( @@ -253,7 +255,6 @@ def compute_one(self, inputs: AgentTrajectoryInput) -> Dict[str, Any]: for tc in step.tool_calls if not tc.success ) - total_calls = sum(len(step.tool_calls) for step in inputs.trajectory) failure_ratio = 1.0 - (failed_calls / total_calls) if total_calls > 0 else 1.0 failure_score = failure_ratio * self.failure_weight details["failed_calls"] = failed_calls diff --git a/tests/test_agent_step_efficiency.py b/tests/test_agent_step_efficiency.py new file mode 100644 index 000000000..c6ca62169 --- /dev/null +++ b/tests/test_agent_step_efficiency.py @@ -0,0 +1,103 @@ +"""StepEfficiency score-bounds regression tests (agent-learning-kit#60). + +The redundancy term counted duplicate *tool calls* but divided by the +number of *steps*, so a step holding several redundant calls pushed the +ratio below zero and the documented 0.0-1.0 score went negative. The fix +divides by total_calls, matching the failure term's denominator. +""" + +from fi.evals.metrics.agents.metrics import StepEfficiency, TrajectoryScore +from fi.evals.metrics.agents.types import ( + AgentStep, + AgentTrajectoryInput, + TaskDefinition, + ToolCall, +) + + +def _calls(name: str, args: dict, n: int) -> list: + return [ToolCall(name=name, arguments=args) for _ in range(n)] + + +def _score(metric, trajectory, **kwargs): + return metric.compute_one( + AgentTrajectoryInput( + trajectory=trajectory, + task=TaskDefinition(description="do the thing"), + **kwargs, + ) + ) + + +class TestStepEfficiencyBounds: + def test_multiple_redundant_calls_in_one_step_stays_in_range(self): + """The reported repro: 5 identical calls in a single step.""" + step = AgentStep( + step_number=1, + tool_calls=_calls("search", {"q": "x"}, 5), + is_final=True, + ) + result = _score(StepEfficiency(), [step]) + # 4 of 5 calls redundant -> ratio 0.2 -> 0.4 + 0.3*0.2 + 0.3 + assert result["output"] == 0.76 + assert 0.0 <= result["output"] <= 1.0 + assert result["details"]["redundant_calls"] == 4 + + def test_single_redundant_call_per_step(self): + """One call per step, repeated across steps — pre-existing case.""" + trajectory = [ + AgentStep( + step_number=i, + tool_calls=_calls("search", {"q": "x"}, 1), + is_final=(i == 3), + ) + for i in range(1, 4) + ] + result = _score(StepEfficiency(), trajectory) + # 2 of 3 calls redundant -> ratio 1/3 -> 0.4 + 0.1 + 0.3 + assert result["output"] == 0.8 + + def test_no_redundant_calls_scores_full_redundancy_weight(self): + trajectory = [ + AgentStep( + step_number=1, + tool_calls=[ + ToolCall(name="search", arguments={"q": "a"}), + ToolCall(name="fetch", arguments={"u": "b"}), + ], + is_final=True, + ) + ] + result = _score(StepEfficiency(), trajectory) + assert result["output"] == 1.0 + assert result["details"]["redundant_calls"] == 0 + + def test_worst_case_all_redundant_never_negative(self): + """Every call after the first redundant: score must bottom at ~0 + for the redundancy term, not below.""" + step = AgentStep( + step_number=1, + tool_calls=_calls("search", {"q": "x"}, 20), + is_final=True, + ) + result = _score(StepEfficiency(), [step]) + assert result["output"] >= 0.0 + # 19/20 redundant -> ratio 0.05 -> 0.4 + 0.015 + 0.3 + assert result["output"] == 0.715 + + def test_zero_tool_calls_no_division_error(self): + step = AgentStep(step_number=1, tool_calls=[], is_final=True) + result = _score(StepEfficiency(), [step]) + assert 0.0 <= result["output"] <= 1.0 + assert result["details"]["redundant_calls"] == 0 + + def test_composed_trajectory_score_stays_in_range(self): + """TrajectoryScore weights StepEfficiency at 30% — a negative + efficiency pulled the composite below zero too.""" + step = AgentStep( + step_number=1, + tool_calls=_calls("search", {"q": "x"}, 5), + is_final=True, + ) + result = _score(TrajectoryScore(), [step]) + assert 0.0 <= result["output"] <= 1.0