From e7e1a44487fbe2d3a697b95e571f5a1edd82be5c Mon Sep 17 00:00:00 2001 From: huyenthanh09 Date: Thu, 29 Oct 2026 15:02:56 +0700 Subject: [PATCH] feat(gooddata-eval): score dashboard layout and the exact filter bar Moving or removing a widget, and removing or retyping a filter, leave nothing for the existing dashboard checks to see. `charts_matched` and `preserved_widgets` ask whether a chart is still on the dashboard, so a move that never happened, or left a second copy behind, passes, and so does a removal that never happened. `filters` ignores filters beyond the expected ones, and `tab_filters` reads only tabbed dashboards and needs every filter's key in advance, which the agent mints at run time for a new or re-added filter. Two opt-in expectation keys close that: - `layout`: per tab, the exact sections in order, each with its exact charts in order (an id, or `{"title"}` for an authored chart), and the section title when stated. Scored as `dashboard_layout_correct`. - `filter_bar`: per tab, the exact filters, the kept ones by key under `filters` and the ones with a minted key matched by content under `added`. Anything left over fails, an empty filter group included. A grouped filter counts by its own key and names its group's title as `group`; without it, the filter must stand outside every group, so a move into or out of a group fails. It reads a dashboard without tabs too. Scored as `dashboard_filter_bar_correct`. An entry without `tab` applies only to a dashboard with one tab, and must be the only entry. `tab_filters` now runs through the same check, so the two read filter groups alike; no existing item names a group, and re-scoring 103 saved `tab_filters` runs changes no verdict. A malformed entry is rejected before the first request. Without either key, nothing changes for existing items. Re-scored against 93 saved runs of the QA-29514 items on ecommerce_demo, the results match the expected outcome of every run. jira: QA-29514 risk: low Co-Authored-By: Claude Opus 5.5 --- .../core/agentic/dashboard_skill.py | 341 ++++++++++++++-- .../tests/test_agentic_dashboard_skill.py | 367 +++++++++++++++++- 2 files changed, 666 insertions(+), 42 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 3d66720d5..fdb797c09 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 @@ -313,20 +313,33 @@ def _grouped_filter_entries(dashboard: dict) -> list[tuple[str, dict, str | None if tabs else [("", dashboard.get("filters") or {})] ) - entries: list[tuple[str, dict, str | None]] = [] - for where, filters in maps: - for key, value in filters.items(): - if not isinstance(value, dict): - continue - label = where or f"filter {key!r}" - if value.get("type") != "filter_group": - entries.append((label, value, None)) - continue - group = str(value.get("title") or "") - entries.extend( - (label, member, group) for member in (value.get("filters") or {}).values() if isinstance(member, dict) - ) - return entries + return [ + (where or f"filter {entry_key!r}", value, group) + for where, filters in maps + for entry_key, _key, value, group in _members_of(filters) + ] + + +def _members_of(filters: dict) -> list[tuple[str, str, dict, str | None]]: + """``(entry key, filter key, filter, group title)`` for every filter of one filter map. + + A group's members are read in its place, each under its own key and with the group's key as + the entry key; a filter outside a group is its own entry and has no group title. + """ + members: list[tuple[str, str, dict, str | None]] = [] + for entry_key, value in filters.items(): + if not isinstance(value, dict): + continue + if value.get("type") != "filter_group": + members.append((entry_key, entry_key, value, None)) + continue + group = str(value.get("title") or "") + members.extend( + (entry_key, key, member, group) + for key, member in (value.get("filters") or {}).items() + if isinstance(member, dict) + ) + return members def _filters_of_type(dashboard: dict, filter_type: str) -> list[tuple[str, dict]]: @@ -549,6 +562,11 @@ def _matching_widgets(widgets: list[dict], new_ids: set[str], expected: dict) -> return [w for w in widgets if w.get("visualization") in new_ids and _norm(str(w.get("title") or "")) == title] +def _chart_entry(chart: str | dict) -> dict: + """A chart as ``tabs`` and ``layout`` name it -- an id, or ``{"title": ...}`` -- as a ``_matching_widgets`` entry.""" + return {"id": chart} if isinstance(chart, str) else {"id": None, **chart} + + def _has_date_bindings(expected_output: dict) -> bool: """Whether any expected chart states the date dataset its widget must follow. @@ -665,10 +683,11 @@ def _check_tabs(dashboard: dict, new_ids: set[str], expected: list[dict]) -> lis if "title" in exp and _norm(str(tab.get("title") or "")) != _norm(str(exp["title"] or "")): failures.append(f"tab {index} is titled {tab.get('title')!r}, expected {exp['title']!r}") tab_widgets = [w for section in tab.get("sections") or [] for w in section.get("widgets") or []] - for chart in exp.get("visualizations") or []: - entry = {"id": chart} if isinstance(chart, str) else {"id": None, **chart} - if not _matching_widgets(tab_widgets, new_ids, entry): - failures.append(f"tab {index} ({tab.get('title')!r}) does not hold chart {chart!r}") + failures.extend( + f"tab {index} ({tab.get('title')!r}) does not hold chart {chart!r}" + for chart in exp.get("visualizations") or [] + if not _matching_widgets(tab_widgets, new_ids, _chart_entry(chart)) + ) return failures @@ -732,27 +751,10 @@ def _check_tab_filters(dashboard: dict, expected: dict[str, dict]) -> list[str]: Exact on purpose: GDAI-2540 is a filter that turns up on a tab it never belonged to, which a check of only the expected entries would pass. Keyed by the saved dashboard's local filter id rather than by label, so two filters on the same label stay distinguishable. + + It is ``filter_bar`` with every filter known by key, so the two read filter groups the same way. """ - tabs = {tab.get("id"): tab for tab in dashboard.get("tabs") or []} - failures: list[str] = [] - for tab_id, expected_filters in expected.items(): - tab = tabs.get(tab_id) - if tab is None: - failures.append(f"the dashboard has no tab {tab_id!r}") - continue - actual = {k: v for k, v in (tab.get("filters") or {}).items() if isinstance(v, dict)} - failures.extend( - f"tab {tab_id!r} carries filter {filter_id!r}, which it should not" - for filter_id in sorted(actual.keys() - expected_filters.keys()) - ) - failures.extend( - f"tab {tab_id!r} lost filter {filter_id!r}" for filter_id in sorted(expected_filters.keys() - actual.keys()) - ) - for filter_id in sorted(expected_filters.keys() & actual.keys()): - failures.extend( - _filter_mismatch(f"tab {tab_id!r} filter {filter_id!r}", actual[filter_id], expected_filters[filter_id]) - ) - return failures + return _check_filter_bar(dashboard, [{"tab": tab_id, "filters": filters} for tab_id, filters in expected.items()]) def _filters_by_id(dashboard: dict) -> dict[str, list[tuple[str, dict]]]: @@ -819,6 +821,167 @@ def _check_preserved_widgets(widgets: list[dict], expected: list[dict]) -> list[ return failures +def _named_tab(dashboard: dict, tab_id: str | None) -> tuple[dict | None, str]: + """The tab an entry of ``layout`` or ``filter_bar`` names, and how to refer to it in a failure. + + An entry without ``tab`` is for a dashboard with one tab only, root sections included: on a + dashboard with several it would be a guess which one the case meant, so it is a failure. + """ + tabs = _tabs_of(dashboard) + if tab_id is None: + if len(tabs) != 1: + return None, f"the dashboard has {len(tabs)} tabs, so the expectation must name one" + return tabs[0], "the dashboard" + tab = next((t for t in tabs if t.get("id") == tab_id), None) + return tab, f"tab {tab_id!r}" if tab is not None else f"the dashboard has no tab {tab_id!r}" + + +def _check_layout(dashboard: dict, new_ids: set[str], expected: list[dict]) -> list[str]: + """Each named tab must hold exactly the expected sections, in order, each with exactly its charts in order. + + Exact on purpose. A move that leaves the widget where it was, places a second copy, or puts + it in the wrong section keeps every chart "on the dashboard", and so does a removal that + never happened; only the whole arrangement tells them apart. A section's ``title`` is + compared when the entry states it, ``null`` meaning untitled. Its ``visualizations`` are + always stated, ``[]`` for a section without charts. A chart is an id, or + ``{"title": ...}`` for one the agent authored. Widgets that show no visualization are not + charts and are left out on both sides. + """ + failures: list[str] = [] + for entry in expected: + tab, where = _named_tab(dashboard, entry.get("tab")) + if tab is None: + failures.append(where) + continue + sections = tab.get("sections") or [] + wanted = entry["sections"] + if len(sections) != len(wanted): + titles = ", ".join(repr(s.get("title")) for s in sections) + failures.append(f"{where} has {len(sections)} section(s) ({titles}), expected {len(wanted)}") + continue + for index, (section, want) in enumerate(zip(sections, wanted, strict=True)): + if "title" in want and _norm(str(section.get("title") or "")) != _norm(str(want["title"] or "")): + failures.append( + f"{where} section {index} is titled {section.get('title')!r}, expected {want['title']!r}" + ) + charts = [w for w in section.get("widgets") or [] if w.get("visualization")] + charts_wanted = want["visualizations"] + if len(charts) != len(charts_wanted) or not all( + _matching_widgets([w], new_ids, _chart_entry(c)) for w, c in zip(charts, charts_wanted, strict=True) + ): + shown = [w.get("visualization") for w in charts] + failures.append(f"{where} section {index} holds {shown}, expected {charts_wanted}") + return failures + + +def _check_filter_bar(dashboard: dict, expected: list[dict]) -> list[str]: + """Each named tab must carry exactly the expected filters: the kept ones by key, the new ones by content. + + ``filters`` names a filter the case knows the key of, the saved one or one the change keeps. + ``added`` holds a filter whose key the agent mints at run time -- a new one, or one it + replaced by removing and re-adding it -- each matched to a different filter that ``filters`` + did not claim. Anything left over fails: a filter that should have been removed, a duplicate, + or one copied in from elsewhere. Unlike ``tab_filters``, it reads a dashboard without tabs too. + + A grouped filter is read by its own key. Its entry names the group's title as ``group``, and + an entry without one must sit outside every group, so a filter moved into or out of a group + fails. A group none of the expected filters sits in fails like a leftover filter -- an empty + one, or one whose members should have been ungrouped -- and so does a key the tab carries twice. + """ + failures: list[str] = [] + for entry in expected: + tab, where = _named_tab(dashboard, entry.get("tab")) + if tab is None: + failures.append(where) + continue + filters = tab.get("filters") or {} + groups = { + key: str(value.get("title") or "") + for key, value in filters.items() + if isinstance(value, dict) and value.get("type") == "filter_group" + } + # Each filter by its own key, with the key of the group it sits in. + actual: dict[str, tuple[dict, str | None]] = {} + for entry_key, key, value, group in _members_of(filters): + if key in actual: + failures.append(f"{where} carries filter {key!r} more than once") + continue + actual[key] = (value, None if group is None else entry_key) + claimed: set[str | None] = set() + for filter_id, want in (entry.get("filters") or {}).items(): + got = actual.pop(filter_id, None) + if got is None: + failures.append(f"{where} lost filter {filter_id!r}") + continue + value, group_key = got + failures.extend(_filter_mismatch(f"{where} filter {filter_id!r}", value, _without_group(want))) + if _in_wanted_group(groups, group_key, want): + claimed.add(group_key) + else: + sits = "no group" if group_key is None else f"group {groups[group_key]!r}" + wanted = f"group {want['group']!r}" if "group" in want else "no group" + failures.append(f"{where} filter {filter_id!r} sits in {sits}, expected {wanted}") + added = entry.get("added") or [] + keys = list(actual) + fits = [ + [ + not _filter_mismatch("", actual[key][0], _without_group(want)) + and _in_wanted_group(groups, actual[key][1], want) + for key in keys + ] + for want in added + ] + for want, index in zip(added, _one_to_one(fits), strict=True): + if index is None: + failures.append(f"{where} carries no new filter matching {want!r}") + continue + claimed.add(actual.pop(keys[index])[1]) + failures.extend(f"{where} carries filter {key!r}, which it should not" for key in sorted(actual)) + failures.extend( + f"{where} carries filter group {key!r} ({title!r}), which holds none of the expected filters" + for key, title in sorted(groups.items()) + if key not in claimed + ) + return failures + + +def _without_group(expected: dict) -> dict: + return {k: v for k, v in expected.items() if k != "group"} + + +def _in_wanted_group(groups: dict[str, str], group_key: str | None, expected: dict) -> bool: + """Whether a filter in the group keyed ``group_key`` (``None`` for none) sits where ``expected`` puts it.""" + if group_key is None or "group" not in expected: + return group_key is None and "group" not in expected + return _norm(groups[group_key]) == _norm(expected["group"]) + + +def _one_to_one(fits: list[list[bool]]) -> list[int | None]: + """For each row, a column it fits, no column taken twice, with as many rows matched as there can be. + + First fit is not enough: an expectation that names fewer keys can take the filter a stricter + one needed and fail a correct dashboard on the order the entries are listed in. Each row + that finds every fitting column taken tries to move that column's owner to another one. + """ + owner: dict[int, int] = {} + + def claim(row: int, seen: set[int]) -> bool: + for column, fit in enumerate(fits[row]): + if fit and column not in seen: + seen.add(column) + if column not in owner or claim(owner[column], seen): + owner[column] = row + return True + return False + + for row in range(len(fits)): + claim(row, set()) + matched: list[int | None] = [None] * len(fits) + for column, row in owner.items(): + matched[row] = column + return matched + + def _date_range_text(date_range: dict | None) -> str: """Render a date range the way a user would say it, for the simulated reply. @@ -986,7 +1149,7 @@ def _validate_expectation(expected_output: dict) -> None: def _validate_layout_expectations(expected_output: dict) -> None: - """Shape checks for the opt-in keys on charts, tabs, per-tab filters and preserved widgets. + """Shape checks for the opt-in keys on charts, tabs, per-tab filters, preserved widgets, layout and filter bar. Raises: ValueError: one of them is unusable. Loud for the same reason as the rest of the @@ -1023,8 +1186,7 @@ def _validate_layout_expectations(expected_output: dict) -> None: ): raise ValueError(f"tab_filters[{tab_id!r}] must map filter ids to typed filters, got {filters!r}") for filter_id, entry in filters.items(): - if entry["type"] == "attribute_filter": - _validate_tab_attribute_filter(f"tab_filters[{tab_id!r}][{filter_id!r}]", entry) + _validate_bar_filter(f"tab_filters[{tab_id!r}][{filter_id!r}]", entry) if "date_filters" in expected_output: date_filters = expected_output["date_filters"] @@ -1057,6 +1219,87 @@ def _validate_layout_expectations(expected_output: dict) -> None: if not isinstance(widget, dict) or not widget.get("visualization") or len(widget) < 2: raise ValueError(f"a preserved widget needs its visualization and a key to guard, got {widget!r}") + if "layout" in expected_output: + for entry in _tab_entries(expected_output, "layout", {"tab", "sections"}): + sections = entry.get("sections") + if not isinstance(sections, list): + raise ValueError(f"a layout entry needs its sections as a list, got {entry!r}") + for section in sections: + if not isinstance(section, dict) or section.keys() - {"title", "visualizations"}: + raise ValueError(f"a layout section takes only title and visualizations, got {section!r}") + if section.get("title") is not None and not isinstance(section["title"], str): + raise ValueError(f"a layout section's title must be a string or null, got {section!r}") + if "visualizations" not in section: + # Missing would read as "no charts" here but as "any charts" in `tabs`. + raise ValueError(f"a layout section must list its visualizations, [] for none, got {section!r}") + charts = section["visualizations"] + if not isinstance(charts, list) or not all( + (isinstance(c, str) and c) + or (isinstance(c, dict) and c.keys() == {"title"} and isinstance(c["title"], str) and c["title"]) + for c in charts + ): + raise ValueError( + f"a layout section's visualizations must be chart ids or {{'title': ...}}, got {section!r}" + ) + + if "filter_bar" in expected_output: + for entry in _tab_entries(expected_output, "filter_bar", {"tab", "filters", "added"}): + where = f"filter_bar[{entry.get('tab')!r}]" + kept = entry.get("filters", {}) + added = entry.get("added", []) + if not isinstance(kept, dict) or not isinstance(added, list): + raise ValueError(f"{where} needs filters as a map of keys and added as a list, got {entry!r}") + named = [(f"{where}[{key!r}]", f) for key, f in kept.items()] + named += [(f"{where} added[{index}]", f) for index, f in enumerate(added)] + for label, body in named: + _validate_bar_filter(label, body) + + +def _validate_bar_filter(label: str, body: object) -> None: + """A filter ``tab_filters`` or ``filter_bar`` expects: typed, in the AAC shape, and never a group. + + A group is not a filter there: both checks read its members by their own keys, so an + expected ``filter_group`` could never match. Its members say which group they sit in. + + Raises: + ValueError: the filter is unusable. + """ + if not isinstance(body, dict) or not body.get("type"): + raise ValueError(f"{label} must be a typed filter, got {body!r}") + if body["type"] == "filter_group": + raise ValueError(f"{label} is a filter_group; list its members instead, each with group set to its title") + if "group" in body and not (isinstance(body["group"], str) and body["group"]): + raise ValueError(f"{label} group must be the title of the filter group, got {body['group']!r}") + if body["type"] == "attribute_filter": + _validate_tab_attribute_filter(label, body) + + +def _tab_entries(expected_output: dict, key: str, allowed: set[str]) -> list[dict]: + """The entries of a per-tab expectation, checked for the shape ``layout`` and ``filter_bar`` share. + + Raises: + ValueError: not a non-empty list of objects, a key outside ``allowed``, or a tab named + twice. An entry without ``tab`` is for a dashboard with one tab, so it must be the + only entry: beside another, both could name that one tab. + """ + entries = expected_output[key] + if not isinstance(entries, list) or not entries or not all(isinstance(e, dict) for e in entries): + raise ValueError(f"{key} must be a non-empty list of per-tab objects, got {entries!r}") + seen: set[str | None] = set() + for entry in entries: + unknown = sorted(entry.keys() - allowed) + if unknown: + raise ValueError(f"a {key} entry cannot state {unknown}, got {entry!r}") + tab = entry.get("tab") + if tab is not None and not (isinstance(tab, str) and tab): + raise ValueError(f"a {key} entry's tab must be a tab id, got {entry!r}") + if tab in seen: + raise ValueError(f"{key} names tab {tab!r} more than once") + seen.add(tab) + if None in seen and len(entries) > 1: + raise ValueError(f"a {key} entry without tab must be the only entry, got {entries!r}") + return entries + def _is_dataset_or_null(value: object) -> bool: return value is None or (isinstance(value, str) and bool(value)) @@ -1103,6 +1346,8 @@ class _Applies: date_filters: bool = False preserved_widgets: bool = False chart_types: bool = False + layout: bool = False + filter_bar: bool = False @classmethod def of(cls, expected_output: dict) -> _Applies: @@ -1116,6 +1361,8 @@ def of(cls, expected_output: dict) -> _Applies: date_filters="date_filters" in expected_output, preserved_widgets="preserved_widgets" in expected_output, chart_types=_has_chart_types(expected_output), + layout="layout" in expected_output, + filter_bar="filter_bar" in expected_output, ) @@ -1157,6 +1404,8 @@ class DashboardEvaluation: date_filters_correct: bool = False widgets_preserved: bool = False chart_types_correct: bool = False + layout_correct: bool = False + filter_bar_correct: bool = False failures: list[str] = field(default_factory=list) notes: list[str] = field(default_factory=list) @@ -1199,6 +1448,10 @@ def strict_checks(self) -> dict[str, bool]: checks["dashboard_widgets_preserved"] = self.widgets_preserved if self.applies.chart_types: checks["dashboard_chart_types_correct"] = self.chart_types_correct + if self.applies.layout: + checks["dashboard_layout_correct"] = self.layout_correct + if self.applies.filter_bar: + checks["dashboard_filter_bar_correct"] = self.filter_bar_correct return checks @property @@ -1414,6 +1667,8 @@ def evaluate_dashboard_response( if applies.chart_types else ([], []) ) + layout_failures = _check_layout(document, new_ids, expected_output["layout"]) if applies.layout else [] + filter_bar_failures = _check_filter_bar(document, expected_output["filter_bar"]) if applies.filter_bar else [] return DashboardEvaluation( drafted=True, @@ -1434,6 +1689,8 @@ def evaluate_dashboard_response( date_filters_correct=not date_filter_failures, widgets_preserved=not preserved_failures, chart_types_correct=not chart_type_failures, + layout_correct=not layout_failures, + filter_bar_correct=not filter_bar_failures, failures=[ *chart_failures, *saved_failures, @@ -1445,6 +1702,8 @@ def evaluate_dashboard_response( *date_filter_failures, *preserved_failures, *chart_type_failures, + *layout_failures, + *filter_bar_failures, *new_failures, ], notes=[*title_notes, *reference_notes, *chart_type_notes], diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index a2c3559ce..94a39a970 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -1477,8 +1477,31 @@ def test_a_tab_that_is_not_there_fails(self): expected = {**_TABBED_EDIT, "tab_filters": {"tab_finance": {}}} assert any("has no tab 'tab_finance'" in f for f in _score_tabbed_edit(expected, _RESIZE_AMOUNT).failures) + def test_a_group_is_read_as_filter_bar_reads_it(self): + """By its members' own keys, so the two exact filter checks agree on the same dashboard.""" + expected = { + **_TABBED_EDIT, + "tab_filters": { + "tab_products": { + "tab2_date": _TAB2_DATE, + "category_filter": _GROUPED_CATEGORY, + "product_brand": {**_BRAND, "group": "Product"}, + } + }, + } + assert _score_tabbed_edit(expected, _GROUP_CATEGORY_WITH_BRAND).tab_filters_correct + ev = self._score(_GROUP_CATEGORY_WITH_BRAND) + assert "tab 'tab_products' filter 'category_filter' sits in group 'Product', expected no group" in ev.failures + assert "tab 'tab_products' carries filter 'product_brand', which it should not" in ev.failures + @pytest.mark.parametrize( - "tab_filters", [{}, {"tab_customers": []}, {"tab_customers": {"main_date": {"granularity": "MONTH"}}}] + "tab_filters", + [ + {}, + {"tab_customers": []}, + {"tab_customers": {"main_date": {"granularity": "MONTH"}}}, + {"tab_products": {"product": {"type": "filter_group", "title": "Product", "filters": {}}}}, + ], ) def test_a_malformed_expectation_is_rejected_up_front(self, tab_filters): client = MagicMock() @@ -1633,6 +1656,339 @@ def test_a_malformed_expectation_is_rejected_up_front(self, preserved): client.send_message.assert_not_called() +_LAYOUT_UNTOUCHED = [ + {"tab": "tab_customers", "sections": [{"title": "Overview", "visualizations": [_NET_SALES, _TOTAL_CUSTOMERS]}]}, + {"tab": "tab_products", "sections": [{"title": None, "visualizations": [_ACTIVE_CUSTOMERS]}]}, +] +# A move between tabs as the agent emits it: placed on the target tab, then taken off the source. +_MOVE_NET_SALES_TO_PRODUCTS = [ + {"op": "test", "path": "/tabs/1/sections/0/widgets/0/visualization", "value": _ACTIVE_CUSTOMERS}, + { + "op": "add", + "path": "/tabs/1/sections/0/widgets/1", + "value": {"visualization": _NET_SALES, "title": False, "columns": 6, "rows": 12, "interactions": _DRILL}, + }, + {"op": "test", "path": "/tabs/0/sections/0/widgets/0/visualization", "value": _NET_SALES}, + {"op": "remove", "path": "/tabs/0/sections/0/widgets/0"}, +] +_LAYOUT_AFTER_MOVE = [ + {"tab": "tab_customers", "sections": [{"title": "Overview", "visualizations": [_TOTAL_CUSTOMERS]}]}, + {"tab": "tab_products", "sections": [{"title": None, "visualizations": [_ACTIVE_CUSTOMERS, _NET_SALES]}]}, +] + + +def _two_section_base() -> dict: + """A saved single-tab dashboard as relayed for editing: version 2, sections at the root.""" + part = _edit_base_part([_widget("Net Sales", _NET_SALES), _widget("Activity by Hour", _ACTIVITY_BY_HOUR)]) + part["dashboard"]["sections"].append({"title": "Orders", "widgets": [_widget("Order status", _ORDER_STATUS)]}) + part["references"]["visualizations"].append({"id": _ORDER_STATUS, "type": "funnel_chart", "title": "Order status"}) + return part + + +def _score_root_edit(expected: dict, operations: list[dict], new_visualizations: list[str] | None = None): + patch_part = _patch_part(operations, new_visualizations=new_visualizations) + return evaluate_dashboard_response( + _patch_result(len(new_visualizations or [])), _two_section_base(), expected, True, patch_part=patch_part + ) + + +_ROOT_EDIT = {**_DE01_EXPECTED, "visualizations": [{"id": _NET_SALES, "title": "Net Sales"}]} + + +class TestLayout: + """A move or a removal is only visible in the whole arrangement: every chart staying "on the + dashboard" also passes a move that never happened or left a second copy behind.""" + + def test_a_move_between_tabs_matches(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "layout": _LAYOUT_AFTER_MOVE}, _MOVE_NET_SALES_TO_PRODUCTS) + assert ev.strict_checks["dashboard_layout_correct"] is True + assert ev.strict_pass, ev.failures + + def test_a_move_that_leaves_the_original_behind_fails(self): + """Placing alone adds a second copy -- the mistake gen-ai's editor prompt warns about.""" + ev = _score_tabbed_edit({**_TABBED_EDIT, "layout": _LAYOUT_AFTER_MOVE}, _MOVE_NET_SALES_TO_PRODUCTS[:2]) + assert not ev.layout_correct + assert any("tab 'tab_customers' section 0 holds" in f for f in ev.failures) + + def test_a_move_that_never_happened_fails(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "layout": _LAYOUT_AFTER_MOVE}, _RESIZE_AMOUNT) + assert not ev.layout_correct + + def test_a_reorder_is_read_in_order(self): + layout = [{"tab": "tab_customers", "sections": [{"visualizations": [_TOTAL_CUSTOMERS, _NET_SALES]}]}] + swap = [{"op": "move", "from": "/tabs/0/sections/0/widgets/1", "path": "/tabs/0/sections/0/widgets/0"}] + assert _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, swap).layout_correct + assert not _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, _RESIZE_AMOUNT).layout_correct + + def test_only_the_named_tabs_are_compared(self): + layout = [_LAYOUT_UNTOUCHED[1]] + assert _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, _RESIZE_AMOUNT).layout_correct + + def test_removing_the_last_widget_of_a_section_removes_the_section(self): + expected = { + **_ROOT_EDIT, + "layout": [ + {"sections": [{"title": "Products overview", "visualizations": [_NET_SALES, _ACTIVITY_BY_HOUR]}]} + ], + } + removed = [{"op": "test", "path": "/sections/1/widgets/0/visualization", "value": _ORDER_STATUS}] + assert _score_root_edit(expected, [*removed, {"op": "remove", "path": "/sections/1"}]).strict_pass + ev = _score_root_edit(expected, [*removed, {"op": "remove", "path": "/sections/1/widgets/0"}]) + assert not ev.layout_correct + assert any("the dashboard has 2 section(s)" in f for f in ev.failures) + + def test_a_section_title_is_compared_when_stated(self): + new_section = {"title": "Order health", "widgets": [_widget("Order status", _ORDER_STATUS)]} + operations = [ + {"op": "remove", "path": "/sections/1"}, + {"op": "add", "path": "/sections/-", "value": new_section}, + ] + layout = [ + { + "sections": [ + {"visualizations": [_NET_SALES, _ACTIVITY_BY_HOUR]}, + {"title": "order HEALTH", "visualizations": [_ORDER_STATUS]}, + ] + } + ] + assert _score_root_edit({**_ROOT_EDIT, "layout": layout}, operations).layout_correct + layout[0]["sections"][1]["title"] = "Orders" + ev = _score_root_edit({**_ROOT_EDIT, "layout": layout}, operations) + assert any("section 1 is titled 'Order health', expected 'Orders'" in f for f in ev.failures) + + def test_an_authored_chart_is_matched_by_title(self): + authored = "b5c2f8d4-0000-4000-8000-000000000001" + operations = [{"op": "add", "path": "/sections/1/widgets/-", "value": _widget("Returns by Category", authored)}] + layout = [ + { + "sections": [ + {"visualizations": [_NET_SALES, _ACTIVITY_BY_HOUR]}, + {"visualizations": [_ORDER_STATUS, {"title": "Returns by category"}]}, + ] + } + ] + expected = {**_ROOT_EDIT, "layout": layout} + assert _score_root_edit(expected, operations, new_visualizations=[authored]).layout_correct + # The same title on a chart that already existed is not the authored one. + assert not _score_root_edit(expected, operations).layout_correct + + def test_an_entry_without_a_tab_on_a_tabbed_dashboard_fails(self): + layout = [{"sections": _LAYOUT_UNTOUCHED[0]["sections"]}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, _RESIZE_AMOUNT) + assert any("the dashboard has 2 tabs, so the expectation must name one" in f for f in ev.failures) + + @pytest.mark.parametrize( + "layout", + [ + [], + [{"sections": "Overview"}], + [{"sections": [{"name": "Overview"}]}], + [{"sections": [{"visualizations": [{"id": _NET_SALES}]}]}], + [{"sections": [{"title": 3, "visualizations": []}]}], + # A chart title that is not text would crash the run instead of failing here. + [{"sections": [{"visualizations": [{"title": 3}]}]}], + # Without visualizations, `layout` would read "no charts" where `tabs` reads "any". + [{"sections": [{"title": "Orders"}]}], + [{"tab": "tab_customers", "sections": []}, {"tab": "tab_customers", "sections": []}], + # On a one-tab dashboard both entries could name that tab. + [{"sections": []}, {"tab": "tab_customers", "sections": []}], + [{"tabs": "tab_customers", "sections": []}], + ], + ) + def test_a_malformed_expectation_is_rejected_up_front(self, layout): + client = MagicMock() + with pytest.raises(ValueError, match="layout"): + _run_with(client, {**_TABBED_EDIT, "layout": layout}) + client.send_message.assert_not_called() + + +_CUSTOMER_CONTAINS = {"type": "text_filter", "using": "label/customer_name", "condition": "contains", "value": "Aaron"} +# A list filter turned into a text filter as the agent emits it: removed, then added under a minted key. +_CUSTOMER_TO_TEXT = [ + {"op": "test", "path": "/tabs/0/filters/customer_filter", "value": _CUSTOMER_BY_NAME}, + {"op": "remove", "path": "/tabs/0/filters/customer_filter"}, + {"op": "add", "path": "/tabs/0/filters/customer_name", "value": _CUSTOMER_CONTAINS}, +] +_BAR_AFTER_TEXT = [ + { + "tab": "tab_customers", + "filters": {"main_date": _MAIN_DATE, "second_date": _SECOND_DATE}, + "added": [_CUSTOMER_CONTAINS], + }, + {"tab": "tab_products", "filters": {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL}}, +] +_BRAND = {"type": "attribute_filter", "using": "label/product_brand"} +_GROUPED_CATEGORY = {**_CATEGORY_ALL, "group": "Product"} +# The category filter taken off the bar and put in a new group beside a new brand filter. +_GROUP_CATEGORY_WITH_BRAND = [ + {"op": "remove", "path": "/tabs/1/filters/category_filter"}, + { + "op": "add", + "path": "/tabs/1/filters/product", + "value": { + "type": "filter_group", + "title": "Product", + "filters": {"category_filter": _CATEGORY_ALL, "product_brand": _BRAND}, + }, + }, +] + + +class TestFilterBar: + """Exact filters per tab, like ``tab_filters``, but able to hold a filter whose key the agent + mints at run time, and to read a dashboard without tabs.""" + + def test_a_list_filter_turned_into_a_text_filter_matches(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": _BAR_AFTER_TEXT}, _CUSTOMER_TO_TEXT) + assert ev.strict_checks["dashboard_filter_bar_correct"] is True + assert ev.strict_pass, ev.failures + + def test_a_change_that_keeps_the_key_matches_too(self): + operations = [{"op": "replace", "path": "/tabs/0/filters/customer_filter", "value": _CUSTOMER_CONTAINS}] + assert _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": _BAR_AFTER_TEXT}, operations).filter_bar_correct + + def test_the_list_filter_left_beside_the_new_one_fails(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": _BAR_AFTER_TEXT}, _CUSTOMER_TO_TEXT[2:]) + assert not ev.filter_bar_correct + assert any( + "tab 'tab_customers' carries filter 'customer_filter', which it should not" in f for f in ev.failures + ) + + def test_a_new_filter_that_is_not_there_fails(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": _BAR_AFTER_TEXT}, _CUSTOMER_TO_TEXT[:2]) + assert any("tab 'tab_customers' carries no new filter matching" in f for f in ev.failures) + + def test_a_new_filter_added_twice_fails(self): + twice = [ + *_CUSTOMER_TO_TEXT, + {"op": "add", "path": "/tabs/0/filters/customer_name_2", "value": _CUSTOMER_CONTAINS}, + ] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": _BAR_AFTER_TEXT}, twice) + assert any("carries filter 'customer_name_2', which it should not" in f for f in ev.failures) + + def test_a_new_filter_on_the_wrong_tab_fails(self): + operations = [{"op": "add", "path": "/tabs/1/filters/customer_name", "value": _CUSTOMER_CONTAINS}] + bar = [ + { + "tab": "tab_customers", + "filters": {"main_date": _MAIN_DATE, "second_date": _SECOND_DATE, "customer_filter": _CUSTOMER_BY_NAME}, + "added": [_CUSTOMER_CONTAINS], + }, + _BAR_AFTER_TEXT[1], + ] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, operations) + assert any("tab 'tab_customers' carries no new filter" in f for f in ev.failures) + assert any("tab 'tab_products' carries filter 'customer_name'" in f for f in ev.failures) + + def test_a_removed_filter_on_a_dashboard_without_tabs(self): + """What `filters` cannot see: it ignores filters beyond the expected ones.""" + kept = {k: v for k, v in _SAVED_FILTERS.items() if k != "1b0255ea4bfd4f11a832a8649abd73b2"} + expected = {**_ROOT_EDIT, "filter_bar": [{"filters": kept}]} + removal = [{"op": "remove", "path": "/filters/1b0255ea4bfd4f11a832a8649abd73b2"}] + assert _score_root_edit(expected, removal).strict_pass + ev = _score_root_edit(expected, [{"op": "add", "path": "/sections/0/widgets/0/columns", "value": 12}]) + assert any("the dashboard carries filter '1b0255ea4bfd4f11a832a8649abd73b2'" in f for f in ev.failures) + + def test_a_changed_selection_is_compared(self): + country = "c47fea61aa7247cca2bab2096ce6e297" + canada = {"type": "attribute_filter", "using": "label/customer_country", "state": {"include": ["Canada"]}} + expected = {**_ROOT_EDIT, "filter_bar": [{"filters": {**_SAVED_FILTERS, country: canada}}]} + assert _score_root_edit( + expected, [{"op": "replace", "path": f"/filters/{country}", "value": canada}] + ).strict_pass + ev = _score_root_edit(expected, [{"op": "add", "path": "/sections/0/widgets/0/columns", "value": 12}]) + assert any( + f"the dashboard filter {country!r} selects all, expected include ['Canada']" in f for f in ev.failures + ) + + def test_a_groups_members_are_read_by_their_own_keys_and_name_the_group(self): + bar = [ + { + "tab": "tab_products", + "filters": {"tab2_date": _TAB2_DATE, "category_filter": {**_CATEGORY_ALL, "group": "product"}}, + "added": [{**_BRAND, "group": "Product"}], + } + ] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, _GROUP_CATEGORY_WITH_BRAND) + assert ev.filter_bar_correct, ev.failures + + def test_a_filter_moved_into_a_group_fails_when_it_should_stand_alone(self): + bar = [ + { + "tab": "tab_products", + "filters": {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL}, + "added": [_BRAND], + } + ] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, _GROUP_CATEGORY_WITH_BRAND) + assert not ev.filter_bar_correct + assert "tab 'tab_products' filter 'category_filter' sits in group 'Product', expected no group" in ev.failures + assert any("carries no new filter matching" in f for f in ev.failures) + assert any("carries filter group 'product' ('Product'), which holds none" in f for f in ev.failures) + + def test_a_filter_left_outside_its_group_fails(self): + bar = [{"tab": "tab_products", "filters": {"tab2_date": _TAB2_DATE, "category_filter": _GROUPED_CATEGORY}}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, _RESIZE_AMOUNT) + assert "tab 'tab_products' filter 'category_filter' sits in no group, expected group 'Product'" in ev.failures + + def test_a_filter_both_grouped_and_left_on_its_own_fails(self): + """Grouped without being taken off the bar: shown twice, under one key.""" + group = {"type": "filter_group", "title": "Product", "filters": {"category_filter": _CATEGORY_ALL}} + operations = [{"op": "add", "path": "/tabs/1/filters/product", "value": group}] + bar = [{"tab": "tab_products", "filters": {"tab2_date": _TAB2_DATE, "category_filter": _GROUPED_CATEGORY}}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, operations) + assert "tab 'tab_products' carries filter 'category_filter' more than once" in ev.failures + + def test_an_empty_group_fails(self): + empty = {"type": "filter_group", "title": "Product", "filters": {}} + operations = [{"op": "add", "path": "/tabs/1/filters/product", "value": empty}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": [_BAR_AFTER_TEXT[1]]}, operations) + assert ev.failures == [ + "tab 'tab_products' carries filter group 'product' ('Product'), which holds none of the expected filters" + ] + + def test_added_entries_match_whatever_order_they_are_listed_in(self): + """A looser entry listed first must not take the filter only the stricter one fits.""" + aaron = _CUSTOMER_CONTAINS + bella = {**_CUSTOMER_CONTAINS, "value": "Bella"} + operations = [ + {"op": "add", "path": "/tabs/1/filters/customer_aaron", "value": aaron}, + {"op": "add", "path": "/tabs/1/filters/customer_bella", "value": bella}, + ] + loose = {"type": "text_filter", "using": "label/customer_name"} + for added in ([loose, aaron], [aaron, loose]): + bar = [{**_BAR_AFTER_TEXT[1], "added": added}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": bar}, operations) + assert ev.filter_bar_correct, (added, ev.failures) + + def test_an_entry_without_a_tab_on_a_tabbed_dashboard_fails(self): + ev = _score_tabbed_edit({**_TABBED_EDIT, "filter_bar": [{"filters": {}}]}, _RESIZE_AMOUNT) + assert any("must name one" in f for f in ev.failures) + + @pytest.mark.parametrize( + ("bar", "match"), + [ + ([], "filter_bar must be a non-empty list"), + ([{"filters": []}], "needs filters as a map"), + ([{"added": {}}], "needs filters as a map"), + ([{"filters": {"main_date": {"granularity": "MONTH"}}}], "must be a typed filter"), + ([{"added": [{"type": "attribute_filter", "using": "label/x", "include": ["a"]}]}], r"puts \['include'\]"), + ([{"tab": "tab_products"}, {"tab": "tab_products"}], "more than once"), + ([{"filters": {}}, {"tab": "tab_products"}], "without tab must be the only entry"), + ([{"tab": 3, "filters": {}}], "tab must be a tab id"), + ([{"filters": {}, "kept": {}}], r"cannot state \['kept'\]"), + # The check reads a group's members by their own keys, so a group could never match. + ([{"filters": {"product": {"type": "filter_group", "filters": {}}}}], "is a filter_group"), + ([{"added": [{**_BRAND, "group": ""}]}], "group must be the title"), + ], + ) + def test_a_malformed_expectation_is_rejected_up_front(self, bar, match): + client = MagicMock() + with pytest.raises(ValueError, match=match): + _run_with(client, {**_TABBED_EDIT, "filter_bar": bar}) + client.send_message.assert_not_called() + + class TestFilterKinds: """A drafted dashboard can carry a filter group, a text filter and a metric value filter, in the shapes gen-ai's draft tool writes them.""" @@ -1869,6 +2225,8 @@ def test_a_run_with_nothing_to_score_fails_every_check_the_case_applies(self): "tab_filters": {"tab_customers": {}}, "date_filters": {"main_date": None}, "preserved_widgets": [{"visualization": _NET_SALES, "title": False}], + "layout": _LAYOUT_UNTOUCHED, + "filter_bar": [{"tab": "tab_customers"}], } ev = evaluate_dashboard_response(None, None, expected, True) for name in ( @@ -1877,9 +2235,16 @@ def test_a_run_with_nothing_to_score_fails_every_check_the_case_applies(self): "dashboard_tab_filters_correct", "dashboard_date_filters_correct", "dashboard_widgets_preserved", + "dashboard_layout_correct", + "dashboard_filter_bar_correct", ): assert ev.strict_checks[name] is False + def test_a_case_without_layout_or_filter_bar_publishes_neither(self): + ev = _score_tabbed_edit(_TABBED_EDIT, _RESIZE_AMOUNT) + assert "dashboard_layout_correct" not in ev.strict_checks + assert "dashboard_filter_bar_correct" not in ev.strict_checks + class TestRunLoop: def test_a_first_turn_draft_never_reaches_the_simulated_user(self):