diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py index d1a6e22f5..6c5a55d67 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py @@ -39,6 +39,7 @@ from gooddata_eval.core.models import ( AgenticAssertionError, AgenticEvalOutcome, + ChatResult, LoopExit, ReasoningStepEvent, ToolCallEvent, @@ -928,6 +929,18 @@ def render_alert_proposal(proposal: dict) -> str: return f"{cta}\n\nAlert proposal:\n{json.dumps(summary, indent=2, sort_keys=True)}" +def render_turn_for_simulated_user(chat_result: ChatResult) -> str: + """Render the whole agent turn the simulated user answers: prose, every other part, and the last proposal. + + A ``clarifyingQuestions`` part carries the options the agent asks the user to pick from, so the + simulated user cannot answer a question whose options it never reads. + """ + chunks = [render_answer_text(chat_result)] + if chat_result.alert_proposals: + chunks.append(render_alert_proposal(chat_result.alert_proposals[-1])) + return "\n\n".join(chunk for chunk in chunks if chunk) + + def run_agentic_alert_skill( host: str, token: str, @@ -1027,11 +1040,7 @@ def _run_once(conv_id: str) -> AlertRunResult: alert_id_to_delete = alert_id exit_reason = LoopExit.SUCCESS break - response_text = (chat_result.text_response or "").strip() - if not response_text and chat_result.alert_proposals: - response_text = render_alert_proposal(chat_result.alert_proposals[-1]) - if not response_text: - response_text = render_answer_text(chat_result) + response_text = render_turn_for_simulated_user(chat_result) # Stop if agent gave a completely empty response (stuck) if not response_text and not chat_result.tool_call_events: exit_reason = LoopExit.AGENT_SILENT diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py index e77832986..464cf592e 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py @@ -378,9 +378,7 @@ def _execute_single_metric_run( metric_result = candidate exit_reason = LoopExit.SUCCESS break - response_text = (chat_result.text_response or "").strip() - if not response_text: - response_text = render_answer_text(chat_result) + response_text = render_answer_text(chat_result) if not response_text and not chat_result.tool_call_events: exit_reason = LoopExit.AGENT_SILENT break diff --git a/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py b/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py index ca3f3d278..dd3d924aa 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py @@ -26,10 +26,10 @@ def render_search_results(part: dict) -> str: def render_unhandled_part(part: dict) -> str: - """Best-effort rendering of a part gd-eval does not model, truncated to a sane size.""" + """Render an unmodelled part, preserving questionnaires and capping other payloads.""" ptype = str(part.get("type") or "unknown") body = json.dumps({k: v for k, v in part.items() if k != "type"}, ensure_ascii=False) - if len(body) > _MAX_UNHANDLED_PART_CHARS: + if ptype != "clarifyingQuestions" and len(body) > _MAX_UNHANDLED_PART_CHARS: body = body[:_MAX_UNHANDLED_PART_CHARS] + "… (truncated)" return f"[{ptype}]\n{body}" diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index b0ff5481d..09cfab387 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -23,6 +23,7 @@ evaluate_agentic_alert_skill, generate_simulated_alert_response, render_alert_proposal, + render_turn_for_simulated_user, run_agentic_alert_skill, ) from gooddata_eval.core.chat.sse_client import ChatError @@ -821,15 +822,67 @@ def test_render_alert_proposal_falls_back_to_default_cta(): assert render_alert_proposal({}).startswith("Should I create this alert?") -def test_run_agentic_alert_skill_answers_proposal_only_confirmation_turn(): - """GDAI-2032 regression: confirmation turn has no text part, only an alertProposal. +_QUESTIONS_PART = { + "type": "clarifyingQuestions", + "questions": [ + { + "text": "Which date?", + "control": { + "type": "singleChoice", + "options": [{"text": "{dataset/date}"}, {"text": "{dataset/created}"}], + }, + }, + ], +} - Without the fallback the simulated user is handed an empty agent message, so the agent - never receives an explicit "yes" and create_metric_alert is never called. - """ + +@pytest.mark.parametrize( + ("turn", "must_contain"), + [ + ({"text_response": "Pick both.", "unhandled_parts": [_QUESTIONS_PART]}, ["Pick both.", "{dataset/date}"]), + ({"text_response": "Here it is.", "alert_proposals": [_PROPOSAL]}, ["Here it is.", "GREATER_THAN"]), + ({"alert_proposals": [_PROPOSAL]}, ["Should I create this alert?", "GREATER_THAN"]), + ({"text_response": "Which metric?"}, ["Which metric?"]), + ], + ids=["text_and_questions", "text_and_proposal", "proposal_only", "text_only"], +) +def test_simulated_user_reads_every_part_of_the_turn(turn, must_contain): + rendered = render_turn_for_simulated_user(ChatResult(**turn)) + for fragment in must_contain: + assert fragment in rendered + + +def test_simulated_user_reads_nothing_of_a_silent_turn(): + assert render_turn_for_simulated_user(ChatResult()) == "" + + +def test_run_agentic_alert_skill_passes_question_options_to_simulated_user() -> None: + client = MagicMock() + client.send_message.return_value = ChatResult(text_response="Pick one.", unhandled_parts=[_QUESTIONS_PART]) + with _patched(client, simulated_reply="Use the created date.") as mock_sim: + run_agentic_alert_skill( + host="http://host", + token="tok", + workspace_id="ws1", + question="Create an alert", + expected_output={"operator": "GREATER_THAN", "threshold": 500}, + k=1, + max_iterations=2, + initial_conversation_id="conv-1", + ) + + agent_message = mock_sim.call_args.args[0] + assert "Pick one." in agent_message + assert "{dataset/date}" in agent_message + assert "{dataset/created}" in agent_message + + +@pytest.mark.parametrize("text_response", [None, "Here it is."]) +def test_run_agentic_alert_skill_answers_proposal_confirmation_turn(text_response: str | None) -> None: + """Confirmation keeps the proposal fields visible alongside any prose.""" proposal_turn = ChatResult.model_validate( { - "text_response": None, + "text_response": text_response, "alertProposals": [_PROPOSAL], "tool_call_events": [ {"functionName": "prepare_metric_alert_proposal", "functionArguments": "{}", "result": None} @@ -866,6 +919,8 @@ def test_run_agentic_alert_skill_answers_proposal_only_confirmation_turn(): ) agent_message = mock_sim.call_args.args[0] + if text_response: + assert text_response in agent_message assert "Should I create this alert?" in agent_message assert "admin@gooddata.com" in agent_message assert summary.best.eval.alert_created is True diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index 65f23a39f..a86a2e9c3 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -331,6 +331,42 @@ def test_run_agentic_metric_skill_creates_conversation(monkeypatch): mock_client.close.assert_called_once() +def test_run_agentic_metric_skill_passes_question_options_to_simulated_user() -> None: + client = _client() + client.send_message.return_value = ChatResult( + text_response="Pick one.", + unhandled_parts=[ + { + "type": "clarifyingQuestions", + "questions": [ + { + "text": "Which metric?", + "control": { + "type": "singleChoice", + "options": [{"text": "{metric/revenue}"}, {"text": "{metric/profit}"}], + }, + } + ], + } + ], + ) + with _patched(client, simulated_reply="Use profit.") as (mock_sim, _): + run_agentic_metric_skill( + host="http://host", + token="tok", + workspace_id="ws1", + question="Create a metric", + expected_output={"maql": "SELECT {metric/profit}"}, + k=1, + max_iterations=2, + ) + + agent_message = mock_sim.call_args.args[0] + assert "Pick one." in agent_message + assert "{metric/revenue}" in agent_message + assert "{metric/profit}" in agent_message + + def test_run_agentic_metric_skill_closes_client_on_no_result(): mock_client = _client() mock_client.send_message.return_value = ChatResult.model_validate( diff --git a/packages/gooddata-eval/tests/test_chat_render.py b/packages/gooddata-eval/tests/test_chat_render.py index 29904158f..17834e2e7 100644 --- a/packages/gooddata-eval/tests/test_chat_render.py +++ b/packages/gooddata-eval/tests/test_chat_render.py @@ -155,6 +155,39 @@ def test_a_huge_unmodelled_part_is_truncated(): assert len(rendered) < 3_000 +def test_long_questionnaire_keeps_every_offered_option() -> None: + questions: list[dict] = [] + for question_index in range(4): + options = [ + {"text": "{dashboard/00000000-0000-4000-8000-" + f"{question_index * 6 + option_index:012d}" + "}"} + for option_index in range(6) + ] + questions.append( + { + "text": ( + f"Which dashboard should receive the alert for metric {question_index}, considering the department, " + "reporting period, audience, and the business context in my original request? " + "For example: choose the closest match." + ), + "control": {"type": "singleChoice", "options": options}, + } + ) + result = parse_sse_lines( + _multipart_lines( + {"type": "text", "text": "Pick one for each metric."}, + {"type": "clarifyingQuestions", "questions": questions}, + ) + ) + + rendered = render_answer_text(result) + + assert "Pick one for each metric." in rendered + for question in questions: + assert question["text"] in rendered + for option in question["control"]["options"]: + assert option["text"] in rendered + + @pytest.mark.parametrize("ptype", ["dashboard", "dashboardPatch", "report", "kda", "whatIf", "clarifyingQuestions"]) def test_no_known_part_type_is_silently_dropped(ptype): result = parse_sse_lines(_multipart_lines({"type": ptype, "payload": {"a": 1}}))