From 5663b01e5d88b9049b25efe8b384187ace13a5a5 Mon Sep 17 00:00:00 2001 From: Jan Tychtl Date: Fri, 6 Nov 2026 16:45:01 +0100 Subject: [PATCH] fix(gooddata-eval): show the simulated user every part of a turn Alert and metric simulations need the options returned beside the agent's prose. Passing only text hides those options and can leave the conversation repeating questions until it reaches the turn limit. Render prose and the other response parts for both loops, plus the last alert proposal for confirmations. Keep clarifyingQuestions complete so the generic 2,000-character cap cannot hide later options. Other unhandled parts keep that cap. Test the actual simulated-user inputs for prose with questions or an alert proposal, and a long questionnaire with all its offered options. The conversation and document loops keep their existing reply handling. jira: trivial risk: nonprod --- .../gooddata_eval/core/agentic/alert_skill.py | 19 ++++-- .../core/agentic/metric_skill.py | 4 +- .../src/gooddata_eval/core/chat/render.py | 4 +- .../tests/test_agentic_alert_skill.py | 67 +++++++++++++++++-- .../tests/test_agentic_metric_skill.py | 36 ++++++++++ .../gooddata-eval/tests/test_chat_render.py | 33 +++++++++ 6 files changed, 147 insertions(+), 16 deletions(-) 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}}))