From 370f0a65bbae135d007d0d30a3f8899eaab67964 Mon Sep 17 00:00:00 2001 From: "my.nguyen" Date: Wed, 11 Nov 2026 17:52:48 +0700 Subject: [PATCH 1/2] fix(gooddata-eval): say a date range only when the case states one The dashboard simulated reply always ended with "Date range: ...". A case without a date_range key read as None and was answered with "all time", although such a case does not look at the date filter. That reply can contradict the cases it is sent for. When a case expects existing charts that carry their own date filter (this month, last 12 months), "all time" leads the agent to author all-time copies instead of reusing them, and the case fails on the missing chart ids although the agent did what the reply said. The reply now says the date range only when the key is present, the same rule the scoring already follows. A null date_range still asks for all time, so the cases that state one get the same reply as before. jira: QA-29724 risk: low --- .../core/agentic/dashboard_skill.py | 18 ++++++++++++++---- .../tests/test_agentic_dashboard_skill.py | 13 +++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py index 2fafc30ba..f21ea3f54 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py @@ -1067,9 +1067,14 @@ def build_simulated_reply(expected_output: dict) -> str: """Build the reply the simulated user sends when the agent asks back instead of drafting. Deterministic and LLM-free: the only things the agent still needs are the charts and the - date range, and both are already in the expectation. A fixed reply also keeps a failure - attributable to the agent rather than to a simulated user that phrased things differently - each run. + date range, and the expectation states both whenever the case checks them. A fixed reply + also keeps a failure attributable to the agent rather than to a simulated user that + phrased things differently each run. + + The date range is said only when the case states one, ``null`` included. A case without + the key does not look at the date filter, so naming a period would be the reply asserting + something the case never did -- and "all time" can contradict the date filters of the + existing charts the case expects to be reused. """ visualizations = expected_output.get("visualizations") or [] existing = [str(v.get("title", "")) for v in visualizations if v.get("id") is not None] @@ -1095,11 +1100,16 @@ def build_simulated_reply(expected_output: dict) -> str: quoted = " and ".join(f'"{c}"' for c in charts) placements.append(f"{quoted} on {on_tab}" if charts else on_tab) tabs = f"Use these tabs: {'; '.join(placements)}. " if placements else "" + date = ( + f"Date range: {_date_range_text(expected_output.get('date_range'))}. " + if _has_date_range(expected_output) + else "" + ) return ( f"Please use these charts: {', and '.join(segments)}. " f"{tabs}" - f"Date range: {_date_range_text(expected_output.get('date_range'))}. " + f"{date}" f"Anything else is up to you. Please create the dashboard now." ) diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index 2edfc58e8..1dd74e87f 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -937,6 +937,19 @@ def test_an_authored_chart_is_appended(self): "Anything else is up to you. Please create the dashboard now." ) + def test_a_case_without_a_date_range_says_no_period(self) -> None: + """The case does not look at the date filter, so the reply must not pick one -- "all time" + could contradict the date filter of an existing chart the case expects to be reused.""" + expected = {k: v for k, v in _DC03_EXPECTED.items() if k != "date_range"} + assert build_simulated_reply(expected) == ( + 'Please use these charts: Activity by Hour, and a new chart titled "Net Orders by Customer Age". ' + "Anything else is up to you. Please create the dashboard now." + ) + + def test_a_null_date_range_still_asks_for_all_time(self) -> None: + """Null is a stated range: the case requires all time and checks it.""" + assert "Date range: all time. " in build_simulated_reply({**_DC03_EXPECTED, "date_range": None}) + def test_an_authored_chart_alone_reads_naturally(self): expected = {"visualizations": [{"id": None, "title": "Net Orders by Customer Age"}], "date_range": None} assert 'Please use these charts: a new chart titled "Net Orders by Customer Age".' in build_simulated_reply( From 4e897449e9373b18665b286a7c335081a846a8d1 Mon Sep 17 00:00:00 2001 From: "my.nguyen" Date: Wed, 11 Nov 2026 17:55:20 +0700 Subject: [PATCH 2/2] feat(gooddata-eval): answer a dashboard edit's clarifying question An edit case stopped at the first turn that asked back instead of patching, by design: the simulated reply is built for creation and names charts and a date range, which answers nothing an edit asks. An edit whose wording left room for a question, such as where in a section new charts should go, therefore failed without a patch even when the agent's question was reasonable and offered a default. An edit case can now carry `clarification_reply`, the text its author wants the simulated user to send. When the agent asks back, that text is sent once. Nothing is generated or guessed, so the reply stays fixed across runs and says only what the fixture says. Whether a turn asked is decided by classify_reply, as the document skill does, so a turn that asked nothing, such as one reporting a rejected patch, is not nudged into a retry. A turn that only asks to go ahead ("Shall I apply this?") is told "Yes, go ahead.", as the conversation runner does; that does not spend the case's reply, and the turn cap still bounds it. A real question that ends in "Please confirm" is still a question, read with asks_in_text, and gets the case's reply. The run still stops when the case has no reply, or when the agent asks a second time. It now records the question it stopped on as a note (not scored) in the assertion message, so triage can tell a fixture that needs a reply from an agent that did the wrong thing. A structured clarifyingQuestions part is quoted in place of the prose beside it, and a run whose turn cap ends it before the reply could be sent says that too. A turn that neither asked nor patched is quoted in a note of its own. These stops were reported as budget_exhausted, although turns were left. They now report a new LoopExit, stopped_early, so a count of exhausted budgets no longer includes runs the harness ended itself. A refusal case, answered in one turn on purpose, reports it too. A run that does hit the cap keeps budget_exhausted. The key must be a non-empty string and is rejected on a creation case, which keeps its reply built from the charts and date range. jira: QA-29515 risk: low --- .../core/agentic/_clarification.py | 5 +- .../core/agentic/dashboard_skill.py | 152 ++++++++++++--- .../src/gooddata_eval/core/models.py | 6 + .../tests/test_agentic_dashboard_skill.py | 181 +++++++++++++++++- 4 files changed, 313 insertions(+), 31 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_clarification.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_clarification.py index 86a23b375..5e0a03b28 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_clarification.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_clarification.py @@ -370,7 +370,8 @@ def render(self) -> str: return "\n\n".join(f"**{text}**\n\n{answer}" for _, text, answer in sorted(sections, key=lambda s: s[0])) -def _all_questions(chat_result: ChatResult) -> list[dict]: +def all_questions(chat_result: ChatResult) -> list[dict]: + """Every question of every ``clarifyingQuestions`` part the reply carried, in order, whatever item it belongs to.""" parts = [p for p in chat_result.unhandled_parts or [] if p.get("type") == "clarifyingQuestions"] return [q for p in parts for q in p.get("questions") or []] @@ -502,7 +503,7 @@ def after_answers_mismatches( if skipped_questions_not_reasked: for question in skipped: asked_again = any( - _same_question(later, question) for _, reply, _ in replies_after for later in _all_questions(reply) + _same_question(later, question) for _, reply, _ in replies_after for later in all_questions(reply) ) if asked_again: out.append(f"asked the skipped question {question['text']!r} again") diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py index f21ea3f54..6fe2a304c 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py @@ -10,6 +10,8 @@ import jsonpatch +from gooddata_eval.core.agentic._clarification import all_questions +from gooddata_eval.core.agentic._conversation_context import CONFIRMATION_REPLY, asks_in_text, classify_reply, clip from gooddata_eval.core.agentic._failed_runs import build_failed_runs from gooddata_eval.core.agentic._gate import ( DEFAULT_GATE, @@ -54,9 +56,10 @@ # the loop ends there rather than replying into silence. Fewer than metric_skill's seven # because those rounds do carry new content, its reply being generated per turn by an LLM. # -# Only creation ever reaches the later rounds. An edit stops after the first turn, since the -# creation reply names charts and a date range and answers nothing a rename or a resize could -# have asked. +# Only creation ever reaches the later rounds. The creation reply names charts and a date range +# and answers nothing a rename or a resize could have asked, so an edit that asks back is +# answered once with its fixture's `clarification_reply`, and stops there when it has none or +# asks again. An edit that only asks to go ahead is told yes, which spends no reply. # # The cost of the slack is that four turns at ChatClient's 300s read timeout exceed the 720s # per-test timeout gdc-nas derives for a k=1 dataset, so a run that stalls on every turn is cut @@ -1253,6 +1256,16 @@ def _validate_expectation(expected_output: dict) -> None: for pattern in must_not_match: _compile_pattern(pattern) + if "clarification_reply" in expected_output: + reply = expected_output["clarification_reply"] + if not isinstance(reply, str) or not reply.strip(): + raise ValueError(f"clarification_reply must be a non-empty reply, got {reply!r}") + if not _is_edit(expected_output): + raise ValueError( + "clarification_reply is for an edit case; a creation case is answered with the reply " + "built from its charts and date range" + ) + forbidden = _forbidden_visualizations(expected_output) if not isinstance(forbidden, list): raise ValueError(f"must_not_contain must be a list of chart expectations, got {forbidden!r}") @@ -2012,6 +2025,47 @@ def evaluate_dashboard_response( ) +# Long enough for a question with its options, short enough to keep the assertion message readable. +_QUOTED_QUESTION_CHARS = 300 + + +def _asked_question(chat_result: ChatResult, response_text: str) -> str: + """What the agent asked: the ``clarifyingQuestions`` part's questions when it sent one, else its text. + + The structured part is what ``classify_reply`` reads as the question, and prose sent beside it + ("I need more details.") would quote the lead-in rather than what was asked. + """ + questions = [str(q.get("text") or "").strip() for q in all_questions(chat_result)] + return " ".join(q for q in questions if q) or response_text + + +def _quote(text: str) -> str: + """The agent's words cut to a readable length and quoted, or a marker when it said nothing.""" + clipped = clip(text, _QUOTED_QUESTION_CHARS) + return repr(clipped) if clipped else "(no text)" + + +def _unacted_edit_note(response_text: str) -> str: + """Say what an edit run stopped on when the agent neither patched nor asked. + + Not scored, like ``_unanswered_edit_note``: it tells a reply that gave up apart from one that asked. + """ + return f"the agent replied on an edit without a patch or a question, so the run stopped: {_quote(response_text)}" + + +def _unanswered_edit_note(question: str, *, replied: bool, has_reply: bool) -> str: + """Say which question an edit run stopped on, so triage can tell a fixture gap from the agent. + + Not scored: the run already fails on the missing patch, and this only says why it never got one. + """ + quoted = _quote(question) + if replied: + return f"the agent asked again after the case's clarification_reply, and the run stopped: {quoted}" + if has_reply: + return f"the turn budget ran out before the case's clarification_reply could be sent: {quoted}" + return f"the agent asked back on an edit and the case has no clarification_reply, so the run stopped: {quoted}" + + def _execute_single_dashboard_run( client: ChatClient, conversation_id: str, @@ -2034,6 +2088,11 @@ def _execute_single_dashboard_run( turns = 0 steps = 0 current_question = question + # Validated to be on an edit case only, and a non-empty string when present. + clarification_reply: str | None = expected_output.get("clarification_reply") + reply_sent = False + # Why an edit stopped without a patch, appended to the evaluation as an unscored note. + edit_note: str | None = None reasoning_steps: list[str] = [] response_id: str | None = None all_tool_call_events: list[ToolCallEvent] = [] @@ -2043,9 +2102,7 @@ def _execute_single_dashboard_run( tool_index_offset = 0 reasoning_index_offset = 0 # Defaults to BUDGET_EXHAUSTED: every other exit assigns explicitly, so a loop that - # simply runs out of range() (or the is_edit "no reply of its own" branch below, which - # is the same shape of outcome -- the agent answered, just not with the dashboard tool) - # is labelled correctly with no trailing else. + # simply runs out of range() is labelled correctly with no trailing else. exit_reason = LoopExit.BUDGET_EXHAUSTED run_started = time.monotonic() @@ -2131,36 +2188,77 @@ def _execute_single_dashboard_run( # restates the charts and the date range and asks again for the dashboard, which is # pressure to build the very thing the case says must not be built -- so a correct # agent would be talked out of the right answer by the harness. + exit_reason = LoopExit.STOPPED_EARLY break + reply_kind = classify_reply(chat_result, response_text) + if is_edit and reply_kind == "confirmation" and asks_in_text(chat_result, response_text): + # "Which widget should I resize? Please confirm." asks something a yes does not answer. + reply_kind = "question" + if is_edit: + # The creation reply names charts and a date range, which answers nothing a rename + # or a resize could have asked, and nothing here can guess what an edit question + # needs. Only the fixture's own reply is sent, only to a turn that asked (as + # document_skill gates it), and only once: a second question is one its author did + # not foresee, so the run stops and says what was asked, which tells a fixture that + # needs a reply apart from an agent that did the wrong thing. A turn that asked + # nothing -- a rejected patch reported back, say -- is not nudged into a retry. A turn + # that only asks to go ahead is told yes, as conversation.py does; that spends no reply, + # and the iteration cap still bounds it. + # Checked before the iteration cap: these stops are the harness having nothing to + # send, not the budget running out, and each still says what went unanswered. + if reply_kind == "no_action": + edit_note = _unacted_edit_note(response_text) + exit_reason = LoopExit.STOPPED_EARLY + break + if reply_kind == "question" and (clarification_reply is None or reply_sent): + edit_note = _unanswered_edit_note( + _asked_question(chat_result, response_text), + replied=reply_sent, + has_reply=clarification_reply is not None, + ) + exit_reason = LoopExit.STOPPED_EARLY + break if iteration >= max_iterations - 1: + if is_edit and reply_kind == "question": + # A question here still had its reply to send; a confirmation needs no note. + edit_note = _unanswered_edit_note( + _asked_question(chat_result, response_text), replied=False, has_reply=True + ) break + if is_edit and reply_kind == "confirmation": + current_question = CONFIRMATION_REPLY + answering_with = "a confirmation" + elif is_edit and clarification_reply is not None: + reply_sent = True + current_question = clarification_reply + answering_with = "the case's clarification_reply" + else: + current_question = build_simulated_reply(expected_output) + answering_with = "the expected charts and date range" log_timer( f"[timer] dashboard_skill {conversation_id} GoodData turn {turns} complete after " - f"{agent_elapsed:.2f}s; answering with the expected charts and date range" + f"{agent_elapsed:.2f}s; answering with {answering_with}" ) - if is_edit: - # The creation reply names charts and a date range, which answers nothing a rename - # or a resize could have asked. Rather than send something the question did not - # ask for, stop: the failure is the signal that an edit case needs a reply of its - # own, and inventing one here would hide which cases actually need it. - break - current_question = build_simulated_reply(expected_output) + + evaluation = evaluate_dashboard_response( + tool_result, + dashboard_part, + expected_output, + _skill_activated(all_tool_call_events, _required_skill(expected_output)), + patch_part=patch_part, + answer_text=answer_text, + # Any call, not just a successful one -- see DashboardEvaluation. BOTH tools, + # not the one this case selected: a refusal is a creation shape, so `tool` is + # `draft_dashboard`, and an agent that routed to the editor and had a + # `patch_dashboard` call rejected would otherwise leave this false and pass. + producing_tool_called=any(tc.function_name in (_DRAFT_TOOL, _PATCH_TOOL) for tc in all_tool_call_events), + ) + if edit_note is not None: + evaluation.notes.append(edit_note) return DashboardRunResult( conversation_id=conversation_id, - evaluation=evaluate_dashboard_response( - tool_result, - dashboard_part, - expected_output, - _skill_activated(all_tool_call_events, _required_skill(expected_output)), - patch_part=patch_part, - answer_text=answer_text, - # Any call, not just a successful one -- see DashboardEvaluation. BOTH tools, - # not the one this case selected: a refusal is a creation shape, so `tool` is - # `draft_dashboard`, and an agent that routed to the editor and had a - # `patch_dashboard` call rejected would otherwise leave this false and pass. - producing_tool_called=any(tc.function_name in (_DRAFT_TOOL, _PATCH_TOOL) for tc in all_tool_call_events), - ), + evaluation=evaluation, tool_result=tool_result, dashboard_part=dashboard_part, patch_part=patch_part, diff --git a/packages/gooddata-eval/src/gooddata_eval/core/models.py b/packages/gooddata-eval/src/gooddata_eval/core/models.py index 435ac811a..094cee08e 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/models.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/models.py @@ -40,6 +40,12 @@ class LoopExit(str, Enum): """The loop hit ``max_iterations`` without the agent producing its output. Says nothing about whether the agent was on track; it may have been one turn away.""" + STOPPED_EARLY = "stopped_early" + """The agent replied and the harness had nothing to send back, so the loop stopped with + turns left: a question the case has no reply for, a second question after the case's + reply was spent, a reply that neither asked nor acted, or a refusal case answered in one + turn, where any follow-up would press the agent to build what it must not.""" + SIMULATED_USER_FAILED = "simulated_user_failed" """The harness's own simulated-user model failed to produce a follow-up. A harness-side fault, not an agent one -- reporting should treat it as an error, not a scored failure.""" diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index 1dd74e87f..e7d577df2 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch import pytest +from gooddata_eval.core.agentic._conversation_context import CONFIRMATION_REPLY from gooddata_eval.core.agentic.dashboard_skill import ( DashboardEvaluation, DashboardSkillAssertionError, @@ -718,8 +719,8 @@ def test_a_patch_that_does_not_apply_claims_nothing_about_charts(self): def test_an_edit_that_asks_back_stops_rather_than_sending_the_creation_reply(self): """The creation reply names charts and a date range, which answers nothing a rename or - a resize could have asked. Sending it anyway would hide which cases need a reply of - their own.""" + a resize could have asked. Without a reply of its own the run stops, and says what the + agent asked so triage sees the case needs one.""" client = MagicMock() client.send_message.return_value = _chat_result( tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], @@ -729,6 +730,180 @@ def test_an_edit_that_asks_back_stops_rather_than_sending_the_creation_reply(sel assert client.send_message.call_count == 1 assert summary.best.total_turns == 1 assert not summary.pass_at_k + # Three turns were left: the run stopped because nothing could answer, not on the budget. + assert summary.best.exit_reason is LoopExit.STOPPED_EARLY + assert summary.best.evaluation.notes == [ + "the agent asked back on an edit and the case has no clarification_reply, so the run stopped: " + "'Which widget did you mean?'" + ] + + def test_an_edit_that_asks_back_is_answered_with_its_clarification_reply(self) -> None: + reply = "Rename the Order status chart. Apply it now, no need to confirm." + client = MagicMock() + client.send_message.side_effect = [ + _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="Which widget did you mean?", + ), + _chat_result( + tool_calls=[_tool_call("patch_dashboard", _patch_result())], + parts=[_edit_base_part(), _patch_part(TestPreservedFilters._RENAME, visualizations=[_ORDER_STATUS])], + ), + ] + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": reply}, max_iterations=None) + assert [call.args[1] for call in client.send_message.call_args_list] == [ + "Could you create a dashboard?", + reply, + ] + assert summary.pass_at_k + assert summary.best.total_turns == 2 + assert summary.best.evaluation.notes == [] + + def test_the_clarification_reply_is_sent_once_and_a_second_question_is_quoted(self) -> None: + """A second question is one the fixture's author did not foresee. Repeating the same + reply would not answer it, so the run stops and names it.""" + reply = "Add both at the end of the section. Apply it now, no need to confirm." + client = MagicMock() + client.send_message.side_effect = [ + _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="Where should I place them?", + ), + _chat_result(text="Should the new chart be full width?"), + ] + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": reply}, max_iterations=None) + assert client.send_message.call_count == 2 + assert not summary.pass_at_k + assert summary.best.exit_reason is LoopExit.STOPPED_EARLY + assert summary.best.evaluation.notes == [ + "the agent asked again after the case's clarification_reply, and the run stopped: " + "'Should the new chart be full width?'" + ] + + def test_an_edit_turn_that_asks_nothing_is_not_sent_the_clarification_reply(self) -> None: + """A turn that reports a rejected patch asked nothing; sending the reply would nudge a retry.""" + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="I could not apply the change to the dashboard.", + ) + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": "Apply it now."}, max_iterations=None) + assert client.send_message.call_count == 1 + assert not summary.pass_at_k + assert summary.best.exit_reason is LoopExit.STOPPED_EARLY + assert summary.best.evaluation.notes == [ + "the agent replied on an edit without a patch or a question, so the run stopped: " + "'I could not apply the change to the dashboard.'" + ] + + def test_an_edit_that_asks_to_go_ahead_is_told_yes_without_spending_its_reply(self) -> None: + """A confirmation asks nothing the fixture has to answer: "Yes, go ahead." settles it, and the + clarification_reply stays for a real question.""" + reply = "Add both at the end of the section. Apply it now, no need to confirm." + client = MagicMock() + client.send_message.side_effect = [ + _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="I will rename the chart. Shall I apply this?", + ), + _chat_result(text="Where should I place them?"), + _chat_result( + tool_calls=[_tool_call("patch_dashboard", _patch_result())], + parts=[_edit_base_part(), _patch_part(TestPreservedFilters._RENAME, visualizations=[_ORDER_STATUS])], + ), + ] + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": reply}, max_iterations=None) + assert [call.args[1] for call in client.send_message.call_args_list] == [ + "Could you create a dashboard?", + CONFIRMATION_REPLY, + reply, + ] + assert summary.pass_at_k + assert summary.best.evaluation.notes == [] + + def test_a_question_asked_beside_a_confirmation_is_sent_the_clarification_reply(self) -> None: + """A real question followed by "Please confirm" is still a question, so the fixture's reply answers it.""" + reply = "Resize the Order status chart. Apply it now, no need to confirm." + client = MagicMock() + client.send_message.side_effect = [ + _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="Which widget should I resize? Please confirm.", + ), + _chat_result( + tool_calls=[_tool_call("patch_dashboard", _patch_result())], + parts=[_edit_base_part(), _patch_part(TestPreservedFilters._RENAME, visualizations=[_ORDER_STATUS])], + ), + ] + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": reply}, max_iterations=None) + assert [call.args[1] for call in client.send_message.call_args_list] == [ + "Could you create a dashboard?", + reply, + ] + assert summary.pass_at_k + + def test_a_cap_reached_before_the_clarification_reply_is_sent_still_quotes_the_question(self) -> None: + """With one turn there is no room for the reply: the budget ended the run, and the note says so.""" + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="Where should I place them?", + ) + summary = _run_with(client, {**_DE01_EXPECTED, "clarification_reply": "At the end."}, max_iterations=1) + assert client.send_message.call_count == 1 + assert summary.best.exit_reason is LoopExit.BUDGET_EXHAUSTED + assert summary.best.evaluation.notes == [ + "the turn budget ran out before the case's clarification_reply could be sent: 'Where should I place them?'" + ] + + def test_a_structured_question_is_quoted_rather_than_the_text_beside_it(self) -> None: + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + parts=[ + { + "type": "clarifyingQuestions", + "questions": [{"text": "Which widget?", "control": {"options": []}}], + } + ], + text="I need more details.", + ) + summary = _run_with(client, _DE01_EXPECTED, max_iterations=None) + assert summary.best.evaluation.notes == [ + "the agent asked back on an edit and the case has no clarification_reply, so the run stopped: " + "'Which widget?'" + ] + + def test_a_long_unanswered_question_is_cut_in_the_note(self) -> None: + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("set_skills", {"skills_to_activate": ["dashboard_editor"]})], + text="Which option? " + "Option " * 100, + ) + note = _run_with(client, _DE01_EXPECTED, max_iterations=None).best.evaluation.notes[0] + assert note.endswith("…'") + assert len(note) < 450 + + @pytest.mark.parametrize( + ("reply", "message"), + [ + ("", "clarification_reply must be a non-empty reply"), + (" ", "clarification_reply must be a non-empty reply"), + (["Add it at the end."], "clarification_reply must be a non-empty reply"), + ], + ) + def test_an_unusable_clarification_reply_is_rejected_up_front(self, reply: object, message: str) -> None: + client = MagicMock() + with pytest.raises(ValueError, match=message): + _run_with(client, {**_DE01_EXPECTED, "clarification_reply": reply}) + client.send_message.assert_not_called() + + def test_a_creation_case_cannot_carry_a_clarification_reply(self) -> None: + """A creation case is answered with the reply built from its charts and date range.""" + client = MagicMock() + with pytest.raises(ValueError, match="clarification_reply is for an edit case"): + _run_with(client, {**_DC05_EXPECTED, "clarification_reply": "Use these charts."}) + client.send_message.assert_not_called() def test_a_creation_case_still_gets_its_clarifying_round(self): """The same cap that an edit never reaches is still spent on creation.""" @@ -2915,3 +3090,5 @@ def test_the_simulated_user_never_argues_a_refusal_into_building(self): summary = _run_with(client, self._NO_DATA) assert client.send_message.call_count == 1 assert summary.best.evaluation.strict_pass + # Turns were left: the harness chose not to reply, so the budget did not end it. + assert summary.best.exit_reason is LoopExit.STOPPED_EARLY