Skip to content

Commit 0cff5d6

Browse files
committed
fix(eval): stop guessing whether the agent asked a question
1 parent 5b04a0a commit 0cff5d6

4 files changed

Lines changed: 193 additions & 27 deletions

File tree

packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
_REF_PATTERN = re.compile(r"\$ref:([\w_]+)\.([\w_]+)")
2727

28+
_DEFAULT_MAX_CLARIFICATION_TURNS = 7
29+
2830

2931
class TurnDefinition(BaseModel):
3032
"""Definition of a single turn in a multi-turn conversation evaluation."""
@@ -192,13 +194,6 @@ def _check_output_correct(turn: TurnDefinition, chat_result: ChatResult) -> bool
192194
return None
193195

194196

195-
def _is_asking_clarification(text: str) -> bool:
196-
if not text:
197-
return False
198-
t = text.lower()
199-
return "?" in t or "could you" in t or "please" in t or "clarif" in t
200-
201-
202197
def _get_sim_user_response(agent_message: str, turn: TurnDefinition, expected_output: dict | None) -> str:
203198
"""Generate a simulated user reply to an agent clarification question."""
204199
otype = turn.expected_output_type
@@ -277,7 +272,7 @@ def run_agentic_conversation(
277272
token: str,
278273
workspace_id: str,
279274
fixture: ConversationFixture,
280-
max_clarification_turns: int = 20,
275+
max_clarification_turns: int = _DEFAULT_MAX_CLARIFICATION_TURNS,
281276
initial_conversation_id: str | None = None,
282277
reasoning_effort: ReasoningEffort | None = None,
283278
) -> ConversationResult:
@@ -307,8 +302,22 @@ def run_agentic_conversation(
307302
owns_conversation = True
308303

309304
for turn in fixture.turns:
310-
# Resolve $ref placeholders using outputs captured from prior turns.
311-
resolved_expected = _resolve_refs(turn.expected_output, turn_outputs)
305+
try:
306+
resolved_expected = _resolve_refs(turn.expected_output, turn_outputs)
307+
except ValueError as exc:
308+
print(f"[SKIP] turn '{turn.turn_id}': {exc}")
309+
turn_results.append(
310+
TurnResult(
311+
turn_id=turn.turn_id,
312+
expected_skill=turn.expected_skill,
313+
skill_routing=False,
314+
output_present=False,
315+
no_error=False,
316+
activated_skills=[],
317+
output_correct=False,
318+
)
319+
)
320+
continue
312321
resolved_turn = turn.model_copy(update={"expected_output": resolved_expected})
313322

314323
clarification_turns = 0
@@ -327,13 +336,13 @@ def run_agentic_conversation(
327336
response_text = (chat_result.text_response or "").strip()
328337
if not response_text and chat_result.alert_proposals:
329338
response_text = render_alert_proposal(chat_result.alert_proposals[-1])
330-
asking = _is_asking_clarification(response_text) or bool(chat_result.alert_proposals)
331-
if asking and clarification_turns < max_clarification_turns:
332-
clarification_turns += 1
333-
total_clarification_turns += 1
334-
current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)
335-
else:
339+
if not response_text and not chat_result.tool_call_events:
340+
break
341+
if clarification_turns >= max_clarification_turns:
336342
break
343+
clarification_turns += 1
344+
total_clarification_turns += 1
345+
current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)
337346

