From e22419eff4e9e5f8eb8d811604dcfc63cd07cabb Mon Sep 17 00:00:00 2001 From: huyenthanh09 Date: Wed, 7 Oct 2026 14:50:07 +0700 Subject: [PATCH] test(gooddata-eval): extend evaluator for dashboard builder skill JIRA: QA-29563 risk: nonprod --- .../core/agentic/dashboard_skill.py | 592 ++++++++++++- .../tests/test_agentic_dashboard_skill.py | 779 ++++++++++++++++++ 2 files changed, 1341 insertions(+), 30 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 c1851383b..a5854ff82 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 @@ -295,17 +295,36 @@ def _filter_entries(dashboard: dict) -> list[tuple[str, dict]]: local identifier, so the role has to be read from each entry's ``type`` instead. The label is only there to say *where* a failure was found. """ + return [(label, value) for label, value, _group in _grouped_filter_entries(dashboard)] + + +def _grouped_filter_entries(dashboard: dict) -> list[tuple[str, dict, str | None]]: + """``(label, filter, group title)`` for every filter, with a group's members read in its place. + + A ``filter_group`` holds its members in a nested map, so reading only the outer map would + miss every filter the agent chose to group and report it missing. The group entry itself is + not a filter and is not returned. + """ tabs = dashboard.get("tabs") - if tabs: - return [ - (f"tab {tab.get('id')!r}", value) - for tab in tabs - for value in (tab.get("filters") or {}).values() - if isinstance(value, dict) - ] - return [ - (f"filter {key!r}", value) for key, value in (dashboard.get("filters") or {}).items() if isinstance(value, dict) - ] + maps = ( + [(f"tab {tab.get('id')!r}", tab.get("filters") or {}) for tab in tabs] + 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 def _filters_of_type(dashboard: dict, filter_type: str) -> list[tuple[str, dict]]: @@ -329,27 +348,29 @@ def _date_filter_mismatch(label: str, date_filter: dict, expected: dict | None) ] -def _check_date_range(dashboard: dict, expected: dict | None) -> list[str]: +def _check_date_range(dashboard: dict, expected: dict | None, is_edit: bool) -> list[str]: """Check the dashboard's date range against the expectation. - What counts as "the" date filter differs by shape, so the two are checked differently. + What counts as "the" date filter differs between a drafted and a saved dashboard, so the two + are checked differently. The case says which one it is: a saved dashboard can carry tabs as + well, so the document's shape cannot. A drafted dashboard carries one per tab, and the draft tool writes the same one onto every tab, so every tab must have it and every one must match. A tab left without a date filter is a failure: the expectation describes what the whole dashboard shows. - A saved dashboard relayed for editing carries a flat map that can legitimately hold more - than one date filter — a dashboard-wide one plus a dataset-scoped one — and nothing in the - document says which is which. Requiring every entry to match would fail a dashboard for - carrying a filter the fixture never described, so one matching entry satisfies the check and - the rest are left to the notes. + A saved dashboard relayed for editing, tabbed or flat, can legitimately hold more than one + date filter — a dashboard-wide one plus a dataset-scoped one. Requiring every entry to match + would fail a dashboard for carrying a filter the fixture never described, so one matching + entry satisfies the check and the rest are left to the notes. A case that cares which filter + holds which range names them in ``date_filters``. """ date_filters = _filters_of_type(dashboard, "date_filter") if not date_filters: return ["the dashboard carries no date filter to check the expected range against"] tabs = dashboard.get("tabs") - if tabs: + if tabs and not is_edit: failures: list[str] = [ f"tab {tab.get('id')!r} carries no date filter" for tab in tabs @@ -386,24 +407,55 @@ def _describe_selection(selection: tuple[str, list] | None) -> str: def _check_filters(dashboard: dict, expected: list[dict]) -> list[str]: - """Every expected attribute filter must be on the dashboard, with the stated selection. + """Every expected filter must be on the dashboard, with the stated selection or condition. + + An entry is an attribute filter unless it names ``type`` as ``text_filter`` or + ``metric_value_filter``. ``group`` requires the filter to sit in the ``filter_group`` of that + title; without it, a grouped filter counts the same as one on its own. This exists because an edit can silently drop or widen the filters the user already had: the change asked for was a rename, and the filters coming back untouched is part of what "untouched" means. Filters the dashboard carries beyond the expected ones are left alone, the same way extra widgets are. """ - actual: dict[str | None, list[dict]] = {} - for _label, attribute_filter in _filters_of_type(dashboard, "attribute_filter"): - actual.setdefault(attribute_filter.get("using"), []).append(attribute_filter) - + entries = _grouped_filter_entries(dashboard) failures: list[str] = [] for exp in expected: + filter_type = _expected_filter_type(exp) using = exp.get("using") - matches = actual.get(using) or [] - if not matches: - failures.append(f"the dashboard carries no attribute filter on {using!r}") + candidates = [ + (f, group) + for _label, f, group in entries + if f.get("type") == filter_type and _same_object_ref(f.get("using"), using) + ] + noun = filter_type.replace("_", " ") + if not candidates: + failures.append(f"the dashboard carries no {noun} on {using!r}") continue + if "group" in exp: + grouped = [ + (f, group) for f, group in candidates if group is not None and _norm(group) == _norm(exp["group"]) + ] + if not grouped: + found = ", ".join(repr(group) for _f, group in candidates) + failures.append(f"{noun} on {using!r} sits in group {found}, expected {exp['group']!r}") + continue + candidates = grouped + matches = [f for f, _group in candidates] + if filter_type != "attribute_filter": + if not any(not _filter_value_mismatch(f, exp) for f in matches): + failures.append(f"{noun} on {using!r}: {'; '.join(_filter_value_mismatch(matches[0], exp))}") + continue + if "display_as" in exp: + # GDAI-2488: a value change on a filter shown through a secondary label moves `using` + # to the primary label and keeps the secondary one as `display_as`. Losing it shows + # the user ids where they had names, while `using` and the selection stay right. + shown = [f for f in matches if _same_object_ref(f.get("display_as"), exp["display_as"])] + if not shown: + found = ", ".join(repr(f.get("display_as")) for f in matches) + failures.append(f"filter on {using!r} is displayed as {found}, expected {exp['display_as']!r}") + continue + matches = shown wanted = _expected_selection(exp) if not any(_selection_of(f) == wanted for f in matches): found = ", ".join(_describe_selection(_selection_of(f)) for f in matches) @@ -411,6 +463,59 @@ def _check_filters(dashboard: dict, expected: list[dict]) -> list[str]: return failures +# The kinds a `filters` entry can name, and for each the keys compared beyond `using`. An entry +# without `type` is an attribute filter, which is every entry written before the others existed. +_FILTER_VALUE_KEYS: dict[str, tuple[str, ...]] = { + "attribute_filter": (), + "text_filter": ("condition", "value", "values", "case_sensitive"), + "metric_value_filter": ("conditions", "dimensionality", "null_values_as_zero"), +} +# What an attribute filter entry is compared on beyond `using`: its display label and selection. +_ATTRIBUTE_FILTER_KEYS = ("display_as", "include", "exclude", "selection") + + +def _expected_filter_type(expected_filter: dict) -> str: + return str(expected_filter.get("type") or "attribute_filter") + + +def _filter_value_mismatch(actual: dict, expected: dict) -> list[str]: + """Why a text or metric value filter differs from the expectation, on the keys it states. + + ``values`` compares without order: the order of whole values for ``is``/``isNot`` carries no + meaning. A key the expectation leaves out is not compared, so a case states what it guards. + """ + mismatches: list[str] = [] + for key in _FILTER_VALUE_KEYS[_expected_filter_type(expected)]: + if key not in expected: + continue + want, got = expected[key], actual.get(key) + same = ( + _same_unordered_values(want, got) + if key == "values" and isinstance(want, list) and isinstance(got, list) + else got == want + ) + if not same: + mismatches.append(f"{key} expected {want!r}, got {got!r}") + return mismatches + + +def _same_unordered_values(want: list, got: list) -> bool: + """One-to-one match in any order, with each pair of the same type. + + Not sorted: AAC allows `null` among the values, and Python cannot order it with strings. + Not stringified: `1` and `"1"` are different values. + """ + remaining = list(got) + for expected in want: + index = next( + (i for i, actual in enumerate(remaining) if type(actual) is type(expected) and actual == expected), None + ) + if index is None: + return False + del remaining[index] + return not remaining + + def _expected_selection(expected_filter: dict) -> tuple[str, list] | None: """Read a fixture's filter entry into the same shape ``_selection_of`` produces. @@ -429,6 +534,231 @@ def _expected_selection(expected_filter: dict) -> tuple[str, list] | None: ) +def _matching_widgets(widgets: list[dict], new_ids: set[str], expected: dict) -> list[dict]: + """The widgets an expected chart entry names: by ``id``, or by title among the authored charts. + + Every widget the id appears under counts, whatever its title -- the checks that use this + look at what a chart is bound to, and a rename is ``charts_matched``'s business. + """ + exp_id = expected.get("id") + if exp_id is not None: + return [w for w in widgets if w.get("visualization") == exp_id] + title = _norm(str(expected.get("title") or "")) + return [w for w in widgets if w.get("visualization") in new_ids and _norm(str(w.get("title") or "")) == title] + + +def _has_date_bindings(expected_output: dict) -> bool: + """Whether any expected chart states the date dataset its widget must follow. + + Per chart, not per case: a chart that reaches no date dataset correctly ignores the date + filter, so "every widget has a date" is not an expectation that holds in general. + """ + return any(isinstance(v, dict) and "date_dataset" in v for v in expected_output.get("visualizations") or []) + + +def _check_date_bindings(widgets: list[dict], new_ids: set[str], expected: list[dict]) -> list[str]: + """Every widget of a chart that states a ``date_dataset`` must be bound to exactly that one. + + ``null`` means the widget must carry no date at all: the chart reaches no date dataset, and + a binding there would be one the dashboard cannot honour. The widget key is AAC's ``date``, + the declarative ``dateDataSet`` (GDAI-2391, GDAI-2462). + """ + failures: list[str] = [] + for exp in expected: + if not isinstance(exp, dict) or "date_dataset" not in exp: + continue + wanted = exp["date_dataset"] + label = exp.get("title") if exp.get("id") is None else exp.get("id") + candidates = _matching_widgets(widgets, new_ids, exp) + if not candidates: + # Not left to charts_matched alone: a check that could not look must not read as passed. + failures.append(f"cannot check the date binding of chart {label!r}: it is not on the dashboard") + continue + # Every placement: the expectation names the chart, so one right placement must not hide a wrong one. + if not all(w.get("date") == wanted for w in candidates): + found = ", ".join(repr(w.get("date")) for w in candidates) + failures.append(f"chart {label!r} follows date {found}, expected {wanted!r}") + return failures + + +def _tabs_of(dashboard: dict) -> list[dict]: + """The dashboard's tabs, reading a document with root sections as its single tab.""" + tabs = dashboard.get("tabs") + if tabs: + return list(tabs) + return [{"sections": dashboard.get("sections") or [], "filters": dashboard.get("filters") or {}}] + + +def _check_tabs(dashboard: dict, new_ids: set[str], expected: list[dict]) -> list[str]: + """The dashboard must have exactly the expected tabs, in order (GDAI-2426). + + A tab entry may name a ``title``, compared like every other title, and ``visualizations`` + that must sit on that tab -- each an id, or ``{"title": ...}`` for an authored chart. An + empty entry asserts only that the tab exists, which is how a case says "one tab, any name". + """ + tabs = _tabs_of(dashboard) + if len(tabs) != len(expected): + titles = ", ".join(repr(t.get("title")) for t in tabs) + return [f"the dashboard has {len(tabs)} tab(s) ({titles}), expected {len(expected)}"] + failures: list[str] = [] + for index, (tab, exp) in enumerate(zip(tabs, expected)): + 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}") + return failures + + +# The keys a date filter is compared on. Absent on both sides is a match -- an all-time filter +# spells itself by leaving `from` and `to` out, and a dashboard-wide one by leaving `date` out. +_DATE_FILTER_KEYS = ("granularity", "from", "to", "date") + + +def _filter_mismatch(label: str, actual: dict, expected: dict) -> list[str]: + """Why ``actual`` is not the ``expected`` filter, or nothing. + + A date filter is compared on its range and dataset, an attribute filter on its labels and + selection, both with absence significant. Any other kind is compared on the keys the + expectation names. + """ + if actual.get("type") != expected.get("type"): + return [f"{label} is a {actual.get('type')!r}, expected a {expected.get('type')!r}"] + if expected.get("type") == "date_filter": + keys: tuple[str, ...] = _DATE_FILTER_KEYS + elif expected.get("type") == "attribute_filter": + if _selection_of(actual) != _selection_of(expected): + return [ + f"{label} selects {_describe_selection(_selection_of(actual))}, " + f"expected {_describe_selection(_selection_of(expected))}" + ] + keys = ("using", "display_as") + else: + keys = tuple(k for k in expected if k != "type") + return [ + f"{label}: {key} expected {expected.get(key)!r}, got {actual.get(key)!r}" + for key in keys + if not ( + _same_object_ref(actual.get(key), expected.get(key)) + if key in ("using", "display_as") + else actual.get(key) == expected.get(key) + ) + ] + + +def _same_object_ref(actual: object, expected: object) -> bool: + """Whether two AAC refs name the same object, reading ``attribute/`` as ``label/``. + + The draft and patch tools accept either prefix on a filter's ``using``, and the model picks + one per run. An attribute and its primary label share the id, so the two spell one filter; + matching the prefix verbatim failed a correct run on the model's wording. + """ + if actual == expected: + return True + if not (isinstance(actual, str) and isinstance(expected, str)): + return False + return _ref_without_attribute_prefix(actual) == _ref_without_attribute_prefix(expected) + + +def _ref_without_attribute_prefix(ref: str) -> str: + return "label/" + ref.removeprefix("attribute/") if ref.startswith("attribute/") else ref + + +def _check_tab_filters(dashboard: dict, expected: dict[str, dict]) -> list[str]: + """Each named tab must carry exactly the expected filters, by the filter's own id. + + 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. + """ + 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 + + +def _filters_by_id(dashboard: dict) -> dict[str, list[tuple[str, dict]]]: + """Every filter by its own id, with where it was found; one id can recur across tabs.""" + found: dict[str, list[tuple[str, dict]]] = {} + for tab in dashboard.get("tabs") or [{"filters": dashboard.get("filters") or {}}]: + where = f"tab {tab['id']!r} " if "id" in tab else "" + for filter_id, value in (tab.get("filters") or {}).items(): + if isinstance(value, dict): + found.setdefault(filter_id, []).append((f"{where}filter {filter_id!r}", value)) + return found + + +def _check_date_filters(dashboard: dict, expected: dict[str, dict | None]) -> list[str]: + """Every named date filter must still be there, on the expected range (GDAI-2491). + + Named by id because a dashboard can carry more than one date filter and the case states + which one holds which range. Filters the expectation does not name are left alone. + + ``date`` is the dataset the filter itself is pinned to, not a widget's date binding, and its + absence is significant as in ``tab_filters``: an entry without it, or ``null`` for all time, + is the main filter, which follows each widget's own date dataset. A range kept on another + dataset, or a main filter pinned to one, is a changed filter. + """ + found = _filters_by_id(dashboard) + failures: list[str] = [] + for filter_id, wanted in expected.items(): + entries = found.get(filter_id) + if not entries: + failures.append(f"the dashboard lost date filter {filter_id!r}") + continue + for label, date_filter in entries: + if date_filter.get("type") != "date_filter": + failures.append(f"{label} is a {date_filter.get('type')!r}, expected a date filter") + continue + failures.extend(_date_filter_mismatch(label, date_filter, wanted)) + dataset = None if wanted is None else wanted.get("date") + if date_filter.get("date") != dataset: + failures.append(f"{label}: date dataset expected {dataset!r}, got {date_filter.get('date')!r}") + return failures + + +def _check_preserved_widgets(widgets: list[dict], expected: list[dict]) -> list[str]: + """A widget the change did not target must keep every key the expectation lists (GDAI-2541). + + Compared on the keys listed and nothing else, so a case states what it guards -- AAC writes + a hidden title as ``title: false`` and drills as ``interactions``. + """ + failures: list[str] = [] + for exp in expected: + viz_id = exp.get("visualization") + keys = [k for k in exp if k != "visualization"] + candidates = [w for w in widgets if w.get("visualization") == viz_id] + if not candidates: + failures.append(f"the widget showing {viz_id!r} is gone") + continue + # Every placement keeps the keys: one intact copy must not hide one that lost its drill. + mismatched = [w for w in candidates if not all(w.get(k) == exp[k] for k in keys)] + if not mismatched: + continue + widget = mismatched[0] + changed = ", ".join(f"{k} {widget.get(k)!r} (expected {exp[k]!r})" for k in keys if widget.get(k) != exp[k]) + failures.append(f"the widget showing {viz_id!r} changed: {changed}") + return failures + + def _date_range_text(date_range: dict | None) -> str: """Render a date range the way a user would say it, for the simulated reply. @@ -464,9 +794,26 @@ def build_simulated_reply(expected_output: dict) -> str: if existing: segments.append(", ".join(existing)) segments.extend(f'a new chart titled "{title}"' for title in authored) + # Only titled tabs are said: an untitled entry asserts a tab count, and asking for "one tab" + # out loud would be the reply leading the agent rather than the question. Each tab names its + # charts, since `_check_tabs` requires every chart on its own tab and the agent cannot guess. + titles_by_id = {v.get("id"): str(v.get("title", "")) for v in visualizations if v.get("id") is not None} + placements = [] + for tab in expected_output.get("tabs") or []: + if not tab.get("title"): + continue + charts = [ + titles_by_id.get(c, c) if isinstance(c, str) else str(c.get("title", "")) + for c in tab.get("visualizations") or [] + ] + on_tab = f'the "{tab["title"]}" tab' + 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 "" return ( f"Please use these charts: {', and '.join(segments)}. " + f"{tabs}" f"Date range: {_date_range_text(expected_output.get('date_range'))}. " f"Anything else is up to you. Please create the dashboard now." ) @@ -545,7 +892,30 @@ def _validate_expectation(expected_output: dict) -> None: raise ValueError(f"a filter expectation must be an object, got {entry!r}") if not entry.get("using"): raise ValueError(f"a filter expectation needs the label it filters on, got {entry!r}") - _expected_selection(entry) + if "display_as" in entry and not (isinstance(entry["display_as"], str) and entry["display_as"]): + raise ValueError(f"a filter's display_as must name a label, got {entry!r}") + if "group" in entry and not (isinstance(entry["group"], str) and entry["group"]): + raise ValueError(f"a filter's group must name the group's title, got {entry!r}") + filter_type = _expected_filter_type(entry) + if filter_type not in _FILTER_VALUE_KEYS: + raise ValueError(f"a filter expectation's type must be one of {sorted(_FILTER_VALUE_KEYS)}, got {entry!r}") + # A key outside the compared ones, a misspelling included, would assert nothing. + compared = _ATTRIBUTE_FILTER_KEYS if filter_type == "attribute_filter" else _FILTER_VALUE_KEYS[filter_type] + unknown = sorted(entry.keys() - {"type", "using", "group", *compared}) + if unknown: + raise ValueError(f"a {filter_type.replace('_', ' ')} expectation cannot state {unknown}, got {entry!r}") + if filter_type == "attribute_filter": + _expected_selection(entry) + elif filter_type == "text_filter": + if not entry.get("condition") or ("value" in entry) == ("values" in entry): + raise ValueError(f"a text filter expectation needs a condition and one of value/values, got {entry!r}") + if "values" in entry and not isinstance(entry["values"], list): + raise ValueError(f"a text filter's values must be a list, got {entry!r}") + if "value" in entry and not isinstance(entry["value"], str): + raise ValueError(f"a text filter's value must be a string, got {entry!r}") + elif not (isinstance(entry.get("conditions"), list) and entry["conditions"]): + raise ValueError(f"a metric value filter expectation needs its conditions, got {entry!r}") + _validate_layout_expectations(expected_output) saved_id = expected_output.get("saved_dashboard_id") if _is_edit(expected_output): if not isinstance(saved_id, str) or not saved_id: @@ -555,6 +925,99 @@ def _validate_expectation(expected_output: dict) -> None: _min_new_visualizations(expected_output) +def _validate_layout_expectations(expected_output: dict) -> None: + """Shape checks for the opt-in keys on charts, tabs, per-tab filters and preserved widgets. + + Raises: + ValueError: one of them is unusable. Loud for the same reason as the rest of the + fixture checks: a typo would otherwise assert nothing and read green. + """ + for chart in expected_output.get("visualizations") or []: + if isinstance(chart, dict) and "date_dataset" in chart and not _is_dataset_or_null(chart["date_dataset"]): + raise ValueError(f"date_dataset must name a date dataset or be null, got {chart!r}") + + if "tabs" in expected_output: + tabs = expected_output["tabs"] + if not isinstance(tabs, list) or not tabs or not all(isinstance(t, dict) for t in tabs): + raise ValueError(f"tabs must be a non-empty list of tab objects, got {tabs!r}") + for tab in tabs: + charts = tab.get("visualizations") or [] + if not isinstance(charts, list) or not all( + (isinstance(c, str) and c) or (isinstance(c, dict) and c.get("title")) for c in charts + ): + raise ValueError(f"a tab's visualizations must be chart ids or {{'title': ...}}, got {tab!r}") + + if "tab_filters" in expected_output: + tab_filters = expected_output["tab_filters"] + if not isinstance(tab_filters, dict) or not tab_filters: + raise ValueError(f"tab_filters must map tab ids to their filters, got {tab_filters!r}") + for tab_id, filters in tab_filters.items(): + if not isinstance(filters, dict) or not all( + isinstance(f, dict) and f.get("type") for f in filters.values() + ): + 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) + + if "date_filters" in expected_output: + date_filters = expected_output["date_filters"] + if not isinstance(date_filters, dict) or not date_filters: + raise ValueError(f"date_filters must map filter ids to a range or null, got {date_filters!r}") + for filter_id, date_range in date_filters.items(): + if date_range is not None and not isinstance(date_range, dict): + raise ValueError(f"date_filters[{filter_id!r}] must be an object or null, got {date_range!r}") + if date_range == {}: + # Not all time: every bound would be compared against an absent one, and a real + # date filter always has a granularity, so the case could never pass. + raise ValueError( + f"date_filters[{filter_id!r}] is empty; use null for all time or state granularity/from/to" + ) + unknown = sorted((date_range or {}).keys() - set(_DATE_FILTER_KEYS)) + if unknown: + raise ValueError( + f"date_filters[{filter_id!r}] states {unknown}, which is not one of {list(_DATE_FILTER_KEYS)}" + ) + if date_range and "date" in date_range and not _is_dataset_or_null(date_range["date"]): + raise ValueError( + f"date_filters[{filter_id!r}] date must name a date dataset or be null, got {date_range['date']!r}" + ) + + if "preserved_widgets" in expected_output: + preserved = expected_output["preserved_widgets"] + if not isinstance(preserved, list) or not preserved: + raise ValueError(f"preserved_widgets must be a non-empty list, got {preserved!r}") + for widget in preserved: + 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}") + + +def _is_dataset_or_null(value: object) -> bool: + return value is None or (isinstance(value, str) and bool(value)) + + +def _validate_tab_attribute_filter(where: str, entry: dict) -> None: + """An attribute filter in ``tab_filters`` uses the AAC shape: the selection sits under ``state``. + + The ``filters`` shorthand (``include``/``exclude``/``selection`` on the entry) does not apply + here. Read as AAC, it would compare as "all" and pass a case that checks nothing. + + Raises: + ValueError: the entry is not in the AAC shape. + """ + shorthand = sorted({"include", "exclude", "selection"} & entry.keys()) + if shorthand: + raise ValueError(f"{where} puts {shorthand} on the filter; tab_filters takes the AAC `state` object") + state = entry.get("state") + if state is None: + return + if not isinstance(state, dict): + raise ValueError(f"{where} state must be an object, got {state!r}") + for kind in ("include", "exclude"): + if kind in state and not isinstance(state[kind], list): + raise ValueError(f"{where} state.{kind} must be a list, got {state[kind]!r}") + + @dataclass(frozen=True) class _Applies: """Which of the conditional checks the case applies. @@ -568,6 +1031,24 @@ class _Applies: patch: bool date: bool filters: bool + date_bindings: bool = False + tabs: bool = False + tab_filters: bool = False + date_filters: bool = False + preserved_widgets: bool = False + + @classmethod + def of(cls, expected_output: dict) -> _Applies: + return cls( + patch=_is_edit(expected_output), + date=_has_date_range(expected_output), + filters=_has_filters(expected_output), + date_bindings=_has_date_bindings(expected_output), + tabs="tabs" in expected_output, + tab_filters="tab_filters" in expected_output, + date_filters="date_filters" in expected_output, + preserved_widgets="preserved_widgets" in expected_output, + ) @dataclass @@ -599,6 +1080,14 @@ class DashboardEvaluation: patch_applies: bool = True references_carried: bool = True titles_matched: bool = True + # False by default, unlike the older checks above: every early return then scores them as + # failed without having to name them, and only the run that reached a document sets them. + # They are published only where the case applies them, so a default never shows otherwise. + date_bindings_correct: bool = False + tabs_correct: bool = False + tab_filters_correct: bool = False + date_filters_correct: bool = False + widgets_preserved: bool = False failures: list[str] = field(default_factory=list) notes: list[str] = field(default_factory=list) @@ -627,6 +1116,18 @@ 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 + # All prefixed, for the same reason as the filters check: the combo report tells skills + # apart by the score names a trace carries. + if self.applies.date_bindings: + checks["dashboard_date_bindings_correct"] = self.date_bindings_correct + if self.applies.tabs: + checks["dashboard_tabs_correct"] = self.tabs_correct + if self.applies.tab_filters: + checks["dashboard_tab_filters_correct"] = self.tab_filters_correct + if self.applies.date_filters: + checks["dashboard_date_filters_correct"] = self.date_filters_correct + if self.applies.preserved_widgets: + checks["dashboard_widgets_preserved"] = self.widgets_preserved return checks @property @@ -694,7 +1195,7 @@ 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)) + applies = _Applies.of(expected_output) tool = _producing_tool(expected_output) if tool_result is None: @@ -813,11 +1314,26 @@ def evaluate_dashboard_response( title_notes = title_mismatches 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 [] + _check_date_range(document, expected_output.get("date_range"), applies.patch) + if _has_date_range(expected_output) + else [] ) filter_failures = ( _check_filters(document, expected_output.get("filters") or []) if _has_filters(expected_output) else [] ) + binding_failures = ( + _check_date_bindings(widgets, new_ids, expected_output.get("visualizations") or []) + if applies.date_bindings + else [] + ) + tab_failures = _check_tabs(document, new_ids, expected_output["tabs"]) if applies.tabs else [] + tab_filter_failures = _check_tab_filters(document, expected_output["tab_filters"]) if applies.tab_filters else [] + date_filter_failures = ( + _check_date_filters(document, expected_output["date_filters"]) if applies.date_filters else [] + ) + preserved_failures = ( + _check_preserved_widgets(widgets, expected_output["preserved_widgets"]) if applies.preserved_widgets else [] + ) return DashboardEvaluation( drafted=True, @@ -832,7 +1348,23 @@ 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], + date_bindings_correct=not binding_failures, + tabs_correct=not tab_failures, + tab_filters_correct=not tab_filter_failures, + date_filters_correct=not date_filter_failures, + widgets_preserved=not preserved_failures, + failures=[ + *chart_failures, + *saved_failures, + *date_failures, + *filter_failures, + *binding_failures, + *tab_failures, + *tab_filter_failures, + *date_filter_failures, + *preserved_failures, + *new_failures, + ], notes=[*title_notes, *reference_notes], ) diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index e949d8a3b..b7101ef37 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -9,6 +9,7 @@ _date_range_text, _extract_tool_result, _skill_activated, + _validate_expectation, _widgets_of, build_simulated_reply, evaluate_agentic_dashboard_skill, @@ -989,6 +990,784 @@ def test_an_unphrasable_date_range_is_rejected_up_front(self): client.send_message.assert_not_called() +# ── layout and filter checks added for the M2 bug fixes ───────────────────── +# A saved two-tab dashboard loaded for editing, on the ecommerce_demo ids used above. Its shape +# follows a real edit run: a dataset-scoped second date filter, a customer filter shown through +# its secondary label, and a widget carrying a drill and a hidden title. +_DRILL = [{"click_on": "m_net_sales", "open_dashboard": "customer_detail"}] + +_MAIN_DATE = {"type": "date_filter", "granularity": "MONTH", "from": -1, "to": -1} +_SECOND_DATE = {"type": "date_filter", "granularity": "QUARTER", "date": "order_date", "from": 0, "to": 0} +_CUSTOMER_BY_NAME = {"type": "attribute_filter", "using": "label/customer_name", "state": {"include": ["cus_1001"]}} +_TAB2_DATE = {"type": "date_filter", "granularity": "YEAR", "from": 0, "to": 0} +_CATEGORY_ALL = {"type": "attribute_filter", "using": "label/product_category"} + + +def _tabbed_base_part() -> dict: + return { + "type": "dashboard", + "dashboard": { + "type": "dashboard", + "id": _OVERVIEW_DASHBOARD, + "version": "3", + "title": "1. Overview", + "tabs": [ + { + "id": "tab_customers", + "title": "Customers", + "filters": { + "main_date": _MAIN_DATE, + "second_date": _SECOND_DATE, + "customer_filter": _CUSTOMER_BY_NAME, + }, + "sections": [ + { + "title": "Overview", + "widgets": [ + { + "visualization": _NET_SALES, + "title": False, + "columns": 6, + "rows": 12, + "interactions": _DRILL, + }, + { + "visualization": _TOTAL_CUSTOMERS, + "title": "Total Customers", + "columns": 6, + "rows": 12, + }, + ], + } + ], + }, + { + "id": "tab_products", + "title": "Products", + "filters": {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL}, + "sections": [ + { + "widgets": [ + { + "visualization": _ACTIVE_CUSTOMERS, + "title": "Active Customers", + "columns": 6, + "rows": 12, + } + ] + } + ], + }, + ], + }, + "saved_dashboard_id": _OVERVIEW_DASHBOARD, + "references": { + "visualizations": [ + {"id": v, "type": "bar_chart", "title": v} for v in (_NET_SALES, _TOTAL_CUSTOMERS, _ACTIVE_CUSTOMERS) + ], + "new_visualizations": [], + }, + } + + +# Operations in the shape the agent emits them, each led by the test guard that pins it. +_RESIZE_AMOUNT = [ + {"op": "test", "path": "/tabs/0/sections/0/widgets/0/visualization", "value": _NET_SALES}, + {"op": "add", "path": "/tabs/0/sections/0/widgets/0/columns", "value": 12}, +] +_SECOND_DATE_LAST_YEAR = {**_SECOND_DATE, "granularity": "YEAR", "from": -1, "to": -1} +_CHANGE_SECOND_DATE = [ + {"op": "test", "path": "/tabs/0/filters/second_date", "value": _SECOND_DATE}, + {"op": "replace", "path": "/tabs/0/filters/second_date", "value": _SECOND_DATE_LAST_YEAR}, +] +_CUSTOMER_CHANGED = { + "type": "attribute_filter", + "using": "label/customer_id", + "display_as": "label/customer_name", + "state": {"include": ["cus_2002"]}, +} +_CHANGE_CUSTOMER = [ + {"op": "test", "path": "/tabs/0/filters/customer_filter", "value": _CUSTOMER_BY_NAME}, + {"op": "replace", "path": "/tabs/0/filters/customer_filter", "value": _CUSTOMER_CHANGED}, +] + +_TABBED_EDIT = { + "type": "dashboardPatch", + "saved_dashboard_id": _OVERVIEW_DASHBOARD, + "visualizations": [{"id": _NET_SALES, "title": ""}], + "min_new_visualizations": 0, +} + + +def _score_tabbed_edit(expected: dict, operations: list[dict]): + patch_part = _patch_part(operations, dashboard_id=_OVERVIEW_DASHBOARD) + return evaluate_dashboard_response(_patch_result(), _tabbed_base_part(), expected, True, patch_part=patch_part) + + +class TestFilterDisplayLabel: + """GDAI-2488: changing a value on a filter shown through a secondary label stores it under + the primary label, and the secondary label must survive as ``display_as``.""" + + _EXPECTED = { + **_TABBED_EDIT, + "filters": [{"using": "label/customer_id", "display_as": "label/customer_name", "include": ["cus_2002"]}], + } + + def test_the_value_moves_to_the_primary_label_and_keeps_its_display_label(self): + ev = _score_tabbed_edit(self._EXPECTED, _CHANGE_CUSTOMER) + assert ev.filters_correct, ev.failures + + def test_a_lost_display_label_fails(self): + without = {k: v for k, v in _CUSTOMER_CHANGED.items() if k != "display_as"} + operations = [_CHANGE_CUSTOMER[0], {**_CHANGE_CUSTOMER[1], "value": without}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert not ev.filters_correct + assert any("is displayed as None, expected 'label/customer_name'" in f for f in ev.failures) + + def test_a_filter_expectation_without_display_as_ignores_it(self): + expected = {**_TABBED_EDIT, "filters": [{"using": "label/customer_id", "include": ["cus_2002"]}]} + assert _score_tabbed_edit(expected, _CHANGE_CUSTOMER).filters_correct + + def test_an_empty_display_label_is_rejected_up_front(self): + client = MagicMock() + expected = {**_TABBED_EDIT, "filters": [{"using": "label/customer_id", "display_as": "", "selection": "all"}]} + with pytest.raises(ValueError, match="display_as must name a label"): + _run_with(client, expected) + client.send_message.assert_not_called() + + +class TestDateBindings: + """GDAI-2391 / GDAI-2462: a widget follows the date filter only through the dataset it names. + Per chart, because a chart that reaches no date dataset is right to carry none.""" + + def _score(self, widgets: list[dict], visualizations: list[dict], new_visualizations: list[str] | None = None): + expected = {**_DC05_EXPECTED, "visualizations": visualizations} + if new_visualizations: + expected["min_new_visualizations"] = len(new_visualizations) + part = _dashboard_part( + widgets, + date_filter=_THIS_YEAR_FILTER, + visualizations=[ + w["visualization"] for w in widgets if w["visualization"] not in (new_visualizations or []) + ], + new_visualizations=new_visualizations, + ) + return evaluate_dashboard_response( + _draft_result(new_visualization_count=len(new_visualizations or [])), part, expected, True + ) + + def test_bound_unbound_and_unchecked_widgets_all_pass(self): + widgets = [ + {**_widget("Total Customers", _TOTAL_CUSTOMERS), "date": "date"}, + _widget("Active Customers", _ACTIVE_CUSTOMERS), + _widget("Return Customers", _RETURN_CUSTOMERS), + ] + ev = self._score( + widgets, + [ + {"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": "date"}, + {"id": _ACTIVE_CUSTOMERS, "title": "Active Customers", "date_dataset": None}, + {"id": _RETURN_CUSTOMERS, "title": "Return Customers"}, + ], + ) + assert ev.strict_checks["dashboard_date_bindings_correct"] is True + assert ev.strict_pass, ev.failures + + def test_a_widget_left_unbound_fails(self): + """The shape GDAI-2462 shipped: the chart reaches a date dataset, the widget names none.""" + ev = self._score( + [_widget("Total Customers", _TOTAL_CUSTOMERS)], + [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": "date"}], + ) + assert not ev.date_bindings_correct + assert any("follows date None, expected 'date'" in f for f in ev.failures) + + def test_a_binding_where_none_is_possible_fails(self): + ev = self._score( + [{**_widget("Total Customers", _TOTAL_CUSTOMERS), "date": "date"}], + [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": None}], + ) + assert not ev.date_bindings_correct + + def test_a_binding_to_another_dataset_fails(self): + ev = self._score( + [{**_widget("Total Customers", _TOTAL_CUSTOMERS), "date": "ship_date"}], + [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": "date"}], + ) + assert any("follows date 'ship_date', expected 'date'" in f for f in ev.failures) + + def test_a_right_placement_does_not_hide_a_wrong_one(self): + ev = self._score( + [ + {**_widget("Total Customers", _TOTAL_CUSTOMERS), "date": "date"}, + {**_widget("Total Customers", _TOTAL_CUSTOMERS), "date": "ship_date"}, + ], + [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": "date"}], + ) + assert not ev.date_bindings_correct + assert any("follows date 'date', 'ship_date', expected 'date'" in f for f in ev.failures) + + def test_an_authored_chart_is_checked_by_title(self): + ev = self._score( + [{**_widget("Population by City", "authored-1"), "date": "date"}], + [{"id": None, "title": "Population by City", "date_dataset": "date"}], + new_visualizations=["authored-1"], + ) + assert ev.date_bindings_correct, ev.failures + + def test_a_missing_chart_cannot_pass_the_binding_check(self): + ev = self._score( + [_widget("Active Customers", _ACTIVE_CUSTOMERS)], + [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers", "date_dataset": None}], + ) + assert not ev.date_bindings_correct + assert any("cannot check the date binding" in f for f in ev.failures) + + def test_a_case_without_date_datasets_does_not_publish_the_check(self): + part = _dashboard_part([_widget("Total Customers", _TOTAL_CUSTOMERS)], date_filter=_THIS_YEAR_FILTER) + ev = evaluate_dashboard_response(_draft_result(), part, _DC05_EXPECTED, skill_activated=True) + assert "dashboard_date_bindings_correct" not in ev.strict_checks + + def test_a_malformed_date_dataset_is_rejected_up_front(self): + client = MagicMock() + expected = {**_DC05_EXPECTED, "visualizations": [{"id": _TOTAL_CUSTOMERS, "title": "T", "date_dataset": 1}]} + with pytest.raises(ValueError, match="date_dataset must name a date dataset or be null"): + _run_with(client, expected) + client.send_message.assert_not_called() + + +class TestTabs: + """GDAI-2426: the tabs the user named, in their order; a plain request stays on one tab.""" + + _SALES = { + "id": "sales", + "title": "Sales", + "filters": {"date": _THIS_YEAR_FILTER}, + "sections": [{"widgets": [_widget("Total Customers", _TOTAL_CUSTOMERS)]}], + } + _MARKETING = { + "id": "marketing", + "title": "Marketing", + "filters": {"date": _THIS_YEAR_FILTER}, + "sections": [{"widgets": [_widget("Active Customers", _ACTIVE_CUSTOMERS)]}], + } + + def _score(self, tabs: list[dict], expected_tabs: list[dict]): + widgets = [w for t in tabs for s in t["sections"] for w in s["widgets"]] + part = _dashboard_part(widgets, date_filter=_THIS_YEAR_FILTER, tabs=tabs) + return evaluate_dashboard_response(_draft_result(), part, {**_DC05_EXPECTED, "tabs": expected_tabs}, True) + + def test_the_named_tabs_in_order_with_their_charts_pass(self): + ev = self._score( + [self._SALES, self._MARKETING], + [ + {"title": "sales", "visualizations": [_TOTAL_CUSTOMERS]}, + {"title": "Marketing", "visualizations": [_ACTIVE_CUSTOMERS]}, + ], + ) + assert ev.strict_checks["dashboard_tabs_correct"] is True + assert ev.strict_pass, ev.failures + + def test_swapped_tabs_fail(self): + ev = self._score([self._MARKETING, self._SALES], [{"title": "Sales"}, {"title": "Marketing"}]) + assert not ev.tabs_correct + assert any("tab 0 is titled 'Marketing', expected 'Sales'" in f for f in ev.failures) + + def test_a_missing_tab_fails(self): + ev = self._score([self._SALES], [{"title": "Sales"}, {"title": "Marketing"}]) + assert any("has 1 tab(s) ('Sales'), expected 2" in f for f in ev.failures) + + def test_a_chart_on_the_wrong_tab_fails(self): + ev = self._score( + [self._SALES, self._MARKETING], + [{"title": "Sales", "visualizations": [_ACTIVE_CUSTOMERS]}, {"title": "Marketing"}], + ) + assert any("tab 0 ('Sales') does not hold chart" in f for f in ev.failures) + + def test_one_untitled_entry_accepts_any_single_tab(self): + assert self._score([self._SALES], [{}]).tabs_correct + + def test_one_untitled_entry_rejects_a_split_dashboard(self): + assert not self._score([self._SALES, self._MARKETING], [{}]).tabs_correct + + def test_the_simulated_reply_names_the_titled_tabs(self): + reply = build_simulated_reply({**_DC05_EXPECTED, "tabs": [{"title": "Sales"}, {"title": "Marketing"}]}) + assert 'Use these tabs: the "Sales" tab; the "Marketing" tab. ' in reply + + def test_the_simulated_reply_says_which_chart_goes_on_which_tab(self): + """`_check_tabs` holds every chart to its own tab, so the reply must say where each goes.""" + expected = { + **_DC05_EXPECTED, + "visualizations": [{"id": "dc8575f5", "title": "Active Customers"}, {"id": None, "title": "Returns"}], + "tabs": [ + {"title": "Customers", "visualizations": ["dc8575f5"]}, + {"title": "Returns", "visualizations": [{"title": "Returns"}]}, + ], + } + reply = build_simulated_reply(expected) + assert 'Use these tabs: "Active Customers" on the "Customers" tab; "Returns" on the "Returns" tab. ' in reply + + def test_the_simulated_reply_does_not_ask_for_an_untitled_tab(self): + assert "tabs" not in build_simulated_reply({**_DC05_EXPECTED, "tabs": [{}]}) + + @pytest.mark.parametrize("tabs", [[], "Sales", [{"visualizations": [{"id": "x"}]}]]) + def test_a_malformed_tab_list_is_rejected_up_front(self, tabs): + client = MagicMock() + with pytest.raises(ValueError, match="tab"): + _run_with(client, {**_DC05_EXPECTED, "tabs": tabs}) + client.send_message.assert_not_called() + + +class TestTabFilters: + """GDAI-2540 / GDAI-2467: an edit leaves every tab's filters as they were, exactly.""" + + _UNTOUCHED = { + "tab_customers": {"main_date": _MAIN_DATE, "second_date": _SECOND_DATE, "customer_filter": _CUSTOMER_BY_NAME}, + "tab_products": {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL}, + } + + def _score(self, operations: list[dict]): + return _score_tabbed_edit({**_TABBED_EDIT, "tab_filters": self._UNTOUCHED}, operations) + + def test_a_resize_leaves_every_tab_as_it_was(self): + ev = self._score(_RESIZE_AMOUNT) + assert ev.strict_checks["dashboard_tab_filters_correct"] is True + assert ev.strict_pass, ev.failures + + def test_the_first_tabs_filters_copied_onto_another_fails(self): + """The GDAI-2540 shape: the second tab gains what only the first had.""" + operations = [ + *_RESIZE_AMOUNT, + {"op": "add", "path": "/tabs/1/filters/customer_filter", "value": _CUSTOMER_BY_NAME}, + ] + ev = self._score(operations) + assert not ev.tab_filters_correct + assert any("tab 'tab_products' carries filter 'customer_filter'" in f for f in ev.failures) + + def test_a_reset_date_range_on_an_untouched_tab_fails(self): + all_time = {"type": "date_filter", "granularity": "YEAR"} + operations = [*_RESIZE_AMOUNT, {"op": "replace", "path": "/tabs/1/filters/tab2_date", "value": all_time}] + ev = self._score(operations) + assert any("tab 'tab_products' filter 'tab2_date': from expected 0, got None" in f for f in ev.failures) + + def test_a_narrowed_attribute_filter_fails(self): + narrowed = {**_CATEGORY_ALL, "state": {"include": ["Electronics"]}} + operations = [*_RESIZE_AMOUNT, {"op": "replace", "path": "/tabs/1/filters/category_filter", "value": narrowed}] + ev = self._score(operations) + assert any("selects include ['Electronics'], expected all" in f for f in ev.failures) + + def test_a_dropped_filter_fails(self): + operations = [*_RESIZE_AMOUNT, {"op": "remove", "path": "/tabs/0/filters/customer_filter"}] + assert any("tab 'tab_customers' lost filter 'customer_filter'" in f for f in self._score(operations).failures) + + 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) + + @pytest.mark.parametrize( + "tab_filters", [{}, {"tab_customers": []}, {"tab_customers": {"main_date": {"granularity": "MONTH"}}}] + ) + def test_a_malformed_expectation_is_rejected_up_front(self, tab_filters): + client = MagicMock() + with pytest.raises(ValueError, match="tab_filters"): + _run_with(client, {**_TABBED_EDIT, "tab_filters": tab_filters}) + client.send_message.assert_not_called() + + @pytest.mark.parametrize( + ("attribute_filter", "match"), + [ + ({"include": ["Electronics"]}, r"puts \['include'\] on the filter"), + ({"selection": "all"}, r"puts \['selection'\] on the filter"), + ({"state": ["Electronics"]}, "state must be an object"), + ({"state": {"exclude": "Electronics"}}, "state.exclude must be a list"), + ], + ) + def test_an_attribute_filter_outside_the_aac_shape_is_rejected_up_front(self, attribute_filter, match): + """The `filters` shorthand would read as "all" here and let the case pass on nothing.""" + entry = {"type": "attribute_filter", "using": "label/product_category", **attribute_filter} + client = MagicMock() + with pytest.raises(ValueError, match=match): + _run_with(client, {**_TABBED_EDIT, "tab_filters": {"tab_products": {"category_filter": entry}}}) + client.send_message.assert_not_called() + + +def test_a_tabbed_saved_dashboard_holds_date_range_to_one_matching_filter(): + """A saved dashboard can carry tabs too. Read as drafted, every date filter on it would have + to match, which a main filter beside a dataset-scoped one never does.""" + expected = {**_TABBED_EDIT, "date_range": {"granularity": "MONTH", "from": -1, "to": -1}} + ev = _score_tabbed_edit(expected, _CHANGE_SECOND_DATE) + assert ev.date_range_correct, ev.failures + + +class TestDateFilters: + """GDAI-2491: on a dashboard with two date filters, changing the second must leave the main + one alone and remove neither. Named by id, since nothing else says which one is main.""" + + _EXPECTED = { + **_TABBED_EDIT, + "date_filters": { + "main_date": {"granularity": "MONTH", "from": -1, "to": -1}, + "second_date": {"granularity": "YEAR", "from": -1, "to": -1, "date": "order_date"}, + }, + } + + def test_the_second_filter_changes_and_the_main_one_stays(self): + ev = _score_tabbed_edit(self._EXPECTED, _CHANGE_SECOND_DATE) + assert ev.strict_checks["dashboard_date_filters_correct"] is True + assert ev.strict_pass, ev.failures + + def test_the_change_landing_on_the_main_filter_fails(self): + """The GDAI-2491 shape: the main filter took the change meant for the second one.""" + main_last_year = {**_MAIN_DATE, "granularity": "YEAR"} + operations = [{"op": "replace", "path": "/tabs/0/filters/main_date", "value": main_last_year}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert not ev.date_filters_correct + assert any("'main_date': date granularity expected 'MONTH', got 'YEAR'" in f for f in ev.failures) + + def test_a_removed_second_filter_fails(self): + operations = [{"op": "remove", "path": "/tabs/0/filters/second_date"}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert any("lost date filter 'second_date'" in f for f in ev.failures) + + def test_a_flat_saved_dashboard_is_read_from_its_root_filters(self): + expected = {**_DE01_EXPECTED, "date_filters": {"0_dateFilter": {"granularity": "MONTH", "from": -1, "to": -1}}} + patch_part = _patch_part( + [{"op": "replace", "path": "/sections/0/widgets/0/title", "value": "Order funnel"}], + visualizations=[_ORDER_STATUS], + ) + ev = evaluate_dashboard_response(_patch_result(), _edit_base_part(), expected, True, patch_part=patch_part) + assert ev.date_filters_correct, ev.failures + + def test_a_range_kept_on_another_dataset_fails(self): + moved = {**_SECOND_DATE_LAST_YEAR, "date": "ship_date"} + operations = [{"op": "replace", "path": "/tabs/0/filters/second_date", "value": moved}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert not ev.date_filters_correct + assert any("'second_date': date dataset expected 'order_date', got 'ship_date'" in f for f in ev.failures) + + @pytest.mark.parametrize("main", [{"granularity": "MONTH", "from": -1, "to": -1}, None]) + def test_a_main_filter_pinned_to_a_dataset_fails(self, main): + """The main filter carries no dataset -- it follows each widget's own date. An entry without + `date`, or null for all time, says so, the same way `tab_filters` reads it.""" + pinned = {**_MAIN_DATE, "date": "order_date"} + if main is None: + pinned = {"type": "date_filter", "granularity": "MONTH", "date": "order_date"} + operations = [{"op": "replace", "path": "/tabs/0/filters/main_date", "value": pinned}] + ev = _score_tabbed_edit({**_TABBED_EDIT, "date_filters": {"main_date": main}}, operations) + assert not ev.date_filters_correct + assert any("'main_date': date dataset expected None, got 'order_date'" in f for f in ev.failures) + + def test_a_dataset_scoped_filter_stated_without_its_dataset_fails(self): + expected = {**_TABBED_EDIT, "date_filters": {"second_date": {"granularity": "YEAR", "from": -1, "to": -1}}} + ev = _score_tabbed_edit(expected, _CHANGE_SECOND_DATE) + assert any("'second_date': date dataset expected None, got 'order_date'" in f for f in ev.failures) + + @pytest.mark.parametrize( + ("entry", "match"), + [ + ("last month", r"date_filters\['main_date'\] must be an object or null"), + ({"granularity": "YEAR", "date": ""}, r"date_filters\['main_date'\] date must name a date dataset.*got ''"), + ({"granularity": "YEAR", "date": 5}, r"date_filters\['main_date'\] date must name a date dataset.*got 5"), + ({"granularity": "YEAR", "dataset": "order_date"}, r"date_filters\['main_date'\] states \['dataset'\]"), + ({}, r"date_filters\['main_date'\] is empty; use null for all time"), + ], + ) + def test_a_malformed_entry_is_rejected_up_front(self, entry, match): + client = MagicMock() + with pytest.raises(ValueError, match=match): + _run_with(client, {**_TABBED_EDIT, "date_filters": {"main_date": entry}}) + client.send_message.assert_not_called() + + +class TestPreservedWidgets: + """GDAI-2541: a drill and a hidden title survive an edit. AAC writes them as + ``interactions`` and ``title: false``.""" + + _EXPECTED = { + **_TABBED_EDIT, + "preserved_widgets": [{"visualization": _NET_SALES, "title": False, "interactions": _DRILL}], + } + + def test_a_resize_of_the_drilled_widget_keeps_its_drill_and_hidden_title(self): + ev = _score_tabbed_edit(self._EXPECTED, _RESIZE_AMOUNT) + assert ev.strict_checks["dashboard_widgets_preserved"] is True + assert ev.strict_pass, ev.failures + + def test_a_widget_rebuilt_without_its_drill_fails(self): + rebuilt = {"visualization": _NET_SALES, "title": "Net Sales", "columns": 12, "rows": 12} + operations = [{"op": "replace", "path": "/tabs/0/sections/0/widgets/0", "value": rebuilt}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert not ev.widgets_preserved + assert any("changed: title 'Net Sales' (expected False), interactions None" in f for f in ev.failures) + + def test_a_removed_widget_fails(self): + expected = {**self._EXPECTED, "visualizations": [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers"}]} + operations = [{"op": "remove", "path": "/tabs/0/sections/0/widgets/0"}] + assert any("is gone" in f for f in _score_tabbed_edit(expected, operations).failures) + + def test_an_intact_placement_does_not_hide_one_that_lost_its_drill(self): + rebuilt = {"visualization": _NET_SALES, "title": False, "columns": 6, "rows": 12} + operations = [{"op": "add", "path": "/tabs/1/sections/0/widgets/-", "value": rebuilt}] + ev = _score_tabbed_edit(self._EXPECTED, operations) + assert not ev.widgets_preserved + assert any("changed: interactions None" in f for f in ev.failures) + + @pytest.mark.parametrize("preserved", [[], [{"visualization": _NET_SALES}], [{"title": False}]]) + def test_a_malformed_expectation_is_rejected_up_front(self, preserved): + client = MagicMock() + with pytest.raises(ValueError, match="preserved"): + _run_with(client, {**_TABBED_EDIT, "preserved_widgets": preserved}) + 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.""" + + _GROUP = { + "type": "filter_group", + "title": "Product", + "filters": { + "product_category": {"type": "attribute_filter", "using": "label/product_category"}, + "product_brand": {"type": "attribute_filter", "using": "label/product_brand"}, + }, + } + _TEXT = {"type": "text_filter", "using": "label/customer_name", "condition": "contains", "value": "Aaron"} + _METRIC = { + "type": "metric_value_filter", + "using": "metric/net_sales", + "conditions": [{"condition": "GREATER_THAN", "value": 1000.0}], + } + _EXPECTED_FILTERS = [ + {"using": "label/product_category", "selection": "all", "group": "product"}, + {"using": "label/product_brand", "selection": "all", "group": "Product"}, + {"type": "text_filter", "using": "label/customer_name", "condition": "contains", "value": "Aaron"}, + { + "type": "metric_value_filter", + "using": "metric/net_sales", + "conditions": [{"condition": "GREATER_THAN", "value": 1000}], + }, + ] + + def _score(self, filters: dict, expected_filters: list[dict]): + tabs = [ + { + "id": "overview", + "title": "Overview", + "filters": {"date": _THIS_YEAR_FILTER, **filters}, + "sections": [{"widgets": [_widget("Total Customers", _TOTAL_CUSTOMERS)]}], + } + ] + part = _dashboard_part([_widget("Total Customers", _TOTAL_CUSTOMERS)], date_filter=_THIS_YEAR_FILTER, tabs=tabs) + expected = {**_DC05_EXPECTED, "visualizations": [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers"}]} + return evaluate_dashboard_response(_draft_result(), part, {**expected, "filters": expected_filters}, True) + + def test_a_group_a_text_filter_and_a_metric_value_filter_all_pass(self): + ev = self._score( + {"product": self._GROUP, "customer_name": self._TEXT, "net_sales": self._METRIC}, self._EXPECTED_FILTERS + ) + assert ev.strict_checks["dashboard_filters_correct"] is True + assert ev.strict_pass, ev.failures + + def test_a_grouped_filter_is_found_when_the_case_names_no_group(self): + """Before groups were read, a filter the agent chose to group read as missing.""" + ev = self._score({"product": self._GROUP}, [{"using": "label/product_brand", "selection": "all"}]) + assert ev.filters_correct, ev.failures + + def test_a_filter_outside_the_named_group_fails(self): + loose = {"product_brand": {"type": "attribute_filter", "using": "label/product_brand"}} + ev = self._score(loose, [{"using": "label/product_brand", "selection": "all", "group": "Product"}]) + assert any( + "attribute filter on 'label/product_brand' sits in group None, expected 'Product'" in f for f in ev.failures + ) + + def test_a_text_filter_with_another_condition_fails(self): + negated = {**self._TEXT, "condition": "doesNotContain"} + ev = self._score({"customer_name": negated}, [self._EXPECTED_FILTERS[2]]) + assert any("condition expected 'contains', got 'doesNotContain'" in f for f in ev.failures) + + def test_whole_text_values_compare_in_any_order(self): + is_filter = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": ["B", "A"]} + expected = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": ["A", "B"]} + assert self._score({"customer_name": is_filter}, [expected]).filters_correct + + def test_a_null_among_whole_text_values_compares_without_ordering_errors(self): + is_filter = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": [None, "A"]} + expected = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": ["A", None]} + assert self._score({"customer_name": is_filter}, [expected]).filters_correct + + @pytest.mark.parametrize( + "got", [["1"], ["A", "A"], ["A"]], ids=["another_type", "a_repeated_value", "a_missing_value"] + ) + def test_whole_text_values_must_match_one_to_one_with_the_same_type(self, got): + is_filter = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": got} + want = [1] if got == ["1"] else ["A", "B"] + expected = {"type": "text_filter", "using": "label/customer_name", "condition": "is", "values": want} + assert not self._score({"customer_name": is_filter}, [expected]).filters_correct + + def test_a_metric_value_filter_with_another_threshold_fails(self): + lower = {**self._METRIC, "conditions": [{"condition": "GREATER_THAN", "value": 100.0}]} + ev = self._score({"net_sales": lower}, [self._EXPECTED_FILTERS[3]]) + assert any("metric value filter on 'metric/net_sales': conditions expected" in f for f in ev.failures) + + def test_an_attribute_ref_matches_the_label_ref_of_the_same_id(self): + """The tools accept `attribute/` or `label/` on `using`, and the model picks one per run.""" + by_attribute = {"customer_country": {"type": "attribute_filter", "using": "attribute/customer_country"}} + ev = self._score(by_attribute, [{"using": "label/customer_country", "selection": "all"}]) + assert ev.filters_correct, ev.failures + + def test_an_attribute_ref_does_not_match_another_id(self): + by_attribute = {"customer_city": {"type": "attribute_filter", "using": "attribute/customer_city"}} + ev = self._score(by_attribute, [{"using": "label/customer_country", "selection": "all"}]) + assert not ev.filters_correct + + def test_a_missing_text_filter_names_its_kind(self): + ev = self._score({}, [self._EXPECTED_FILTERS[2]]) + assert any("the dashboard carries no text filter on 'label/customer_name'" in f for f in ev.failures) + + @pytest.mark.parametrize( + ("entry", "match"), + [ + ({"type": "range_filter", "using": "label/x"}, "type must be one of"), + ({"type": "text_filter", "using": "label/x", "condition": "contains"}, "one of value/values"), + ({"type": "text_filter", "using": "label/x", "condition": "is", "values": "AB"}, "values must be a list"), + ({"type": "text_filter", "using": "label/x", "condition": "is", "values": 5}, "values must be a list"), + ( + {"type": "text_filter", "using": "label/x", "condition": "contains", "value": 5}, + "value must be a string", + ), + ( + {"type": "text_filter", "using": "label/x", "condition": "is", "value": "a", "values": ["a"]}, + "one of value/values", + ), + ({"type": "metric_value_filter", "using": "metric/x"}, "needs its conditions"), + ( + { + "type": "text_filter", + "using": "label/x", + "condition": "contains", + "value": "a", + "display_as": "label/y", + }, + r"text filter expectation cannot state \['display_as'\]", + ), + ( + {"type": "metric_value_filter", "using": "metric/x", "conditions": [{}], "include": ["a"]}, + r"metric value filter expectation cannot state \['include'\]", + ), + ( + {"type": "text_filter", "using": "label/x", "condition": "is", "value": "a", "case_sensitve": True}, + r"text filter expectation cannot state \['case_sensitve'\]", + ), + ({"using": "label/x", "selection": "all", "group": ""}, "group must name the group's title"), + ( + {"using": "label/x", "selection": "all", "dispaly_as": "label/y", "grop": "Geo"}, + r"attribute filter expectation cannot state \['dispaly_as', 'grop'\]", + ), + ], + ) + def test_a_malformed_filter_kind_is_rejected_up_front(self, entry, match): + client = MagicMock() + with pytest.raises(ValueError, match=match): + _run_with(client, {**_DC05_EXPECTED, "filters": [entry]}) + client.send_message.assert_not_called() + + +class TestUnusualShapes: + """Shapes an agent or a saved dashboard can produce that the main cases do not reach.""" + + def test_a_non_object_entry_in_a_filter_map_is_skipped(self): + tabs = [ + { + "id": "overview", + "title": "Overview", + "filters": { + "date": _THIS_YEAR_FILTER, + "broken": None, + "country": {"type": "attribute_filter", "using": "label/customer_country"}, + }, + "sections": [{"widgets": [_widget("Total Customers", _TOTAL_CUSTOMERS)]}], + } + ] + part = _dashboard_part([_widget("Total Customers", _TOTAL_CUSTOMERS)], date_filter=_THIS_YEAR_FILTER, tabs=tabs) + expected = { + **_DC05_EXPECTED, + "visualizations": [{"id": _TOTAL_CUSTOMERS, "title": "Total Customers"}], + "filters": [{"using": "label/customer_country", "selection": "all"}], + } + assert evaluate_dashboard_response(_draft_result(), part, expected, True).filters_correct + + def test_a_flat_saved_dashboard_reads_as_one_tab(self): + patch_part = _patch_part( + [{"op": "replace", "path": "/sections/0/widgets/0/title", "value": "Order funnel"}], + visualizations=[_ORDER_STATUS], + ) + ev = evaluate_dashboard_response( + _patch_result(), _edit_base_part(), {**_DE01_EXPECTED, "tabs": [{}]}, True, patch_part=patch_part + ) + assert ev.tabs_correct, ev.failures + + def test_a_tab_filter_replaced_by_another_kind_fails(self): + as_text = {"type": "text_filter", "using": "label/product_category", "condition": "contains", "value": "a"} + operations = [{"op": "replace", "path": "/tabs/1/filters/category_filter", "value": as_text}] + expected = { + **_TABBED_EDIT, + "tab_filters": {"tab_products": {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL}}, + } + ev = _score_tabbed_edit(expected, operations) + assert any("is a 'text_filter', expected a 'attribute_filter'" in f for f in ev.failures) + + def test_a_metric_value_filter_in_tab_filters_is_compared_on_its_stated_keys(self): + mvf = { + "type": "metric_value_filter", + "using": "metric/net_orders", + "conditions": [{"condition": "LESS_THAN", "value": 1000}], + } + changed = {**mvf, "conditions": [{"condition": "LESS_THAN", "value": 500}]} + operations = [{"op": "add", "path": "/tabs/1/filters/net_orders_filter", "value": changed}] + filters = {"tab2_date": _TAB2_DATE, "category_filter": _CATEGORY_ALL, "net_orders_filter": mvf} + ev = _score_tabbed_edit({**_TABBED_EDIT, "tab_filters": {"tab_products": filters}}, operations) + assert any("'net_orders_filter': conditions expected" in f for f in ev.failures) + + def test_a_date_filter_id_that_names_another_kind_fails(self): + expected = {**_TABBED_EDIT, "date_filters": {"customer_filter": None}} + ev = _score_tabbed_edit(expected, _RESIZE_AMOUNT) + assert any("'customer_filter' is a 'attribute_filter', expected a date filter" in f for f in ev.failures) + + @pytest.mark.parametrize("date_filters", [{}, ["main_date"]]) + def test_an_empty_or_non_object_date_filters_is_rejected_up_front(self, date_filters): + client = MagicMock() + with pytest.raises(ValueError, match="date_filters must map filter ids"): + _run_with(client, {**_TABBED_EDIT, "date_filters": date_filters}) + client.send_message.assert_not_called() + + def test_an_attribute_tab_filter_without_state_is_accepted(self): + """No `state` is how AAC writes an attribute filter that selects all.""" + _validate_expectation({**_TABBED_EDIT, "tab_filters": {"tab_products": {"category_filter": _CATEGORY_ALL}}}) + + +class TestNewChecksOnAnEmptyRun: + def test_a_run_with_nothing_to_score_fails_every_check_the_case_applies(self): + """An early return must not publish an unearned pass for the checks it never ran.""" + expected = { + **_TABBED_EDIT, + "visualizations": [{"id": _NET_SALES, "title": "", "date_dataset": None}], + "tabs": [{}], + "tab_filters": {"tab_customers": {}}, + "date_filters": {"main_date": None}, + "preserved_widgets": [{"visualization": _NET_SALES, "title": False}], + } + ev = evaluate_dashboard_response(None, None, expected, True) + for name in ( + "dashboard_date_bindings_correct", + "dashboard_tabs_correct", + "dashboard_tab_filters_correct", + "dashboard_date_filters_correct", + "dashboard_widgets_preserved", + ): + assert ev.strict_checks[name] is False + + class TestRunLoop: def test_a_first_turn_draft_never_reaches_the_simulated_user(self): part = _dashboard_part(