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 9e017d57a..2784c1b71 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 @@ -3,6 +3,7 @@ from __future__ import annotations +import re import time from dataclasses import dataclass, field from typing import Any @@ -270,6 +271,65 @@ def _check_visualizations(widgets: list[dict], new_ids: set[str], expected: list return failures, title_mismatches +def _check_forbidden(widgets: list[dict], forbidden: list[dict]) -> list[str]: + """Report charts the expectation says must NOT be on the dashboard. + + The counterpart to ``_check_visualizations``, which allows extras. Extras are the right + default -- a user asking for three charts is not wronged by a fourth -- but it leaves the + cases that are about what the agent must *refrain* from unassertable: a chart the agent was + told it could not use, and a substitute authored in place of one it could not resolve. + + An entry with an ``id`` is matched on that id alone; a ``title`` beside it is documentation + for whoever reads the fixture, not a second matcher. Titles are not unique in a real + workspace -- the catalog this was written against has 102 visualizations whose title + contains "approval rate" -- so widening an id entry to also match its title would fail the + case on a chart that merely shares a name with the forbidden one. + + An entry with only a ``title`` matches on the title, which is what catches a substitution + whose id is not known in advance. + """ + failures: list[str] = [] + for entry in forbidden: + exp_id = entry.get("id") + exp_title = str(entry.get("title") or "") + if exp_id is not None: + if any(w.get("visualization") == exp_id for w in widgets): + failures.append(f"chart id={exp_id!r} title={exp_title!r} is on the dashboard and must not be") + else: + hits = [w for w in widgets if _norm(str(w.get("title") or "")) == _norm(exp_title)] + if hits: + ids = ", ".join(repr(w.get("visualization")) for w in hits) + failures.append(f"a chart titled {exp_title!r} is on the dashboard ({ids}) and must not be") + return failures + + +def _check_answer(answer: str, must_include: list[str], must_not_match: list[str]) -> list[str]: + """Score the text the user actually reads. + + Nothing else in this evaluator looks at the answer: every other check reads the drafted + document, so a run that builds the right dashboard and describes it wrongly scores as a + clean pass. The two cases that need this are the ones where the text *is* the deliverable + -- an agent that cannot build what was asked has to say so, and one that can has to say it + without leaking the raw ``{visualization/}`` tokens the renderer was meant to resolve. + + ``must_include`` is substring, case- and whitespace-insensitive, because it asserts that a + point was made rather than that a sentence was phrased a particular way. ``must_not_match`` + is a regular expression, because what it rules out is usually a shape -- a token, an id, a + claim pattern -- and not a fixed string. + """ + normalized = _norm(answer) + failures: list[str] = [ + f"the answer does not mention {phrase!r}" for phrase in must_include if _norm(str(phrase)) not in normalized + ] + for pattern in must_not_match: + # Compiled at validation time too, so a bad pattern fails the fixture rather than the + # run; compiling again here keeps this function usable on its own. + found = re.search(str(pattern), answer, re.IGNORECASE) + if found: + failures.append(f"the answer matches {pattern!r}, which it must not (matched {found.group(0)!r})") + return failures + + def _check_references(widgets: list[dict], known_ids: set[str]) -> list[str]: """Report widgets whose id the response's references never carried. @@ -491,6 +551,76 @@ def _min_new_visualizations(expected_output: dict) -> int: return value +def _max_new_visualizations(expected_output: dict) -> int | None: + """Upper bound on the number of charts the agent may author, or ``None`` for no bound. + + The bound that was missing. ``min_new_visualizations`` alone cannot express "use what is + already there", which is the central requirement wherever a curated catalog exists: with + only a floor, ``0`` is satisfied by any number of authored charts, so a case meant to + assert that the agent reused existing work passes however much it invented. Pairing it + with ``max_new_visualizations: 0`` is what turns that into an assertion. + + Absent means unbounded, which is the behaviour every fixture written before this had. + ``0`` is a real bound and must not be read as absence. + + Raises: + ValueError: the bound is not a number, is negative, or is below the floor — a ceiling + under the floor cannot be satisfied by any run, so the fixture is unscoreable + rather than strict. + """ + raw = expected_output.get("max_new_visualizations") + if raw is None: + return None + try: + value = int(raw) + except (TypeError, ValueError) as exc: + raise ValueError(f"max_new_visualizations is not a number: {raw!r}") from exc + if value < 0: + raise ValueError(f"max_new_visualizations must not be negative, got {value}") + minimum = _min_new_visualizations(expected_output) + if value < minimum: + raise ValueError(f"max_new_visualizations ({value}) is below min_new_visualizations ({minimum})") + return value + + +def _forbidden_visualizations(expected_output: dict) -> list[dict]: + """The ``must_not_contain`` entries, as a list.""" + return expected_output.get("must_not_contain") or [] + + +def _answer_assertions(expected_output: dict) -> tuple[list[str], list[str]]: + """The answer-text assertions, as ``(must_include, must_not_match)``.""" + return ( + list(expected_output.get("answer_must_include") or []), + list(expected_output.get("answer_must_not_match") or []), + ) + + +def _compile_pattern(pattern: object) -> re.Pattern[str]: + """Compile one ``answer_must_not_match`` entry, naming the fixture key on failure. + + Raises: + ValueError: the pattern does not compile. + """ + try: + return re.compile(str(pattern)) + except re.error as exc: + raise ValueError(f"answer_must_not_match carries an invalid regular expression {pattern!r}: {exc}") from exc + + +def _expects_dashboard(expected_output: dict) -> bool: + """Whether the case expects a dashboard at all. + + ``false`` is the refusal shape: the agent was asked for something it cannot build, and the + right outcome is that it says so and drafts nothing. Those cases could not be authored + before, because an expectation with no ``visualizations`` is rejected as vacuous -- and + correctly so, since every chart check would pass against nothing. What makes a refusal + scoreable is not relaxing that rule but replacing the assertion: the answer carries the + whole of what the run is judged on, so a refusal fixture has to state it. + """ + return expected_output.get("expects_dashboard", True) is not False + + def _has_filters(expected_output: dict) -> bool: """Whether the expectation says anything about the attribute filters. @@ -521,7 +651,39 @@ def _validate_expectation(expected_output: dict) -> None: Raises: ValueError: the expectation is unusable. """ - if not expected_output.get("visualizations"): + must_include, must_not_match = _answer_assertions(expected_output) + for key, value in (("answer_must_include", must_include), ("answer_must_not_match", must_not_match)): + if not isinstance(expected_output.get(key, []), list): + raise ValueError(f"{key} must be a list of strings, got {expected_output.get(key)!r}") + if any(not str(entry).strip() for entry in value): + raise ValueError(f"{key} carries an empty entry, which asserts nothing: {value!r}") + # Compiled here so an unparseable pattern fails the fixture before the first API call, + # rather than raising mid-scoring with a run already spent. + for pattern in must_not_match: + _compile_pattern(pattern) + + 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}") + for entry in forbidden: + if not isinstance(entry, dict): + raise ValueError(f"a must_not_contain entry must be an object, got {entry!r}") + if entry.get("id") is None and not str(entry.get("title") or "").strip(): + raise ValueError(f"a must_not_contain entry needs an id or a title, got {entry!r}") + + if not _expects_dashboard(expected_output): + # The refusal shape. Each of these would otherwise produce a fixture that looks strict + # and asserts nothing, which is the failure mode the vacuity check below exists to stop. + if expected_output.get("visualizations"): + raise ValueError("expects_dashboard is false, so the case must not list visualizations to find") + if _is_edit(expected_output): + raise ValueError("expects_dashboard is false is a creation shape; an edit always produces a patch") + if not must_include and not must_not_match: + raise ValueError( + "expects_dashboard is false needs an answer assertion; without one the case scores nothing " + "beyond the absence of a draft, which an agent that fell over satisfies too" + ) + elif not expected_output.get("visualizations"): raise ValueError("expected_output lists no visualizations; every chart check would pass vacuously") if _has_date_range(expected_output): date_range = expected_output.get("date_range") @@ -552,6 +714,7 @@ def _validate_expectation(expected_output: dict) -> None: elif saved_id is not None: raise ValueError(f"a creation expectation must not name a saved_dashboard_id, got {saved_id!r}") _min_new_visualizations(expected_output) + _max_new_visualizations(expected_output) @dataclass(frozen=True) @@ -567,6 +730,12 @@ class _Applies: patch: bool date: bool filters: bool + forbidden: bool = False + answer: bool = False + # Not a check of its own but a switch over the whole set: a refusal case is scored on the + # absence of a draft and on what the answer said, so publishing the document checks beside + # it would report six passes for a document that was never produced. + refusal: bool = False @dataclass @@ -598,6 +767,12 @@ class DashboardEvaluation: patch_applies: bool = True references_carried: bool = True titles_matched: bool = True + forbidden_absent: bool = True + answer_matched: bool = True + # Whether EITHER dashboard-producing tool was called at all, successful or not. Only a + # refusal case reads it: everywhere else a failed call is a retry the agent recovered + # from, which is why `_extract_tool_result` skips it. + producing_tool_called: bool = False failures: list[str] = field(default_factory=list) notes: list[str] = field(default_factory=list) @@ -607,6 +782,25 @@ def strict_pass(self) -> bool: @property def strict_checks(self) -> dict[str, bool]: + if self.applies.refusal: + # Named for what has to be true, not negated at read time: `dashboard_not_drafted` + # false means a dashboard came back from a case that asked for none, which is the + # failure. A successful tool call with no part, or a part with no successful call, + # are each a dashboard the user would be shown. + # + # The third term is the one that is easy to miss: an agent that CALLED the tool and + # had the call rejected leaves no result and no part, so the first two would read it + # as a clean refusal. It is not one -- the agent tried to build and the tool stopped + # it, and scoring that as correct credits the model for a guardrail. + checks = { + "dashboard_not_drafted": ( + not self.drafted and not self.part_present and not self.producing_tool_called + ), + "dashboard_skill_activated": self.skill_activated, + } + if self.applies.answer: + checks["answer_matched"] = self.answer_matched + return checks checks = { # Kept as-is through the editing work: every Langfuse view, saved filter and # combo-report field list already refers to it, and a rename would break them for @@ -626,6 +820,10 @@ def strict_checks(self) -> dict[str, bool]: # Prefixed: alert_skill already publishes a `filters_correct` score, and the combo # report resolves a trace's skill by which score names it carries. checks["dashboard_filters_correct"] = self.filters_correct + if self.applies.forbidden: + checks["forbidden_charts_absent"] = self.forbidden_absent + if self.applies.answer: + checks["answer_matched"] = self.answer_matched return checks @property @@ -682,6 +880,8 @@ def evaluate_dashboard_response( expected_output: dict, skill_activated: bool, patch_part: dict | None = None, + answer_text: str = "", + producing_tool_called: bool = False, ) -> DashboardEvaluation: """Score one dashboard response against its expectation. @@ -693,8 +893,58 @@ def evaluate_dashboard_response( without an agent. """ is_edit = _is_edit(expected_output) - applies = _Applies(patch=is_edit, date=_has_date_range(expected_output), filters=_has_filters(expected_output)) + forbidden = _forbidden_visualizations(expected_output) + must_include, must_not_match = _answer_assertions(expected_output) + expects_dashboard = _expects_dashboard(expected_output) + applies = _Applies( + patch=is_edit, + date=_has_date_range(expected_output), + filters=_has_filters(expected_output), + forbidden=bool(forbidden), + answer=bool(must_include or must_not_match), + refusal=not expects_dashboard, + ) tool = _producing_tool(expected_output) + answer_failures = _check_answer(answer_text, must_include, must_not_match) + + if not expects_dashboard: + # The whole of a refusal case: nothing was drafted, and the answer said the right + # thing. Scored before the branches below so the absence of a tool result reads as the + # outcome asked for rather than as "the agent never produced a successful draft". + drafted = tool_result is not None + return DashboardEvaluation( + drafted=drafted, + part_present=dashboard_part is not None, + charts_matched=True, + date_range_correct=True, + filters_correct=True, + new_visualizations_met=True, + applies=applies, + skill_activated=skill_activated, + producing_tool_called=producing_tool_called, + answer_matched=not answer_failures, + failures=[ + *( + [f"the case expects no dashboard, but the agent produced a successful {tool} call"] + if drafted + else [] + ), + *( + ["the case expects no dashboard, but the response carries a 'dashboard' part"] + if dashboard_part is not None + else [] + ), + *( + [ + "the case expects no dashboard, but the agent called a dashboard-producing " + "tool (the call did not succeed)" + ] + if producing_tool_called and not drafted + else [] + ), + *answer_failures, + ], + ) if tool_result is None: return DashboardEvaluation( @@ -708,15 +958,25 @@ def evaluate_dashboard_response( skill_activated=skill_activated, saved_dashboard_id_correct=False, patch_applies=False, - failures=[f"the agent never produced a successful {tool} call"], + answer_matched=not answer_failures, + failures=[f"the agent never produced a successful {tool} call", *answer_failures], ) min_new = _min_new_visualizations(expected_output) + max_new = _max_new_visualizations(expected_output) actual_new = tool_result.get("new_visualization_count") if isinstance(actual_new, int): - new_met = actual_new >= min_new + over = max_new is not None and actual_new > max_new + new_met = actual_new >= min_new and not over # Naming the shortfall, never "expected at least 0" -- see the envelope branch below. - new_failures = [] if new_met else [f"expected at least {min_new} authored chart(s), tool reported {actual_new}"] + # The ceiling is reported as its own sentence: "too few" and "too many" are opposite + # diagnoses and a single range message would make whoever reads the failure work out + # which end was missed. + new_failures: list[str] = [] + if actual_new < min_new: + new_failures.append(f"expected at least {min_new} authored chart(s), tool reported {actual_new}") + if over: + new_failures.append(f"expected at most {max_new} authored chart(s), tool reported {actual_new}") else: # Not a shortfall: the tool result no longer carries the key. Said plainly, because # "expected at least 0 authored chart(s), tool reported None" reads as a broken test @@ -741,9 +1001,11 @@ def evaluate_dashboard_response( skill_activated=skill_activated, saved_dashboard_id_correct=False, patch_applies=False, + answer_matched=not answer_failures, failures=[ f"the response carries no {' and no '.join(repr(p) for p in missing_parts)} part", *new_failures, + *answer_failures, ], ) @@ -794,7 +1056,8 @@ def evaluate_dashboard_response( skill_activated=skill_activated, saved_dashboard_id_correct=not saved_failures, patch_applies=False, - failures=[*patch_failures, *saved_failures, *new_failures], + answer_matched=not answer_failures, + failures=[*patch_failures, *saved_failures, *new_failures, *answer_failures], ) widgets = _widgets_of(document) @@ -810,6 +1073,7 @@ def evaluate_dashboard_response( title_notes: list[str] = [] else: title_notes = title_mismatches + forbidden_failures = _check_forbidden(widgets, forbidden) reference_notes = _check_references(widgets, known_ids) date_failures = ( _check_date_range(document, expected_output.get("date_range")) if _has_date_range(expected_output) else [] @@ -831,7 +1095,17 @@ def evaluate_dashboard_response( patch_applies=True, references_carried=not reference_notes, titles_matched=not title_mismatches, - failures=[*chart_failures, *saved_failures, *date_failures, *filter_failures, *new_failures], + forbidden_absent=not forbidden_failures, + answer_matched=not answer_failures, + failures=[ + *chart_failures, + *forbidden_failures, + *saved_failures, + *date_failures, + *filter_failures, + *new_failures, + *answer_failures, + ], notes=[*title_notes, *reference_notes], ) @@ -850,6 +1124,8 @@ def _execute_single_dashboard_run( """ tool = _producing_tool(expected_output) is_edit = _is_edit(expected_output) + expects_dashboard = _expects_dashboard(expected_output) + answer_text = "" tool_result: dict | None = None dashboard_part: dict | None = None patch_part: dict | None = None @@ -883,6 +1159,13 @@ def _execute_single_dashboard_run( all_reasoning_step_events.extend(chat_result.reasoning_step_events or []) steps += chat_result.reasoning_step_count + # Read every turn, not only the ones that answer without drafting: the turn that + # produces the draft carries the text describing it, and that text is exactly what the + # answer assertions are about. Kept as the last turn that said anything, so a silent + # turn after a spoken one does not erase what the user was shown. + response_text = (chat_result.text_response or "").strip() or render_answer_text(chat_result) + answer_text = response_text or answer_text + candidate = _extract_tool_result(chat_result.tool_call_events or [], tool) if candidate is not None: log_timer( @@ -905,9 +1188,14 @@ def _execute_single_dashboard_run( patch_part = _extract_dashboard_part(chat_result, _PATCH_TYPE) break - response_text = (chat_result.text_response or "").strip() or render_answer_text(chat_result) if not response_text and not chat_result.tool_call_events: break + if not expects_dashboard: + # A refusal case is answered in one turn. The reply this loop would otherwise send + # 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. + break if iteration >= max_iterations - 1: break log_timer( @@ -930,6 +1218,12 @@ def _execute_single_dashboard_run( 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), ), tool_result=tool_result, dashboard_part=dashboard_part, diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index e949d8a3b..afc728c8b 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -1192,3 +1192,379 @@ def _evaluate_with(client, expected_output): expected_output=expected_output, initial_conversation_id="conv-1", ) + + +# ── negative and answer-text assertions ───────────────────────────────────── +# The four capabilities a curated-catalog engagement needs and the document checks above +# cannot express: a ceiling on authored charts, charts that must not appear, a case whose +# right outcome is no dashboard at all, and the text the user actually reads. + +_SUBSTITUTE = "269d4650-2c1f-4bbf-8e30-5f2bba7f74b3" + + +class TestMaxNewVisualizations: + """`min` alone cannot say "use what is already there".""" + + def test_the_ceiling_is_unbounded_when_absent(self): + """Every fixture written before the ceiling existed carries no key, and must keep + passing when the agent authors charts it was not asked for.""" + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + ev = evaluate_dashboard_response( + _draft_result(new_visualization_count=5), part, _DC05_EXPECTED, skill_activated=True + ) + assert ev.new_visualizations_met + assert ev.strict_pass + + def test_authoring_beyond_the_ceiling_fails(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "max_new_visualizations": 0} + ev = evaluate_dashboard_response(_draft_result(new_visualization_count=1), part, expected, skill_activated=True) + assert not ev.new_visualizations_met + assert any("at most 0" in f for f in ev.failures) + + def test_a_zero_ceiling_is_a_bound_not_an_absence(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "max_new_visualizations": 0} + assert evaluate_dashboard_response( + _draft_result(new_visualization_count=0), part, expected, skill_activated=True + ).strict_pass + + def test_too_few_and_too_many_are_reported_as_different_diagnoses(self): + """One range message would make whoever reads the failure work out which end was + missed, and the two have opposite fixes.""" + part = _dashboard_part([_widget("Activity by Hour", _ACTIVITY_BY_HOUR)], date_filter=_ALL_TIME_FILTER) + under = evaluate_dashboard_response( + _draft_result(new_visualization_count=0), + part, + {**_DC03_EXPECTED, "max_new_visualizations": 2}, + skill_activated=True, + ) + assert any("at least 1" in f for f in under.failures) + assert not any("at most" in f for f in under.failures) + + def test_a_ceiling_below_the_floor_is_rejected_up_front(self): + client = MagicMock() + expected = {**_DC05_EXPECTED, "min_new_visualizations": 2, "max_new_visualizations": 1} + with pytest.raises(ValueError, match="below min_new_visualizations"): + _run_with(client, expected) + client.send_message.assert_not_called() + + def test_an_unparseable_ceiling_is_rejected_up_front(self): + client = MagicMock() + with pytest.raises(ValueError, match="max_new_visualizations is not a number"): + _run_with(client, {**_DC05_EXPECTED, "max_new_visualizations": "none"}) + client.send_message.assert_not_called() + + +class TestMustNotContain: + def test_a_forbidden_chart_id_on_the_dashboard_fails(self): + part = _dashboard_part( + [ + _widget("Total Customers", _TOTAL_CUSTOMERS), + _widget("Active Customers", _ACTIVE_CUSTOMERS), + _widget("Approval Rate", _SUBSTITUTE), + ], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "must_not_contain": [{"id": _SUBSTITUTE, "title": "Approval Rate"}]} + ev = evaluate_dashboard_response(_draft_result(), part, expected, skill_activated=True) + assert not ev.forbidden_absent + assert not ev.strict_pass + assert any(_SUBSTITUTE in f for f in ev.failures) + + def test_the_check_is_published_only_when_the_case_carries_it(self): + """A check that could not fail is not evidence, and publishing it as passed lifts + `quality_score` above what the run earned.""" + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + plain = evaluate_dashboard_response(_draft_result(), part, _DC05_EXPECTED, skill_activated=True) + assert "forbidden_charts_absent" not in plain.strict_checks + expected = {**_DC05_EXPECTED, "must_not_contain": [{"id": _SUBSTITUTE}]} + guarded = evaluate_dashboard_response(_draft_result(), part, expected, skill_activated=True) + assert guarded.strict_checks["forbidden_charts_absent"] is True + + def test_an_id_entry_does_not_also_forbid_its_title(self): + """Titles are not unique in a real workspace -- the catalog this was written against + holds 102 visualizations whose title contains "approval rate" -- so widening an id + entry to match its title too would fail the case on a different chart.""" + part = _dashboard_part( + [ + _widget("Total Customers", _TOTAL_CUSTOMERS), + _widget("Active Customers", _ACTIVE_CUSTOMERS), + _widget("Approval Rate", _RETURN_CUSTOMERS), + ], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "must_not_contain": [{"id": _SUBSTITUTE, "title": "Approval Rate"}]} + assert evaluate_dashboard_response(_draft_result(), part, expected, skill_activated=True).forbidden_absent + + def test_a_title_only_entry_catches_a_substitute_whose_id_is_not_known_up_front(self): + part = _dashboard_part( + [ + _widget("Total Customers", _TOTAL_CUSTOMERS), + _widget("Active Customers", _ACTIVE_CUSTOMERS), + _widget("approval RATE", _SUBSTITUTE), + ], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "must_not_contain": [{"title": "Approval Rate"}]} + ev = evaluate_dashboard_response(_draft_result(), part, expected, skill_activated=True) + assert not ev.forbidden_absent + assert any(_SUBSTITUTE in f for f in ev.failures) + + def test_an_entry_naming_neither_id_nor_title_is_rejected_up_front(self): + client = MagicMock() + with pytest.raises(ValueError, match="needs an id or a title"): + _run_with(client, {**_DC05_EXPECTED, "must_not_contain": [{"columns": 6}]}) + client.send_message.assert_not_called() + + +class TestAnswerAssertions: + def test_a_missing_phrase_fails_the_case(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "answer_must_include": ["could not be covered"]} + ev = evaluate_dashboard_response( + _draft_result(), part, expected, skill_activated=True, answer_text="Here is your dashboard." + ) + assert not ev.answer_matched + assert not ev.strict_pass + + def test_the_phrase_match_ignores_case_and_whitespace(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "answer_must_include": ["Could Not Be Covered"]} + ev = evaluate_dashboard_response( + _draft_result(), + part, + expected, + skill_activated=True, + answer_text="One widget\ncould not\nbe covered by the catalog.", + ) + assert ev.answer_matched + + def test_a_forbidden_pattern_in_the_answer_fails(self): + """The raw `{visualization/}` token the renderer was meant to resolve. A run + that leaks it builds the right dashboard and still shows the user markup.""" + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = {**_DC05_EXPECTED, "answer_must_not_match": [r"\{visualization/[0-9a-f-]+\}"]} + ev = evaluate_dashboard_response( + _draft_result(), + part, + expected, + skill_activated=True, + answer_text=f"I can use {{visualization/{_SUBSTITUTE}}} for approvals.", + ) + assert not ev.answer_matched + assert any("visualization/" in f for f in ev.failures) + + def test_a_clean_answer_passes_both_kinds_of_assertion(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + expected = { + **_DC05_EXPECTED, + "answer_must_include": ["Total Customers"], + "answer_must_not_match": [r"\{visualization/"], + } + ev = evaluate_dashboard_response( + _draft_result(), + part, + expected, + skill_activated=True, + answer_text="I put Total Customers and Active Customers on the draft.", + ) + assert ev.answer_matched + assert ev.strict_pass + + def test_the_check_is_published_only_when_the_case_carries_it(self): + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + ev = evaluate_dashboard_response(_draft_result(), part, _DC05_EXPECTED, skill_activated=True) + assert "answer_matched" not in ev.strict_checks + + def test_an_invalid_regular_expression_is_rejected_up_front(self): + client = MagicMock() + with pytest.raises(ValueError, match="invalid regular expression"): + _run_with(client, {**_DC05_EXPECTED, "answer_must_not_match": ["([unclosed"]}) + client.send_message.assert_not_called() + + def test_an_empty_phrase_is_rejected_up_front(self): + """`""` is a substring of everything, so it would read as an assertion and be none.""" + client = MagicMock() + with pytest.raises(ValueError, match="asserts nothing"): + _run_with(client, {**_DC05_EXPECTED, "answer_must_include": [" "]}) + client.send_message.assert_not_called() + + def test_the_text_of_the_turn_that_drafted_is_what_gets_scored(self): + """The draft arrives with the sentence describing it, and that sentence is what the + assertions are about -- so the loop must read every turn, not only the ones that + answer without drafting.""" + part = _dashboard_part( + [_widget("Total Customers", _TOTAL_CUSTOMERS), _widget("Active Customers", _ACTIVE_CUSTOMERS)], + date_filter=_THIS_YEAR_FILTER, + ) + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("draft_dashboard", _draft_result())], + parts=[part], + text="I left one widget out because the catalog has no fraud data.", + ) + expected = {**_DC05_EXPECTED, "answer_must_include": ["no fraud data"]} + summary = _run_with(client, expected) + assert summary.best.evaluation.answer_matched + assert summary.best.evaluation.strict_pass + + +class TestRefusalCases: + """`expects_dashboard: false` -- the agent was asked for something it cannot build.""" + + _NO_DATA = { + "type": "dashboard", + "saved_dashboard_id": None, + "expects_dashboard": False, + "visualizations": [], + "answer_must_include": ["no fraud data"], + } + + def test_an_answer_with_no_draft_passes(self): + ev = evaluate_dashboard_response( + None, + None, + self._NO_DATA, + skill_activated=True, + answer_text="The workspace has no fraud data, so I cannot build that dashboard.", + ) + assert ev.strict_pass + assert ev.failures == [] + + def test_the_document_checks_are_not_published_for_a_refusal(self): + """Reporting six passes for a document that was never produced would make a refusal + read as the best-scoring case in the suite.""" + ev = evaluate_dashboard_response( + None, None, self._NO_DATA, skill_activated=True, answer_text="There is no fraud data here." + ) + assert set(ev.strict_checks) == {"dashboard_not_drafted", "dashboard_skill_activated", "answer_matched"} + + def test_building_anyway_fails(self): + """The defect this shape exists to catch: an agent that proxies the request with + whatever it could find rather than saying it cannot be met.""" + part = _dashboard_part([_widget("Total Customers", _TOTAL_CUSTOMERS)], date_filter=_ALL_TIME_FILTER) + ev = evaluate_dashboard_response( + _draft_result(), part, self._NO_DATA, skill_activated=True, answer_text="There is no fraud data here." + ) + assert not ev.strict_checks["dashboard_not_drafted"] + assert not ev.strict_pass + + def test_falling_over_silently_is_not_a_refusal(self): + """Absence of a draft alone is satisfied by an agent that crashed, which is why the + answer assertion is mandatory for this shape.""" + ev = evaluate_dashboard_response(None, None, self._NO_DATA, skill_activated=True, answer_text="") + assert ev.strict_checks["dashboard_not_drafted"] + assert not ev.answer_matched + assert not ev.strict_pass + + def test_a_rejected_draft_attempt_is_not_a_refusal(self): + """The gap the first two checks leave: `_extract_tool_result` takes only SUCCESSFUL + calls, so an agent whose draft the tool rejected leaves no result and no part and + would read as a clean refusal. It is not one -- it tried to build and a guardrail + stopped it, and scoring that as correct credits the model for the guardrail.""" + ev = evaluate_dashboard_response( + None, + None, + self._NO_DATA, + skill_activated=True, + answer_text="There is no fraud data here.", + producing_tool_called=True, + ) + assert not ev.strict_checks["dashboard_not_drafted"] + assert not ev.strict_pass + assert any("did not succeed" in f for f in ev.failures) + + def test_a_failed_draft_attempt_is_read_off_the_turn_not_the_result(self): + """End to end, because the flag is computed in the run loop rather than passed in by + a fixture: the tool call is present on the turn, its result is an error, so nothing + reaches the scorer except the fact that the call happened.""" + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("draft_dashboard", {"status": "error", "message": "no such metric"})], + text="There is no fraud data in this workspace, so I cannot build that.", + ) + summary = _run_with(client, self._NO_DATA) + assert summary.best.evaluation.producing_tool_called + assert not summary.best.evaluation.strict_pass + + def test_a_rejected_patch_call_counts_too(self): + """A refusal is a creation shape, so the case's selected tool is `draft_dashboard`. + An agent that routed to the editor instead and had its `patch_dashboard` call + rejected leaves the same empty result, so matching only the selected tool would let + it pass.""" + client = MagicMock() + client.send_message.return_value = _chat_result( + tool_calls=[_tool_call("patch_dashboard", {"status": "error", "message": "no such dashboard"})], + text="There is no fraud data in this workspace, so I cannot build that.", + ) + summary = _run_with(client, self._NO_DATA) + assert summary.best.evaluation.producing_tool_called + assert not summary.best.evaluation.strict_pass + + def test_an_answer_alone_still_passes_when_nothing_was_called(self): + """The control for the two above: the flag must not fail a genuine refusal.""" + client = MagicMock() + client.send_message.return_value = _chat_result( + text="There is no fraud data in this workspace, so I cannot build that." + ) + summary = _run_with(client, self._NO_DATA) + assert not summary.best.evaluation.producing_tool_called + assert summary.best.evaluation.strict_pass + + def test_a_refusal_case_without_an_answer_assertion_is_rejected_up_front(self): + client = MagicMock() + expected = {"type": "dashboard", "expects_dashboard": False, "visualizations": []} + with pytest.raises(ValueError, match="needs an answer assertion"): + _run_with(client, expected) + client.send_message.assert_not_called() + + def test_a_refusal_case_listing_charts_is_rejected_up_front(self): + client = MagicMock() + expected = {**self._NO_DATA, "visualizations": [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers"}]} + with pytest.raises(ValueError, match="must not list visualizations"): + _run_with(client, expected) + client.send_message.assert_not_called() + + def test_an_edit_cannot_be_a_refusal_case(self): + client = MagicMock() + expected = {**self._NO_DATA, "type": "dashboardPatch", "saved_dashboard_id": _OVERVIEW_DASHBOARD} + with pytest.raises(ValueError, match="creation shape"): + _run_with(client, expected) + client.send_message.assert_not_called() + + def test_the_simulated_user_never_argues_a_refusal_into_building(self): + """The reply this loop sends restates the charts and asks again, which is pressure to + build the very thing the case says must not be built.""" + client = MagicMock() + client.send_message.return_value = _chat_result(text="The workspace has no fraud data.") + summary = _run_with(client, self._NO_DATA) + assert client.send_message.call_count == 1 + assert summary.best.evaluation.strict_pass