338347
activated = _activated_skills(all_tool_calls)
339348
skill_routing = turn.expected_skill in activated if activated else False
@@ -397,7 +406,7 @@ def evaluate_agentic_conversation(
397406
token: str,
398407
workspace_id: str,
399408
fixture: ConversationFixture,
400-
max_clarification_turns: int = 20,
409+
max_clarification_turns: int = _DEFAULT_MAX_CLARIFICATION_TURNS,
401410
initial_conversation_id: str | None = None,
402411
langfuse: object | None = None,
403412
dataset_item_id: str = "",

packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,6 @@ def _delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> None:
166166
print(f"[CLEANUP] Failed to delete metric {metric_id}: {exc}")
167167

168168

169-
def _is_asking_clarification(text: str) -> bool:
170-
if not text:
171-
return False
172-
t = text.lower()
173-
return "?" in t or "could you" in t or "please provide" in t or "clarif" in t
174-
175-
176169
def _execute_single_metric_run(
177170
client: ChatClient,
178171
sdk: GoodDataSdk,
@@ -204,9 +197,14 @@ def _execute_single_metric_run(
204197
metric_id_to_delete = candidate.get("metric_id")
205198
break
206199
response_text = (chat_result.text_response or "").strip()
207-
if _is_asking_clarification(response_text):
200+
if not response_text and not chat_result.tool_call_events:
201+
break
202+
if _iteration >= max_iterations - 1:
203+
break
204+
try:
208205
current_question = generate_simulated_response(response_text, primary_expected)
209-
else:
206+
except Exception as exc:
207+
print(f"[SIM-USER] Simulated reply failed for conversation {conversation_id}: {exc}")
210208
break
211209

212210
actual_maql = (metric_result or {}).get("maql", "")

packages/gooddata-eval/tests/test_agentic_conversation.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,118 @@ def test_run_agentic_conversation_treats_alert_proposal_as_a_clarification():
352352
assert "Should I create this alert?" in mock_sim.call_args.args[0]
353353
assert result.turn_results[0].clarification_turns_used == 1
354354
assert result.turn_results[0].skill_success is True
355+
356+
357+
def _viz_turn_result(text=None, viz=None, tool_calls=()):
358+
r = MagicMock()
359+
r.text_response = text
360+
r.created_visualizations = viz
361+
r.tool_call_events = list(tool_calls)
362+
r.alert_proposals = []
363+
return r
364+
365+
366+
def test_run_agentic_conversation_replies_to_a_statement_without_a_question_mark():
367+
"""QA-28982 regression: gpt-5.2 answered "I need to confirm ... Next I'll:" -- no question
368+
mark, so the old substring heuristic ended the turn and no metric was ever created."""
369+
mock_client = MagicMock()
370+
mock_client.create_conversation.return_value = "conv-1"
371+
stalling_turn = _viz_turn_result(
372+
text="I can create that, but first I need to confirm which Net Sales calculation to use. Next I'll: ...",
373+
tool_calls=[_skills_tc("metric")],
374+
)
375+
mock_client.send_message.side_effect = [
376+
stalling_turn,
377+
_metric_turn_result([_skills_tc("metric"), _create_metric_tc("m1")]),
378+
]
379+
380+
with (
381+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
382+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
383+
patch(
384+
"gooddata_eval.core.agentic.conversation._get_sim_user_response",
385+
return_value="Go ahead with Net Sales.",
386+
) as mock_sim,
387+
):
388+
result = run_agentic_conversation(
389+
host="http://host/api/v1/actions/workspaces/ws1/ai",
390+
token="tok",
391+
workspace_id="ws1",
392+
fixture=_two_metric_turn_fixture().model_copy(update={"turns": _two_metric_turn_fixture().turns[:1]}),
393+
)
394+
395+
mock_sim.assert_called_once()
396+
assert result.turn_results[0].clarification_turns_used == 1
397+
assert result.turn_results[0].skill_success is True
398+
399+
400+
def test_run_agentic_conversation_stops_when_the_agent_says_nothing():
401+
"""An agent that returns neither text nor tool calls is stuck -- no point replying to it."""
402+
mock_client = MagicMock()
403+
mock_client.create_conversation.return_value = "conv-1"
404+
mock_client.send_message.return_value = _viz_turn_result(text=None)
405+
406+
with (
407+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
408+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
409+
patch("gooddata_eval.core.agentic.conversation._get_sim_user_response") as mock_sim,
410+
):
411+
result = run_agentic_conversation(
412+
host="http://host/api/v1/actions/workspaces/ws1/ai",
413+
token="tok",
414+
workspace_id="ws1",
415+
fixture=_two_metric_turn_fixture().model_copy(update={"turns": _two_metric_turn_fixture().turns[:1]}),
416+
)
417+
418+
mock_sim.assert_not_called()
419+
assert mock_client.send_message.call_count == 1
420+
assert result.turn_results[0].skill_success is False
421+
422+
423+
def test_run_agentic_conversation_records_a_failed_turn_when_a_ref_cannot_be_resolved():
424+
"""QA-28982 regression: turn 1 producing no metric used to raise ValueError out of the whole
425+
run, hiding which turn broke and skipping every later turn."""
426+
mock_client = MagicMock()
427+
mock_client.create_conversation.return_value = "conv-1"
428+
mock_client.send_message.side_effect = [
429+
_viz_turn_result(text="Which Net Sales metric?", tool_calls=[_skills_tc("metric")]),
430+
_viz_turn_result(text="Working on it.", tool_calls=[_skills_tc("metric")]),
431+
_metric_turn_result([_skills_tc("metric"), _create_metric_tc("m2")]),
432+
]
433+
fixture = ConversationFixture(
434+
id="test-ref",
435+
expected_skills=["metric"],
436+
turns=[
437+
TurnDefinition(
438+
turn_id="t1", message="Create shared", expected_skill="metric", expected_output_type="metric"
439+
),
440+
TurnDefinition(
441+
turn_id="t2",
442+
message="Chart it",
443+
expected_skill="visualization",
444+
expected_output={"metrics": ["metric/$ref:t1.metric_id"]},
445+
),
446+
TurnDefinition(
447+
turn_id="t3", message="Create another", expected_skill="metric", expected_output_type="metric"
448+
),
449+
],
450+
)
451+
452+
with (
453+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
454+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
455+
patch("gooddata_eval.core.agentic.conversation._get_sim_user_response", return_value="Go ahead."),
456+
):
457+
result = run_agentic_conversation(
458+
host="http://host/api/v1/actions/workspaces/ws1/ai",
459+
token="tok",
460+
workspace_id="ws1",
461+
fixture=fixture,
462+
max_clarification_turns=1,
463+
)
464+
465+
assert [t.turn_id for t in result.turn_results] == ["t1", "t2", "t3"]
466+
assert result.turn_results[0].skill_success is False
467+
assert result.turn_results[1].no_error is False
468+
assert result.turn_results[2].skill_success is True
469+
assert result.conversation_success is False

packages/gooddata-eval/tests/test_agentic_metric_skill.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,13 @@ def test_run_agentic_metric_skill_closes_client_on_no_result():
8383
"reasoningStepCount": 1,
8484
}
8585
)
86-
with patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client):
86+
with (
87+
patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client),
88+
patch(
89+
"gooddata_eval.core.agentic.metric_skill.generate_simulated_response",
90+
return_value="Go ahead and create it.",
91+
) as mock_sim,
92+
):
8793
summary = run_agentic_metric_skill(
8894
host="http://host/api/v1/actions/workspaces/ws1/ai",
8995
token="tok",
@@ -96,6 +102,7 @@ def test_run_agentic_metric_skill_closes_client_on_no_result():
96102
mock_client.close.assert_called_once()
97103
assert summary.pass_at_k is False
98104
assert summary.best.metric_created is False
105+
mock_sim.assert_called_once()
99106

100107

101108
def test_run_agentic_metric_skill_uses_initial_conversation_for_run_0():
@@ -224,3 +231,40 @@ def test_run_agentic_metric_skill_deletes_metric_even_when_teardown_fails():
224231
)
225232

