Skip to content

Commit b6d35c5

Browse files
Tomkessclaude
andcommitted
fix(gooddata-eval): record a dashboard chat fault instead of letting it escape
ChatClient.send_message can raise ChatError, and the dashboard loop was the only one that let it through. Escaping run_agentic_dashboard_skill discarded every K-run already completed along with this one's exit_reason, so the item surfaced as a bare error with no per-run diagnosis -- exactly what the shared LoopExit contract exists to prevent. metric_skill.py and conversation.py already record it; this matches them. A bare catch would have discarded ChatError.partial_result. What the stream delivered before it died is still the turn's work, so its reasoning, tool calls and step count are merged -- shifted and indexed the way a completed turn is, since left raw a late turn's call_ts restarts near zero and timeline_detail would report it as overlapping the first. DashboardRunResult's comment claimed CHAT_ERROR was unreachable here because the loop had no try/except. That had it backwards: the missing try/except was the bug, not a reason the fault could not occur. SIMULATED_USER_FAILED really is unreachable -- the follow-up is built by build_simulated_reply with no judge call to fail -- and the comment now says so. Three tests, each verified to fail without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 9dd2b28 commit b6d35c5

2 files changed

Lines changed: 80 additions & 4 deletions

File tree

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

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
utc_now,
2929
)
3030
from gooddata_eval.core.chat.render import render_answer_text
31-
from gooddata_eval.core.chat.sse_client import ChatClient
31+
from gooddata_eval.core.chat.sse_client import ChatClient, ChatError
3232
from gooddata_eval.core.config import ReasoningEffort
3333
from gooddata_eval.core.models import (
3434
AgenticAssertionError,
@@ -667,8 +667,9 @@ class DashboardRunResult:
667667
timings: PhaseTimings = field(default_factory=PhaseTimings)
668668
# Why the loop stopped -- see LoopExit. A dashboard not produced alone cannot tell a
669669
# refusal from a run that hit max_iterations while still on track (matches
670-
# kda_skill.py/alert_skill.py). No CHAT_ERROR/SIMULATED_USER_FAILED member here: this
671-
# loop has neither a send_message try/except nor a simulated-user call to fail.
670+
# kda_skill.py/alert_skill.py). CHAT_ERROR is reachable; SIMULATED_USER_FAILED is not --
671+
# this loop's follow-up is built from the expectation by build_simulated_reply, with no
672+
# judge call that could fail.
672673
exit_reason: LoopExit = LoopExit.BUDGET_EXHAUSTED
673674

674675

@@ -879,7 +880,33 @@ def _execute_single_dashboard_run(
879880
for iteration in range(max_iterations):
880881
turns += 1
881882
agent_started = time.monotonic()
882-
chat_result = client.send_message(conversation_id, current_question)
883+
try:
884+
chat_result = client.send_message(conversation_id, current_question)
885+
except ChatError as exc:
886+
# Recorded rather than left to escape: this runs inside one of K runs, so an
887+
# uncaught ChatError discarded every run already completed along with this one's
888+
# exit_reason, and the item surfaced as a bare error with no per-run diagnosis.
889+
# Matches metric_skill.py and conversation.py, which record it the same way.
890+
timings.agent_s += time.monotonic() - agent_started
891+
print(f"[CHAT] send_message failed for conversation {conversation_id}: {exc}")
892+
partial = exc.partial_result
893+
if partial is not None:
894+
# Shifted and indexed exactly as a completed turn is, before anything reads
895+
# the events: left raw, a late turn's call_ts restarts near zero and
896+
# timeline_detail reports it as overlapping the first one.
897+
reasoning_steps.extend(partial.reasoning_steps or [])
898+
response_id = partial.response_id or response_id
899+
turn_offset, tool_index_offset, reasoning_index_offset = shift_and_index_events(
900+
partial,
901+
turn_offset=turn_offset,
902+
tool_index_offset=tool_index_offset,
903+
reasoning_index_offset=reasoning_index_offset,
904+
)
905+
all_tool_call_events.extend(partial.tool_call_events or [])
906+
all_reasoning_step_events.extend(partial.reasoning_step_events or [])
907+
steps += partial.reasoning_step_count
908+
exit_reason = LoopExit.CHAT_ERROR
909+
break
883910
agent_elapsed = time.monotonic() - agent_started
884911
timings.agent_s += agent_elapsed
885912
reasoning_steps.extend(chat_result.reasoning_steps or [])

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
evaluate_dashboard_response,
1616
run_agentic_dashboard_skill,
1717
)
18+
from gooddata_eval.core.chat.sse_client import ChatError
1819
from gooddata_eval.core.models import ChatResult, LoopExit, ToolCallEvent
1920

2021
# Ids and titles are the ones the eval layout seeds; shapes are trimmed from real runs of
@@ -1104,6 +1105,54 @@ def test_a_caller_supplied_conversation_is_not_deleted(self):
11041105
client.create_conversation.assert_not_called()
11051106
client.delete_conversation.assert_not_called()
11061107

1108+
def test_a_chat_error_is_recorded_rather_than_left_to_escape(self):
1109+
"""It used to escape run_agentic_dashboard_skill, taking every completed K-run with
1110+
it: no DashboardRunResult, no exit_reason, just a bare error on the item. Recorded
1111+
the way metric_skill.py and conversation.py record it, so an infrastructure fault
1112+
stays distinguishable from the agent failing to draft."""
1113+
client = MagicMock()
1114+
client.send_message.side_effect = ChatError("gen-ai fell over")
1115+
1116+
summary = _run_with(client, _DC05_EXPECTED)
1117+
1118+
assert summary.run_results[0].exit_reason is LoopExit.CHAT_ERROR
1119+
assert summary.run_results[0].evaluation.strict_pass is False
1120+
assert summary.pass_at_k is False
1121+
1122+
def test_a_chat_error_keeps_the_partial_turn_s_work(self):
1123+
"""A bare catch would discard ChatError.partial_result. What the stream delivered
1124+
before it died is still this turn's work -- the steps were taken and the tools ran."""
1125+
partial = _chat_result(
1126+
tool_calls=[{"functionName": "search_objects", "functionArguments": "{}", "callTs": 1, "durationMs": 10}],
1127+
text="Here is the draft.",
1128+
)
1129+
partial.reasoning_steps = ["looked for the charts"]
1130+
partial.reasoning_step_count = 1
1131+
client = MagicMock()
1132+
client.send_message.side_effect = ChatError("stream died mid-answer", partial_result=partial)
1133+
1134+
summary = _run_with(client, _DC05_EXPECTED)
1135+
run = summary.run_results[0]
1136+
1137+
assert run.exit_reason is LoopExit.CHAT_ERROR
1138+
assert run.reasoning_steps == ["looked for the charts"]
1139+
assert run.total_steps == 1
1140+
assert [tc.function_name for tc in run.tool_call_events] == ["search_objects"]
1141+
1142+
def test_a_chat_error_on_a_later_turn_keeps_the_earlier_ones(self):
1143+
"""The whole point of recording it: turns already completed survive the fault."""
1144+
client = MagicMock()
1145+
client.send_message.side_effect = [
1146+
_chat_result(text="Which charts did you have in mind?"),
1147+
ChatError("gen-ai fell over"),
1148+
]
1149+
1150+
summary = _run_with(client, _DC05_EXPECTED)
1151+
run = summary.run_results[0]
1152+
1153+
assert run.exit_reason is LoopExit.CHAT_ERROR
1154+
assert run.total_turns == 2
1155+
11071156

11081157
class TestEvaluateEntryPoint:
11091158
def test_a_failing_run_raises_with_the_failures_attached(self):

0 commit comments

Comments
 (0)