Skip to content

Commit a047206

Browse files
Tomkessclaude
andcommitted
fix(gooddata-eval): a rejected draft attempt is not a refusal
`_extract_tool_result` takes only successful calls, which is right for the normal path -- a failed call there is a retry the agent recovered from. But a refusal case was scored on `tool_result is None` and `dashboard_part is None`, and an agent whose draft the tool REJECTED leaves both empty. So a run that tried to build and was stopped by a guardrail read as a clean refusal, which credits the model for the guardrail. `dashboard_not_drafted` now also requires that the producing tool was never called at all. The flag is computed in the run loop over every tool-call event, not from the extracted result, so it survives the filtering that hides the failed call. Nothing outside the refusal shape reads it. Found in review by CodeRabbit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 544aefa commit a047206

2 files changed

Lines changed: 63 additions & 3 deletions

File tree

‎packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,10 @@ class DashboardEvaluation:
769769
titles_matched: bool = True
770770
forbidden_absent: bool = True
771771
answer_matched: bool = True
772+
# Whether the producing tool was called AT ALL, successful or not. Only a refusal case
773+
# reads it: everywhere else a failed call is a retry the agent recovered from, which is
774+
# why `_extract_tool_result` skips it.
775+
producing_tool_called: bool = False
772776
failures: list[str] = field(default_factory=list)
773777
notes: list[str] = field(default_factory=list)
774778

@@ -781,10 +785,17 @@ def strict_checks(self) -> dict[str, bool]:
781785
if self.applies.refusal:
782786
# Named for what has to be true, not negated at read time: `dashboard_not_drafted`
783787
# false means a dashboard came back from a case that asked for none, which is the
784-
# failure. Both halves are required -- a successful tool call with no part, or a
785-
# part with no successful call, are each a dashboard the user would be shown.
788+
# failure. A successful tool call with no part, or a part with no successful call,
789+
# are each a dashboard the user would be shown.
790+
#
791+
# The third term is the one that is easy to miss: an agent that CALLED the tool and
792+
# had the call rejected leaves no result and no part, so the first two would read it
793+
# as a clean refusal. It is not one -- the agent tried to build and the tool stopped
794+
# it, and scoring that as correct credits the model for a guardrail.
786795
checks = {
787-
"dashboard_not_drafted": not self.drafted and not self.part_present,
796+
"dashboard_not_drafted": (
797+
not self.drafted and not self.part_present and not self.producing_tool_called
798+
),
788799
"dashboard_skill_activated": self.skill_activated,
789800
}
790801
if self.applies.answer:
@@ -870,6 +881,7 @@ def evaluate_dashboard_response(
870881
skill_activated: bool,
871882
patch_part: dict | None = None,
872883
answer_text: str = "",
884+
producing_tool_called: bool = False,
873885
) -> DashboardEvaluation:
874886
"""Score one dashboard response against its expectation.
875887
@@ -909,6 +921,7 @@ def evaluate_dashboard_response(
909921
new_visualizations_met=True,
910922
applies=applies,
911923
skill_activated=skill_activated,
924+
producing_tool_called=producing_tool_called,
912925
answer_matched=not answer_failures,
913926
failures=[
914927
*(
@@ -921,6 +934,11 @@ def evaluate_dashboard_response(
921934
if dashboard_part is not None
922935
else []
923936
),
937+
*(
938+
[f"the case expects no dashboard, but the agent called {tool} (the call did not succeed)"]
939+
if producing_tool_called and not drafted
940+
else []
941+
),
924942
*answer_failures,
925943
],
926944
)
@@ -1198,6 +1216,8 @@ def _execute_single_dashboard_run(
11981216
_skill_activated(all_tool_call_events, _required_skill(expected_output)),
11991217
patch_part=patch_part,
12001218
answer_text=answer_text,
1219+
# Any call, not just a successful one -- see DashboardEvaluation.
1220+
producing_tool_called=any(tc.function_name == tool for tc in all_tool_call_events),
12011221
),
12021222
tool_result=tool_result,
12031223
dashboard_part=dashboard_part,

‎packages/gooddata-eval/tests/test_agentic_dashboard_skill.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,46 @@ def test_falling_over_silently_is_not_a_refusal(self):
14851485
assert not ev.answer_matched
14861486
assert not ev.strict_pass
14871487

1488+
def test_a_rejected_draft_attempt_is_not_a_refusal(self):
1489+
"""The gap the first two checks leave: `_extract_tool_result` takes only SUCCESSFUL
1490+
calls, so an agent whose draft the tool rejected leaves no result and no part and
1491+
would read as a clean refusal. It is not one -- it tried to build and a guardrail
1492+
stopped it, and scoring that as correct credits the model for the guardrail."""
1493+
ev = evaluate_dashboard_response(
1494+
None,
1495+
None,
1496+
self._NO_DATA,
1497+
skill_activated=True,
1498+
answer_text="There is no fraud data here.",
1499+
producing_tool_called=True,
1500+
)
1501+
assert not ev.strict_checks["dashboard_not_drafted"]
1502+
assert not ev.strict_pass
1503+
assert any("did not succeed" in f for f in ev.failures)
1504+
1505+
def test_a_failed_draft_attempt_is_read_off_the_turn_not_the_result(self):
1506+
"""End to end, because the flag is computed in the run loop rather than passed in by
1507+
a fixture: the tool call is present on the turn, its result is an error, so nothing
1508+
reaches the scorer except the fact that the call happened."""
1509+
client = MagicMock()
1510+
client.send_message.return_value = _chat_result(
1511+
tool_calls=[_tool_call("draft_dashboard", {"status": "error", "message": "no such metric"})],
1512+
text="There is no fraud data in this workspace, so I cannot build that.",
1513+
)
1514+
summary = _run_with(client, self._NO_DATA)
1515+
assert summary.best.evaluation.producing_tool_called
1516+
assert not summary.best.evaluation.strict_pass
1517+
1518+
def test_an_answer_alone_still_passes_when_nothing_was_called(self):
1519+
"""The control for the two above: the flag must not fail a genuine refusal."""
1520+
client = MagicMock()
1521+
client.send_message.return_value = _chat_result(
1522+
text="There is no fraud data in this workspace, so I cannot build that."
1523+
)
1524+
summary = _run_with(client, self._NO_DATA)
1525+
assert not summary.best.evaluation.producing_tool_called
1526+
assert summary.best.evaluation.strict_pass
1527+
14881528
def test_a_refusal_case_without_an_answer_assertion_is_rejected_up_front(self):
14891529
client = MagicMock()
14901530
expected = {"type": "dashboard", "expects_dashboard": False, "visualizations": []}

0 commit comments

Comments
 (0)