226233
mock_sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "foo_metric")
234+
235+
236+
@pytest.mark.parametrize(
237+
"exc",
238+
[
239+
OSError("OPENAI_API_KEY environment variable is not set"),
240+
RuntimeError("openai package is required for generate_simulated_response"),
241+
],
242+
)
243+
def test_run_agentic_metric_skill_fails_the_run_when_the_simulated_reply_cannot_be_generated(exc):
244+
mock_client = MagicMock()
245+
mock_client.create_conversation.return_value = "conv-1"
246+
mock_client.send_message.return_value = ChatResult.model_validate(
247+
{
248+
"textResponse": "Which brand field should I count?",
249+
"toolCallEvents": [],
250+
"reasoningStepCount": 1,
251+
}
252+
)
253+
with (
254+
patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client),
255+
patch("gooddata_eval.core.agentic.metric_skill.generate_simulated_response", side_effect=exc),
256+
):
257+
summary = run_agentic_metric_skill(
258+
host="http://host/api/v1/actions/workspaces/ws1/ai",
259+
token="tok",
260+
workspace_id="ws1",
261+
question="Create metric foo",
262+
expected_output={"maql": "SELECT {metric/foo}"},
263+
k=1,
264+
max_iterations=3,
265+
)
266+
267+
assert summary.pass_at_k is False
268+
assert summary.best.metric_created is False
269+
assert summary.best.total_turns == 1.0
270+
mock_client.close.assert_called_once()

0 commit comments

Comments
 (0)