diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_filters.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_filters.py new file mode 100644 index 000000000..1ecbb5af6 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_filters.py @@ -0,0 +1,46 @@ +# (C) 2026 GoodData Corporation +"""Attribute filter selections and filter values, read the same way by the dashboard and document graders.""" + +from __future__ import annotations + + +def selection_of(attribute_filter: dict) -> tuple[str, list] | None: + """The elements an attribute filter restricts to, or ``None`` when it restricts nothing. + + An unrestricted filter is spelled as the absence of a selection, and an empty list means + the same thing, so both read as "all". + """ + state = attribute_filter.get("state") or {} + for kind in ("include", "exclude"): + values = state.get(kind) + if values: + return kind, list(values) + return None + + +def describe_selection(selection: tuple[str, list] | None) -> str: + return "all" if selection is None else f"{selection[0]} {selection[1]!r}" + + +def same_selection(actual: tuple[str, list] | None, expected: tuple[str, list] | None) -> bool: + """Whether two selections restrict to the same elements: an include or exclude list is a set, not a sequence.""" + if actual is None or expected is None: + return actual is None and expected is None + return actual[0] == expected[0] and same_unordered_values(expected[1], actual[1]) + + +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 diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py index 2fafc30ba..e0b1fff72 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 @@ -11,6 +11,7 @@ import jsonpatch from gooddata_eval.core.agentic._failed_runs import build_failed_runs +from gooddata_eval.core.agentic._filters import describe_selection, same_unordered_values, selection_of from gooddata_eval.core.agentic._gate import ( DEFAULT_GATE, EvalGate, @@ -465,24 +466,6 @@ def _check_date_range(dashboard: dict, expected: dict | None, is_edit: bool) -> return [reason for m in mismatches for reason in m] -def _selection_of(attribute_filter: dict) -> tuple[str, list] | None: - """The elements an attribute filter restricts to, or ``None`` when it restricts nothing. - - An unrestricted filter is spelled as the absence of a selection, and an empty list means - the same thing, so both read as "all". - """ - state = attribute_filter.get("state") or {} - for kind in ("include", "exclude"): - values = state.get(kind) - if values: - return kind, list(values) - return None - - -def _describe_selection(selection: tuple[str, list] | None) -> str: - return "all" if selection is None else f"{selection[0]} {selection[1]!r}" - - def _check_filters(dashboard: dict, expected: list[dict]) -> list[str]: """Every expected filter must be on the dashboard, with the stated selection or condition. @@ -534,9 +517,9 @@ def _check_filters(dashboard: dict, expected: list[dict]) -> list[str]: 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) - failures.append(f"filter on {using!r} selects {found}, expected {_describe_selection(wanted)}") + if not any(selection_of(f) == wanted for f in matches): + found = ", ".join(describe_selection(selection_of(f)) for f in matches) + failures.append(f"filter on {using!r} selects {found}, expected {describe_selection(wanted)}") return failures @@ -567,7 +550,7 @@ def _filter_value_mismatch(actual: dict, expected: dict) -> list[str]: continue want, got = expected[key], actual.get(key) same = ( - _same_unordered_values(want, got) + same_unordered_values(want, got) if key == "values" and isinstance(want, list) and isinstance(got, list) else got == want ) @@ -576,25 +559,8 @@ def _filter_value_mismatch(actual: dict, expected: dict) -> list[str]: 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. + """Read a fixture's filter entry into the same shape ``selection_of`` produces. Raises: ValueError: the entry states no selection the evaluator knows how to check. @@ -770,10 +736,10 @@ def _filter_mismatch(label: str, actual: dict, expected: dict) -> list[str]: 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): + if selection_of(actual) != selection_of(expected): return [ - f"{label} selects {_describe_selection(_selection_of(actual))}, " - f"expected {_describe_selection(_selection_of(expected))}" + f"{label} selects {describe_selection(selection_of(actual))}, " + f"expected {describe_selection(selection_of(expected))}" ] keys = ("using", "display_as") else: diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/document_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/document_skill.py index b06182647..9dc0368e5 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/document_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/document_skill.py @@ -16,6 +16,7 @@ from gooddata_eval.core.agentic._conversation_context import classify_reply from gooddata_eval.core.agentic._editor_takeaways import editor_takeaway_input from gooddata_eval.core.agentic._failed_runs import build_failed_runs +from gooddata_eval.core.agentic._filters import describe_selection, same_selection, selection_of from gooddata_eval.core.agentic._gate import ( DEFAULT_GATE, EvalGate, @@ -80,9 +81,35 @@ "asked_first_required", "clarification_reply", "rework", + "chart_periods", + } +) +_REWORK_KEYS = frozenset({"base_document_id", "keeps_pages", "page_charts", "new_page_charts", "title", "filters"}) +# A date filter's range keys, compared even when a rework item leaves them out: an all-time filter +# spells itself by leaving `from` and `to` out, so a drafted range must not pass it, and an offset +# means nothing without its granularity, so a range stated without one must not pass any granularity. +_DATE_FILTER_RANGE_KEYS = ("granularity", "from", "to") +# What a filter applies to, which a rework item must state: the label and selection of an attribute +# filter, and the date dataset of a date filter. +_REQUIRED_FILTER_KEYS = {"attribute_filter": ("using", "state"), "date_filter": ("date",)} +# A v2 draft arrives as AaC, where a relative period is `{granularity, from, to}` and an absolute one +# is `{from, to}`: the granularity is what tells them apart. These are the names the code converter +# writes; it reads `WEEK` too but writes it as `WEEK_US`, so a fixture stating `WEEK` could never match. +_AAC_PERIOD_GRANULARITIES = frozenset( + { + "SECOND", + "MINUTE", + "HOUR", + "DAY", + "WEEK_US", + "MONTH", + "QUARTER", + "YEAR", + "FISCAL_MONTH", + "FISCAL_QUARTER", + "FISCAL_YEAR", } ) -_REWORK_KEYS = frozenset({"base_document_id", "keeps_pages", "page_charts", "title", "filters"}) # Template tokens the document renders at export time ({documentName}, {periodStart}, ...). _PLACEHOLDER_RE = re.compile(r"\{\w+\}") _SUMMARY_SLOT = "summary" @@ -279,12 +306,17 @@ def _period(document: dict) -> dict: return period if isinstance(period, dict) else {} +def _is_relative(period: dict) -> bool: + """Whether an AaC period is relative: it states a granularity, which an absolute one never does.""" + return "granularity" in period + + def _period_bounds(document: dict) -> tuple[Any, Any] | None: """The document's first and last day, or ``None`` for a v2 relative period, which has no fixed dates.""" period = _period(document) if not _is_v2(document): return period.get("start"), period.get("end") - if period.get("type") == "relative": + if _is_relative(period): return None return period.get("from"), period.get("to") @@ -304,6 +336,13 @@ def _period_text(document: dict) -> str: return f"{bounds[0]} to {bounds[1]}" +def _expected_period_text(period: dict) -> str: + """A fixture's ``period``: ``start`` and ``end`` dates, or a relative range as AaC writes it.""" + if _is_relative(period): + return f"the relative range {period.get('granularity')} {period.get('from')} to {period.get('to')}" + return f"{period.get('start')} to {period.get('end')}" + + def render_document_text(document: dict) -> str: """The document as the judge reads it: title, period, and per content page its heading, charts and summaries.""" lines = [f"Document: {document.get('title')}", f"Period: {_period_text(document)}"] @@ -331,6 +370,10 @@ def _has_visualizations(expected_output: dict) -> bool: return "visualizations" in expected_output +def _has_chart_periods(expected_output: dict) -> bool: + return "chart_periods" in expected_output + + def _expects_clarification(expected_output: dict) -> bool: return "expects_clarification" in expected_output @@ -350,9 +393,17 @@ def _validate_rework(rework: Any) -> None: A rework is scored on aac-v2 documents only. ``keeps_pages`` checks that each page id is still visible, not what the page shows. ``page_charts`` maps a kept page to the charts that must stay - on it; the page may hold others too. ``visualizations`` checks the charts anywhere in the document. - ``filters`` is compared with ``==`` against the drafted document's ``content.filters``, so it - must be copied from a real aac-v2 run: the wire form is not the shape a filter is authored in. + on it; the page may hold others too. ``new_page_charts`` lists the charts that must sit on a + visible page outside ``keeps_pages``, which is what "add a page" asks for; ``visualizations`` + alone accepts a chart pushed onto an old page. ``filters`` maps each filter id the document must + have, and no other, to the keys it is compared on, in the drafted document's ``content.filters`` + wire form. Each entry states at least the filter's ``type``, an attribute filter also its + ``using`` and ``state``, with a ``null`` state for all, and a date filter its ``date``. Other keys + an entry leaves out are + not compared, so a converter that adds one does not fail the run, except a date filter's + ``granularity``, ``from`` and ``to``: leaving out ``from`` and ``to`` states all time, and leaving + out ``granularity`` states none. An attribute filter's ``state`` is compared on its selection: no + values means all. """ if not isinstance(rework, dict): raise ValueError(f"rework must be an object, got {type(rework).__name__}") @@ -380,12 +431,36 @@ def _validate_rework(rework: Any) -> None: raise ValueError(f"rework.page_charts[{page_id!r}] must list the charts that stay on the page") if not all(isinstance(chart, str) and chart.strip() for chart in charts): raise ValueError(f"every rework.page_charts[{page_id!r}] entry must be a chart id, got {charts!r}") + if "new_page_charts" in rework: + new_page_charts = rework["new_page_charts"] + if not isinstance(new_page_charts, list) or not new_page_charts: + raise ValueError("rework.new_page_charts is empty; the new page check would pass vacuously") + if not all(isinstance(chart, str) and chart.strip() for chart in new_page_charts): + raise ValueError(f"every rework.new_page_charts entry must be a chart id, got {new_page_charts!r}") + if "keeps_pages" not in rework: + raise ValueError("rework.new_page_charts needs rework.keeps_pages to tell the open document's pages apart") if "title" in rework: title = rework["title"] if not isinstance(title, str) or not title.strip(): raise ValueError(f"rework.title must be the title the edit leaves, got {title!r}") - if "filters" in rework and not isinstance(rework["filters"], dict): - raise ValueError(f"rework.filters must be the document's filters keyed by id, got {rework['filters']!r}") + if "filters" in rework: + filters = rework["filters"] + if not isinstance(filters, dict): + raise ValueError(f"rework.filters must be the document's filters keyed by id, got {filters!r}") + for filter_id, entry in filters.items(): + if not isinstance(entry, dict) or not isinstance(entry.get("type"), str) or not entry["type"].strip(): + raise ValueError( + f"rework.filters[{filter_id!r}] must state at least the filter's 'type', got {entry!r}" + ) + unstated = [key for key in _REQUIRED_FILTER_KEYS.get(entry["type"], ()) if key not in entry] + if unstated: + raise ValueError( + f"rework.filters[{filter_id!r}] of type {entry['type']!r} leaves out {unstated}: an unstated key " + f"is not compared, so what the filter applies to would go unchecked; a null state means all" + ) + problem = _state_problem(entry.get("state"), entry["type"]) + if problem is not None: + raise ValueError(f"rework.filters[{filter_id!r}].state is {entry['state']!r}, {problem}") def _validate_expectation(expected_output: Any) -> None: @@ -406,8 +481,20 @@ def _validate_expectation(expected_output: Any) -> None: raise ValueError(f"unknown expected_output key(s) {unknown}; known: {sorted(_EXPECTATION_KEYS)}") if _has_period(expected_output): period = expected_output.get("period") - if not isinstance(period, dict) or not period.get("start") or not period.get("end"): - raise ValueError(f"period needs both 'start' and 'end', got {period!r}") + if isinstance(period, dict) and _is_relative(period): + granularity = period["granularity"] + if not isinstance(granularity, str) or granularity not in _AAC_PERIOD_GRANULARITIES: + raise ValueError( + f"a relative period's 'granularity' must be one AaC writes, one of " + f"{sorted(_AAC_PERIOD_GRANULARITIES)}, got {period!r}" + ) + if not all( + isinstance(period.get(bound), int) and not isinstance(period.get(bound), bool) + for bound in ("from", "to") + ): + raise ValueError(f"a relative period needs whole numbers 'from' and 'to', got {period!r}") + elif not isinstance(period, dict) or not period.get("start") or not period.get("end"): + raise ValueError(f"period needs both 'start' and 'end', or a relative 'granularity', got {period!r}") if _has_visualizations(expected_output): visualizations = expected_output.get("visualizations") if not isinstance(visualizations, list) or not visualizations: @@ -415,6 +502,18 @@ def _validate_expectation(expected_output: Any) -> None: for entry in visualizations: if not isinstance(entry, dict) or not entry.get("id"): raise ValueError(f"every visualization needs an 'id', got {entry!r}") + if _has_chart_periods(expected_output): + chart_periods = expected_output.get("chart_periods") + if not isinstance(chart_periods, dict) or not chart_periods: + raise ValueError( + "chart_periods must map chart ids to the date dataset each applies the period on, " + f"or null for a chart that ignores it, got {chart_periods!r}" + ) + for chart, dataset in chart_periods.items(): + if not isinstance(chart, str) or not chart.strip(): + raise ValueError(f"every chart_periods key must be a chart id, got {chart!r}") + if dataset is not None and (not isinstance(dataset, str) or not dataset.strip()): + raise ValueError(f"chart_periods[{chart!r}] must be a date dataset id or null, got {dataset!r}") if _has_narrative(expected_output): narrative = expected_output.get("narrative") if not isinstance(narrative, str) or not narrative.strip(): @@ -463,7 +562,8 @@ def _validate_rework_target(expected_output: dict, user_context: dict | None) -> Raises: ValueError: the item states a rework, and ``user_context.view.document`` is missing, - has no id, or has another id. + has no id, or has another id; or it states ``new_page_charts`` while the open document + sends no pages or has a hidden page, or ``keeps_pages`` leaves out one of its pages. """ rework = _rework(expected_output) if rework is None: @@ -477,6 +577,32 @@ def _validate_rework_target(expected_output: dict, user_context: dict | None) -> raise ValueError(f"rework needs the open document's id in user_context.view.document.id, got {open_id!r}") if open_id != rework["base_document_id"]: raise ValueError(f"rework.base_document_id {rework['base_document_id']!r} is not the open document {open_id!r}") + if "new_page_charts" in rework: + # A new page is one outside keeps_pages, so an open page left off the list would count as new, + # and a chart pushed onto it would pass. A hidden open page cannot be listed, since a kept page + # must stay visible, so an item over a document with one cannot state new_page_charts. + definition = document.get("definition") + open_pages = _pages(definition) if isinstance(definition, dict) else [] + if not open_pages: + raise ValueError( + "rework.new_page_charts needs the open document's pages in user_context.view.document.definition" + ) + hidden = [page.get("id") for page in open_pages if isinstance(page, dict) and page.get("visible") is False] + if hidden: + raise ValueError( + f"rework.new_page_charts cannot be scored over an open document with hidden pages {hidden!r}: " + "showing one again would count as a new page" + ) + unlisted = [ + page.get("id") + for page in open_pages + if isinstance(page, dict) and page.get("id") not in rework["keeps_pages"] + ] + if unlisted: + raise ValueError( + f"rework.new_page_charts needs keeps_pages to list every page of the open document; " + f"missing {unlisted!r}" + ) def build_simulated_reply(expected_output: dict, *, asked: bool = True) -> str: @@ -496,7 +622,7 @@ def build_simulated_reply(expected_output: dict, *, asked: bool = True) -> str: segments.append(f"Please use these charts: {titles}.") period = expected_output.get("period") if isinstance(period, dict): - segments.append(f"Period: {period.get('start')} to {period.get('end')}.") + segments.append(f"Period: {_expected_period_text(period)}.") if _rework(expected_output) is not None: segments.append("Anything else is up to you. Please make the change in the open document now.") else: @@ -519,8 +645,10 @@ class _Applies: rework: bool = False pages_kept: bool = False page_charts: bool = False + new_page_charts: bool = False title: bool = False filters: bool = False + chart_periods: bool = False @dataclass @@ -544,8 +672,10 @@ class DocumentEvaluation: reworks_open: bool = False pages_kept: bool = False page_charts_kept: bool = False + new_page_charts_placed: bool = False title_correct: bool = False filters_kept: bool = False + chart_periods_correct: bool = False judge_reasoning: str = "" # Set when the judge returned something unreadable. The run then has no narrative verdict: # it is neither published as a 0 nor allowed to pass on the remaining checks. @@ -600,10 +730,14 @@ def strict_checks(self) -> dict[str, bool]: checks["document_pages_kept"] = self.pages_kept if self.applies.page_charts: checks["document_page_charts_kept"] = self.page_charts_kept + if self.applies.new_page_charts: + checks["document_new_page_charts"] = self.new_page_charts_placed if self.applies.title: checks["document_title_correct"] = self.title_correct if self.applies.filters: checks["document_filters_kept"] = self.filters_kept + if self.applies.chart_periods: + checks["document_chart_periods"] = self.chart_periods_correct return checks @@ -654,10 +788,107 @@ def evaluate_document_response( return evaluation +def _state_problem(state: Any, kind: str) -> str | None: + """Why a filter's ``state`` cannot be read as a selection, or nothing. + + A missing or ``null`` state reads as all, as does an empty object; any other falsey value is malformed. + """ + if state is None: + return None + if not isinstance(state, dict): + return "expected an object" + for key in ("include", "exclude"): + if state.get(key) is not None and not isinstance(state[key], list): + return f"expected a list of values under {key!r}" + if state.get("include") and state.get("exclude"): + return "expected either 'include' or 'exclude', not both" + if kind == "attribute_filter" and not set(state) <= {"include", "exclude"}: + return "expected only 'include' or 'exclude'" + return None + + +def _filter_mismatch(filter_id: str, actual: Any, expected: dict) -> list[str]: + """Why one drafted filter differs from the expectation, or nothing. + + The kind must match. An attribute filter's ``state`` is compared on its selection, a date + filter on its range with absence significant, and any other key the expectation states as written. + """ + if not isinstance(actual, dict): + return [f"filter {filter_id!r} is {actual!r}, expected an object"] + kind = expected["type"] + if actual.get("type") != kind: + return [f"filter {filter_id!r} has type {actual.get('type')!r}, expected {kind!r}"] + keys = [key for key in expected if key != "type"] + if kind == "date_filter": + keys += [key for key in _DATE_FILTER_RANGE_KEYS if key not in expected] + reasons: list[str] = [] + for key in keys: + if key == "state" and kind == "attribute_filter": + problem = _state_problem(actual.get("state"), kind) + if problem is not None: + reasons.append(f"filter {filter_id!r} has state {actual.get('state')!r}, {problem}") + elif not same_selection(selection_of(actual), selection_of(expected)): + reasons.append( + f"filter {filter_id!r} selects {describe_selection(selection_of(actual))}, " + f"expected {describe_selection(selection_of(expected))}" + ) + elif actual.get(key) != expected.get(key): + reasons.append(f"filter {filter_id!r} has {key} {actual.get(key)!r}, expected {expected.get(key)!r}") + return reasons + + +def _new_page_misses(new_page_charts: list[str], keeps_pages: list[str], pages: list[Any]) -> list[str]: + """Why a chart the edit should have put on a new page is not on one.""" + kept = set(keeps_pages) + reasons: list[str] = [] + for chart in new_page_charts: + holders = [page.get("id") for page in pages if isinstance(page, dict) and chart in _page_visualizations(page)] + if any(page_id not in kept for page_id in holders): + continue + if holders: + reasons.append( + f"chart {chart!r} must be on a new page, but it is only on page(s) {holders!r} of the open document" + ) + else: + reasons.append(f"chart {chart!r} must be on a new page, but no visible page has it") + return reasons + + +def _chart_period_misses(chart_periods: dict[str, str | None], document: dict) -> list[str]: + """Why charts do not apply the document period where the item says, or nothing. + + A chart applies the period on its widget's ``date``; a widget with ``ignore_document_period`` + applies none, as a dashboard chart the date filter does not reach. A dataset of ``None`` asks + for the latter. Every placement of a chart on a page a reader sees must agree. + """ + placements: dict[str, list[dict]] = {} + for page in _visible_pages(document): + if isinstance(page, dict): + for node in _nodes(page.get("layout")): + if (chart := _visualization_id(node)) is not None: + placements.setdefault(chart, []).append(node) + misses: list[str] = [] + for chart, expected in chart_periods.items(): + widgets = placements.get(chart) + if not widgets: + misses.append(f"chart {chart!r} is not in the document, so where it applies the period is unknown") + continue + for widget in widgets: + ignores = widget.get("ignore_document_period") is True + if expected is None and not ignores: + misses.append( + f"chart {chart!r} applies the period on {widget.get('date')!r}, expected it to ignore the period" + ) + elif expected is not None and (ignores or widget.get("date") != expected): + applied = "ignores the period" if ignores else f"applies the period on {widget.get('date')!r}" + misses.append(f"chart {chart!r} {applied}, expected it on {expected!r}") + return list(dict.fromkeys(misses)) + + def _score_rework( rework: dict, document: dict, document_part: dict, pages: list[Any], failures: list[str] -) -> tuple[bool, bool, bool, bool, bool]: - """``(reworks_open, pages_kept, page_charts_kept, title_correct, filters_kept)`` of a v2 edit. +) -> tuple[bool, bool, bool, bool, bool, bool]: + """``(reworks_open, pages_kept, page_charts_kept, new_page_charts_placed, title_correct, filters_kept)`` of a v2 edit. A check the item does not state is False. """ @@ -694,6 +925,12 @@ def _score_rework( ) page_charts_kept = not moved + new_page_charts_placed = False + if "new_page_charts" in rework: + misses = _new_page_misses(rework["new_page_charts"], rework["keeps_pages"], pages) + failures.extend(misses) + new_page_charts_placed = not misses + title_correct = False if "title" in rework: title_correct = document.get("title") == rework["title"] @@ -703,11 +940,29 @@ def _score_rework( filters_kept = False if "filters" in rework: # A document without filters omits the key, which matches an expectation of none. - actual_filters = _filters(document) or {} - filters_kept = actual_filters == rework["filters"] - if not filters_kept: - failures.append(f"the document filters are {actual_filters!r}, expected {rework['filters']!r}") - return reworks_open, pages_kept, page_charts_kept, title_correct, filters_kept + actual_filters = _filters(document) + if actual_filters is None: + actual_filters = {} + expected_filters = rework["filters"] + if not isinstance(actual_filters, dict): + reasons = [f"the document filters are {actual_filters!r}, expected an object keyed by filter id"] + else: + reasons = [ + f"the edit lost filter {filter_id!r}" + for filter_id in expected_filters + if filter_id not in actual_filters + ] + reasons += [ + f"the edit added filter {filter_id!r}" + for filter_id in actual_filters + if filter_id not in expected_filters + ] + for filter_id, expected in expected_filters.items(): + if filter_id in actual_filters: + reasons += _filter_mismatch(filter_id, actual_filters[filter_id], expected) + failures.extend(reasons) + filters_kept = not reasons + return reworks_open, pages_kept, page_charts_kept, new_page_charts_placed, title_correct, filters_kept def _score_response( @@ -726,8 +981,10 @@ def _score_response( rework=rework is not None, pages_kept=rework is not None and "keeps_pages" in rework, page_charts=rework is not None and "page_charts" in rework, + new_page_charts=rework is not None and "new_page_charts" in rework, title=rework is not None and "title" in rework, filters=rework is not None and "filters" in rework, + chart_periods=_has_chart_periods(expected_output), ) if tool_result is None: @@ -786,7 +1043,8 @@ def _score_response( failures.append(f"{draft} must not be saved yet, but it reports saved_document_id {saved_document_id!r}") not_saved = False - new_draft = reworks_open = pages_kept = page_charts_kept = title_correct = filters_kept = False + new_draft = reworks_open = pages_kept = page_charts_kept = new_page_charts_placed = False + title_correct = filters_kept = False if rework is None: new_draft = True base_document_id = document_part.get("base_document_id") @@ -803,18 +1061,23 @@ def _score_response( f"a rework is scored on aac-v2 documents only, but the draft is version {document.get('version')!r}" ) else: - reworks_open, pages_kept, page_charts_kept, title_correct, filters_kept = _score_rework( + reworks_open, pages_kept, page_charts_kept, new_page_charts_placed, title_correct, filters_kept = _score_rework( rework, document, document_part, pages, failures ) period_correct = False if applies.period: expected_period = expected_output["period"] - period_correct = _period_bounds(document) == (expected_period.get("start"), expected_period.get("end")) + if _is_relative(expected_period): + actual = _period(document) if _is_v2(document) else {} + period_correct = _is_relative(actual) and all( + actual.get(key) == expected_period[key] for key in ("granularity", "from", "to") + ) + else: + period_correct = _period_bounds(document) == (expected_period.get("start"), expected_period.get("end")) if not period_correct: failures.append( - f"the document covers {_period_text(document)}, " - f"expected {expected_period.get('start')} to {expected_period.get('end')}" + f"the document covers {_period_text(document)}, expected {_expected_period_text(expected_period)}" ) charts_matched = False @@ -824,6 +1087,15 @@ def _score_response( failures.extend(f"the document does not show chart {v.get('id')!r} ({v.get('title')!r})" for v in missing) charts_matched = not missing + chart_periods_correct = False + if applies.chart_periods: + if _is_v2(document): + misses = _chart_period_misses(expected_output["chart_periods"], document) + else: + misses = ["where a chart applies the period is scored on aac-v2 documents only"] + failures.extend(misses) + chart_periods_correct = not misses + summaries_present = False if applies.narrative: slots = [(page, slot) for _number, page in _content_pages(document) for slot in _summary_slots(page)] @@ -853,8 +1125,10 @@ def _score_response( reworks_open=reworks_open, pages_kept=pages_kept, page_charts_kept=page_charts_kept, + new_page_charts_placed=new_page_charts_placed, title_correct=title_correct, filters_kept=filters_kept, + chart_periods_correct=chart_periods_correct, failures=failures, ) diff --git a/packages/gooddata-eval/tests/test_agentic_document_skill.py b/packages/gooddata-eval/tests/test_agentic_document_skill.py index 364b06324..ac9d7a6f8 100644 --- a/packages/gooddata-eval/tests/test_agentic_document_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_document_skill.py @@ -99,10 +99,10 @@ def _document_part( } -# v2 shapes follow the Publisher content v2 model: a top-level `version`, an absolute or relative -# `period`, and `content` holding the pages, whose layout nests `row` and `column` containers -# through `children` down to typed widgets. -_V2_PERIOD = {"type": "absolute", "granularity": "GDC.time.date", "from": "2026-01-01", "to": "2026-06-30"} +# v2 shapes follow the AaC form a v2 draft arrives in: a top-level `version`, an absolute `{from, to}` +# or relative `{granularity, from, to}` `period`, and `content` holding the pages, whose layout nests +# `row` and `column` containers through `children` down to typed widgets. +_V2_PERIOD = {"from": "2026-01-01", "to": "2026-06-30"} def _v2_cover_page() -> dict: @@ -423,7 +423,7 @@ def _rework_part(pages: list[dict] | None = None, **kwargs: Any) -> dict: return _v2_document_part(pages, **kwargs) -def _with_filters(part: dict, filters: dict) -> dict: +def _with_filters(part: dict, filters: Any) -> dict: part["document"]["content"]["filters"] = filters return part @@ -465,6 +465,21 @@ def test_an_edit_of_the_open_document_passes_every_rework_check() -> None: } +def test_every_rework_check_is_published_when_the_fixture_states_it() -> None: + pages = [ + _v2_cover_page(), + _v2_content_page("page2", _REVENUE_TREND), + _v2_content_page("page3", _RETURNS_BY_CATEGORY), + ] + rework = {**_NEW_PAGE_REWORK, "page_charts": {"page2": [_REVENUE_TREND]}, "filters": _US_ONLY} + part = _with_filters(_rework_part(pages), _drafted_country_filter()) + evaluation = _evaluate(part, expected={"rework": rework}, tool=_draft_result(page_count=3)) + assert evaluation.failures == [] + assert {"document_new_page_charts", "document_page_charts_kept", "document_filters_kept"} <= set( + evaluation.strict_checks + ) + + def test_a_rework_of_a_v1_document_fails_once_and_says_why() -> None: expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "keeps_pages": ["page1"], "title": "Sales overview"}} evaluation = _evaluate(_document_part(base_document_id=_OPEN_DOCUMENT), expected=expected) @@ -611,12 +626,328 @@ def test_a_rename_that_left_the_old_title_fails() -> None: assert evaluation.failures == ["the document is titled 'Sales overview', expected 'E2 Sales Review'"] -def test_an_edit_that_changed_the_document_filters_fails() -> None: +def test_an_edit_that_dropped_a_document_filter_fails() -> None: part = _with_filters(_rework_part(), {}) expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _OPEN_FILTERS}} evaluation = _evaluate(part, expected=expected) assert evaluation.strict_checks["document_filters_kept"] is False - assert evaluation.failures == [f"the document filters are {{}}, expected {_OPEN_FILTERS!r}"] + assert evaluation.failures == ["the edit lost filter 'document_filter_customer_country'"] + + +# What a rework item states: the filter's kind, label and selection, not its display fields. +_US_ONLY = { + "document_filter_customer_country": { + "type": "attribute_filter", + "using": "label/customer_country", + "state": {"include": ["United States"]}, + } +} + + +def _drafted_country_filter(**changes: Any) -> dict: + drafted = { + "type": "attribute_filter", + "title": "Customer country", + "using": "label/customer_country", + "multiselect": False, + "state": {"include": ["United States"]}, + } + drafted.update(changes) + return {"document_filter_customer_country": {k: v for k, v in drafted.items() if v is not None}} + + +@pytest.mark.parametrize( + "drafted", + [ + _drafted_country_filter(), + _drafted_country_filter(localIdentifier="f1"), + _drafted_country_filter(title=None, multiselect=None), + _drafted_country_filter(title="Country"), + ], + ids=["as-drafted", "extra-key", "display-keys-dropped", "retitled"], +) +def test_a_filter_is_compared_only_on_the_keys_the_fixture_states(drafted: dict) -> None: + part = _with_filters(_rework_part(), drafted) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _US_ONLY}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_filters_kept"] is True + + +@pytest.mark.parametrize( + ("drafted", "failure"), + [ + ( + _drafted_country_filter(state={}), + "filter 'document_filter_customer_country' selects all, expected include ['United States']", + ), + ( + _drafted_country_filter(state={"include": []}), + "filter 'document_filter_customer_country' selects all, expected include ['United States']", + ), + ( + _drafted_country_filter(state={"include": ["Canada"]}), + "filter 'document_filter_customer_country' selects include ['Canada'], expected include ['United States']", + ), + ( + _drafted_country_filter(state={"exclude": ["United States"]}), + "filter 'document_filter_customer_country' selects exclude ['United States'], " + "expected include ['United States']", + ), + ( + _drafted_country_filter(using="label/customer_state"), + "filter 'document_filter_customer_country' has using 'label/customer_state', " + "expected 'label/customer_country'", + ), + ], + ids=["selection-dropped", "selection-emptied", "other-value", "inverted", "other-label"], +) +def test_a_filter_that_changed_what_it_selects_fails(drafted: dict, failure: str) -> None: + part = _with_filters(_rework_part(), drafted) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _US_ONLY}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [failure] + + +def test_an_edit_that_added_a_document_filter_fails() -> None: + drafted = {**_drafted_country_filter(), "document_filter_product_category": {"type": "attribute_filter"}} + part = _with_filters(_rework_part(), drafted) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _US_ONLY}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == ["the edit added filter 'document_filter_product_category'"] + + +def test_a_filter_of_another_kind_fails() -> None: + drafted = _drafted_country_filter(type="text_filter") + part = _with_filters(_rework_part(), drafted) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _US_ONLY}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [ + "filter 'document_filter_customer_country' has type 'text_filter', expected 'attribute_filter'" + ] + + +_ALL_TIME = {"document_filter_date": {"type": "date_filter", "granularity": "YEAR", "date": "date"}} + + +@pytest.mark.parametrize( + ("drafted", "failure"), + [ + ( + {"type": "date_filter", "granularity": "YEAR", "date": "date", "from": -1, "to": -1}, + "filter 'document_filter_date' has from -1, expected None", + ), + ( + {"type": "date_filter", "granularity": "MONTH", "date": "date"}, + "filter 'document_filter_date' has granularity 'MONTH', expected 'YEAR'", + ), + ], + ids=["range-added", "other-granularity"], +) +def test_a_date_filter_left_all_time_fails_on_a_drafted_range(drafted: dict, failure: str) -> None: + part = _with_filters(_rework_part(), {"document_filter_date": drafted}) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _ALL_TIME}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert failure in evaluation.failures + + +def test_a_date_range_stated_without_granularity_fails_on_a_drafted_granularity() -> None: + drafted = { + "document_filter_date": {"type": "date_filter", "granularity": "MONTH", "date": "date", "from": -1, "to": -1} + } + part = _with_filters(_rework_part(), drafted) + expected = { + "rework": { + "base_document_id": _OPEN_DOCUMENT, + "filters": {"document_filter_date": {"type": "date_filter", "date": "date", "from": -1, "to": -1}}, + } + } + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == ["filter 'document_filter_date' has granularity 'MONTH', expected None"] + + +def test_a_date_filter_kept_all_time_passes() -> None: + part = _with_filters(_rework_part(), {"document_filter_date": dict(_ALL_TIME["document_filter_date"])}) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _ALL_TIME}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_filters_kept"] is True + + +@pytest.mark.parametrize("dataset", ["customer_created_date", None], ids=["other-dataset", "dataset-dropped"]) +def test_a_date_filter_moved_off_its_date_dataset_fails(dataset: str | None) -> None: + drafted = {k: v for k, v in _ALL_TIME["document_filter_date"].items() if k != "date"} + if dataset is not None: + drafted["date"] = dataset + part = _with_filters(_rework_part(), {"document_filter_date": drafted}) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _ALL_TIME}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [f"filter 'document_filter_date' has date {dataset!r}, expected 'date'"] + + +@pytest.mark.parametrize( + ("drafted", "problem"), + [ + (["United States"], "expected an object"), + ("United States", "expected an object"), + ({"include": 1}, "expected a list of values under 'include'"), + ({"exclude": "Canada"}, "expected a list of values under 'exclude'"), + ([], "expected an object"), + ("", "expected an object"), + (False, "expected an object"), + ( + {"include": ["United States"], "exclude": ["Canada"]}, + "expected either 'include' or 'exclude', not both", + ), + ({"values": ["United States"]}, "expected only 'include' or 'exclude'"), + ], + ids=[ + "list", + "string", + "include-not-a-list", + "exclude-not-a-list", + "empty-list", + "empty-string", + "false", + "both", + "unknown-key", + ], +) +def test_a_drafted_state_that_is_not_a_selection_fails(drafted: Any, problem: str) -> None: + part = _with_filters(_rework_part(), _drafted_country_filter(state=drafted)) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": _US_ONLY}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [f"filter 'document_filter_customer_country' has state {drafted!r}, {problem}"] + + +@pytest.mark.parametrize("drafted", [[], "", False], ids=["empty-list", "empty-string", "false"]) +def test_a_falsey_drafted_state_does_not_pass_as_all(drafted: Any) -> None: + unrestricted = {"type": "attribute_filter", "using": "label/customer_country", "state": {}} + part = _with_filters(_rework_part(), _drafted_country_filter(state=drafted)) + expected = { + "rework": {"base_document_id": _OPEN_DOCUMENT, "filters": {"document_filter_customer_country": unrestricted}} + } + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + + +@pytest.mark.parametrize( + ("drafted", "kept"), [(None, True), ({"include": ["United States"]}, False)], ids=["all", "one-value"] +) +def test_a_null_expected_state_means_all(drafted: Any, kept: bool) -> None: + unrestricted = {"type": "attribute_filter", "using": "label/customer_country", "state": None} + part = _with_filters(_rework_part(), _drafted_country_filter(state=drafted)) + expected = { + "rework": {"base_document_id": _OPEN_DOCUMENT, "filters": {"document_filter_customer_country": unrestricted}} + } + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is kept + + +def test_a_selection_is_compared_without_its_order() -> None: + both = { + "type": "attribute_filter", + "using": "label/customer_country", + "state": {"include": ["United States", "Canada"]}, + } + reordered = _drafted_country_filter(state={"include": ["Canada", "United States"]}) + part = _with_filters(_rework_part(), reordered) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": {"document_filter_customer_country": both}}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_filters_kept"] is True + + +def test_a_selection_with_a_value_missing_fails() -> None: + both = { + "type": "attribute_filter", + "using": "label/customer_country", + "state": {"include": ["United States", "Canada"]}, + } + part = _with_filters(_rework_part(), _drafted_country_filter()) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": {"document_filter_customer_country": both}}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [ + "filter 'document_filter_customer_country' selects include ['United States'], " + "expected include ['United States', 'Canada']" + ] + + +@pytest.mark.parametrize("drafted", [[{"type": "attribute_filter"}], "none"], ids=["list", "string"]) +def test_drafted_filters_that_are_not_keyed_by_id_fail_an_expectation_of_none(drafted: Any) -> None: + part = _with_filters(_rework_part(), drafted) + expected = {"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": {}}} + evaluation = _evaluate(part, expected=expected) + assert evaluation.strict_checks["document_filters_kept"] is False + assert evaluation.failures == [f"the document filters are {drafted!r}, expected an object keyed by filter id"] + + +def test_a_text_filter_state_is_compared_as_written() -> None: + expected_filter = {"document_filter_city": {"type": "text_filter", "state": {"values": ["Prague"]}}} + drafted = {"document_filter_city": {"type": "text_filter", "state": {"values": ["Brno"]}}} + evaluation = _evaluate( + _with_filters(_rework_part(), drafted), + expected={"rework": {"base_document_id": _OPEN_DOCUMENT, "filters": expected_filter}}, + ) + assert evaluation.failures == [ + "filter 'document_filter_city' has state {'values': ['Brno']}, expected {'values': ['Prague']}" + ] + + +# ── a chart the edit adds on a new page ───────────────────────────────────── + +_NEW_PAGE_REWORK = { + "base_document_id": _OPEN_DOCUMENT, + "keeps_pages": ["page1", "page2"], + "new_page_charts": [_RETURNS_BY_CATEGORY], +} + + +def test_a_chart_on_a_new_page_passes() -> None: + pages = [ + _v2_cover_page(), + _v2_content_page("page2", _REVENUE_TREND), + _v2_content_page("page3", _RETURNS_BY_CATEGORY), + ] + evaluation = _evaluate(_rework_part(pages), expected={"rework": _NEW_PAGE_REWORK}, tool=_draft_result(page_count=3)) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_new_page_charts"] is True + + +def test_a_chart_pushed_onto_an_old_page_fails() -> None: + pages = [_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND, _RETURNS_BY_CATEGORY)] + evaluation = _evaluate(_rework_part(pages), expected={"rework": _NEW_PAGE_REWORK}) + assert evaluation.strict_checks["document_new_page_charts"] is False + assert evaluation.failures == [ + "chart 'returns_by_category' must be on a new page, but it is only on page(s) ['page2'] of the open document" + ] + + +def test_a_chart_on_both_an_old_and_a_new_page_passes() -> None: + pages = [ + _v2_cover_page(), + _v2_content_page("page2", _REVENUE_TREND, _RETURNS_BY_CATEGORY), + _v2_content_page("page3", _RETURNS_BY_CATEGORY), + ] + evaluation = _evaluate(_rework_part(pages), expected={"rework": _NEW_PAGE_REWORK}, tool=_draft_result(page_count=3)) + assert evaluation.strict_checks["document_new_page_charts"] is True + + +def test_a_chart_on_a_hidden_new_page_fails() -> None: + hidden = _v2_content_page("page3", _RETURNS_BY_CATEGORY) + hidden["visible"] = False + pages = [_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND), hidden] + evaluation = _evaluate(_rework_part(pages), expected={"rework": _NEW_PAGE_REWORK}) + assert evaluation.strict_checks["document_new_page_charts"] is False + assert evaluation.failures == ["chart 'returns_by_category' must be on a new page, but no visible page has it"] def test_a_document_without_filters_keeps_an_expectation_of_none() -> None: @@ -628,7 +959,13 @@ def test_a_document_without_filters_keeps_an_expectation_of_none() -> None: def test_rework_checks_are_published_only_for_what_the_fixture_states() -> None: evaluation = _evaluate(_rework_part(), expected={"rework": {"base_document_id": _OPEN_DOCUMENT}}) assert "document_reworks_open" in evaluation.strict_checks - for name in ("document_pages_kept", "document_page_charts_kept", "document_title_correct", "document_filters_kept"): + for name in ( + "document_pages_kept", + "document_page_charts_kept", + "document_new_page_charts", + "document_title_correct", + "document_filters_kept", + ): assert name not in evaluation.strict_checks assert evaluation.strict_pass @@ -803,6 +1140,51 @@ def test_a_rework_that_does_not_match_the_open_document_fails_before_any_request assert client.sent == [] +def _open_document_with(pages: list[dict]) -> dict: + return {"view": {"document": {"id": _OPEN_DOCUMENT, "definition": _v2_document(pages)}}} + + +def test_a_new_page_rework_must_list_every_open_page(monkeypatch: pytest.MonkeyPatch) -> None: + client = _ScriptedChatClient([_drafting_turn()]) + _install_client(monkeypatch, client) + user_context = _open_document_with([_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND)]) + expected = {"rework": {**_NEW_PAGE_REWORK, "keeps_pages": ["page1"]}} + with pytest.raises(ValueError, match=r"keeps_pages to list every page of the open document; missing \['page2'\]"): + run_agentic_document_skill("https://h", "tok", "ws", "Add a page", expected, user_context=user_context) + assert client.sent == [] + + +def test_a_new_page_rework_without_the_open_pages_fails_before_any_request(monkeypatch: pytest.MonkeyPatch) -> None: + client = _ScriptedChatClient([_drafting_turn()]) + _install_client(monkeypatch, client) + user_context = {"view": {"document": {"id": _OPEN_DOCUMENT, "title": "Q3 review"}}} + with pytest.raises(ValueError, match="needs the open document's pages"): + run_agentic_document_skill( + "https://h", "tok", "ws", "Add a page", {"rework": _NEW_PAGE_REWORK}, user_context=user_context + ) + assert client.sent == [] + + +def test_a_new_page_rework_over_a_hidden_open_page_fails_before_any_request(monkeypatch: pytest.MonkeyPatch) -> None: + client = _ScriptedChatClient([_drafting_turn()]) + _install_client(monkeypatch, client) + hidden = {**_v2_content_page("page3", _REVENUE_TREND), "visible": False} + user_context = _open_document_with([_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND), hidden]) + expected = {"rework": _NEW_PAGE_REWORK} + with pytest.raises(ValueError, match=r"open document with hidden pages \['page3'\]"): + run_agentic_document_skill("https://h", "tok", "ws", "Add a page", expected, user_context=user_context) + assert client.sent == [] + + +def test_a_new_page_rework_listing_every_open_page_reaches_the_chat(monkeypatch: pytest.MonkeyPatch) -> None: + client = _ScriptedChatClient([_drafting_turn()]) + _install_client(monkeypatch, client) + user_context = _open_document_with([_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND)]) + expected = {"rework": _NEW_PAGE_REWORK} + run_agentic_document_skill("https://h", "tok", "ws", "Add a page", expected, user_context=user_context) + assert client.sent == ["Add a page"] + + def test_a_rework_of_the_open_document_reaches_the_chat(monkeypatch: pytest.MonkeyPatch) -> None: client = _ScriptedChatClient([_drafting_turn()]) _install_client(monkeypatch, client) @@ -920,6 +1302,69 @@ def test_the_document_format_is_recorded_in_the_run_metadata( ), ({"rework": {"base_document_id": "q3", "title": " "}}, "rework.title must be"), ({"rework": {"base_document_id": "q3", "filters": ["customer_country"]}}, "rework.filters must be"), + ({"rework": {"base_document_id": "q3", "filters": {"f1": {}}}}, r"rework.filters\['f1'\] must state at least"), + ( + {"rework": {"base_document_id": "q3", "filters": {"f1": {"state": {"include": ["US"]}}}}}, + r"rework.filters\['f1'\] must state at least the filter's 'type'", + ), + ( + { + "rework": { + "base_document_id": "q3", + "filters": {"f1": {"type": "attribute_filter", "using": "label/country", "state": ["US"]}}, + } + }, + r"rework.filters\['f1'\].state is \['US'\], expected an object", + ), + ( + { + "rework": { + "base_document_id": "q3", + "filters": { + "f1": {"type": "attribute_filter", "using": "label/country", "state": {"include": "US"}} + }, + } + }, + r"rework.filters\['f1'\].state is \{'include': 'US'\}, expected a list of values under 'include'", + ), + ( + { + "rework": { + "base_document_id": "q3", + "filters": { + "f1": {"type": "attribute_filter", "using": "label/country", "state": {"values": ["US"]}} + }, + } + }, + r"rework.filters\['f1'\].state is \{'values': \['US'\]\}, expected only 'include' or 'exclude'", + ), + ( + {"rework": {"base_document_id": "q3", "filters": {"f1": {"type": "attribute_filter", "state": None}}}}, + r"rework.filters\['f1'\] of type 'attribute_filter' leaves out \['using'\]", + ), + ( + {"rework": {"base_document_id": "q3", "filters": {"f1": {"type": "attribute_filter", "using": "label/c"}}}}, + r"rework.filters\['f1'\] of type 'attribute_filter' leaves out \['state'\]", + ), + ( + {"rework": {"base_document_id": "q3", "filters": {"d": {"type": "date_filter", "granularity": "YEAR"}}}}, + r"rework.filters\['d'\] of type 'date_filter' leaves out \['date'\]", + ), + ( + {"rework": {"base_document_id": "q3", "keeps_pages": ["p2"], "new_page_charts": []}}, + "new_page_charts is empty", + ), + ( + {"rework": {"base_document_id": "q3", "keeps_pages": ["p2"], "new_page_charts": [" "]}}, + "every rework.new_page_charts entry must be a chart id", + ), + ({"rework": {"base_document_id": "q3", "new_page_charts": ["c1"]}}, "new_page_charts needs rework.keeps_pages"), + ({"period": {"from": -6, "to": -1}}, "period needs both 'start' and 'end', or a relative 'granularity'"), + ({"period": {"granularity": "GDC.time.month", "from": -6, "to": -1}}, "must be one AaC writes"), + ({"period": {"granularity": "WEEK", "from": -2, "to": -1}}, "must be one AaC writes"), + ({"period": {"granularity": ["MONTH"], "from": -6, "to": -1}}, "must be one AaC writes"), + ({"period": {"granularity": "MONTH", "from": "-6", "to": -1}}, "a relative period needs whole numbers"), + ({"period": {"granularity": "MONTH", "from": True, "to": -1}}, "a relative period needs whole numbers"), ], ) def test_an_unusable_fixture_is_rejected_before_any_request(expected: dict, message: str) -> None: @@ -1617,16 +2062,153 @@ def test_a_bare_visualization_id_is_matched_as_in_as_code() -> None: def test_a_relative_period_never_matches_the_expected_dates() -> None: - relative = {"type": "relative", "granularity": "GDC.time.month", "from": -6, "to": -1} + relative = {"granularity": "MONTH", "from": -6, "to": -1} part = _v2_document_part(period=relative) evaluation = _evaluate(part, expected={"period": _PERIOD}) assert evaluation.strict_checks["document_period_correct"] is False assert evaluation.failures == [ - "the document covers the relative range GDC.time.month -6 to -1, expected 2026-01-01 to 2026-06-30" + "the document covers the relative range MONTH -6 to -1, expected 2026-01-01 to 2026-06-30" ] - assert render_document_text(part["document"]).splitlines()[1] == ( - "Period: the relative range GDC.time.month -6 to -1" - ) + assert render_document_text(part["document"]).splitlines()[1] == ("Period: the relative range MONTH -6 to -1") + + +# As gen-ai drafts "the last 6 full months": AaC writes a relative period with its granularity and no type. +_RELATIVE = {"granularity": "MONTH", "from": -6, "to": -1} + + +def test_a_relative_period_kept_as_stated_passes() -> None: + evaluation = _evaluate(_v2_document_part(period=dict(_RELATIVE)), expected={"period": _RELATIVE}) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_period_correct"] is True + + +@pytest.mark.parametrize( + ("period", "covers"), + [ + ({**_RELATIVE, "from": -12}, "the relative range MONTH -12 to -1"), + ({**_RELATIVE, "granularity": "QUARTER"}, "the relative range QUARTER -6 to -1"), + ({"from": "2026-05-01", "to": "2026-10-31"}, "2026-05-01 to 2026-10-31"), + ], + ids=["shifted", "other-granularity", "made-absolute"], +) +def test_a_relative_period_that_changed_fails(period: dict, covers: str) -> None: + evaluation = _evaluate(_v2_document_part(period=period), expected={"period": _RELATIVE}) + assert evaluation.strict_checks["document_period_correct"] is False + assert evaluation.failures == [f"the document covers {covers}, expected the relative range MONTH -6 to -1"] + + +def test_a_relative_period_never_matches_a_v1_document() -> None: + evaluation = _evaluate(_document_part(), expected={"period": _RELATIVE}) + assert evaluation.strict_checks["document_period_correct"] is False + + +def test_the_simulated_reply_states_a_relative_period() -> None: + reply = build_simulated_reply({"period": _RELATIVE}) + assert "Period: the relative range MONTH -6 to -1." in reply + + +# As gen-ai drafts a document from a dashboard, in AaC: a chart the dashboard's date filter reaches applies +# the period on its own date dataset, and one it does not reach keeps a `date` but ignores the period. +_GOAL, _SEASONALITY = "sales_goal", "seasonality" +_CHART_PERIODS = {_REVENUE_TREND: "date", _GOAL: "customer_created_date", _SEASONALITY: None} + + +def _with_chart_dates(page: dict, dates: dict[str, Any]) -> dict: + """``page`` with each chart's widget given a ``date``, or ``ignore_document_period`` where the value is None.""" + for node in page["layout"]["children"][1]["children"][0]["children"]: + chart = node["visualization"] + node["date"] = dates[chart] if dates[chart] is not None else "date" + if dates[chart] is None: + node["ignore_document_period"] = True + return page + + +def _dashboard_document_part(dates: dict[str, Any] | None = None) -> dict: + charts = dates if dates is not None else _CHART_PERIODS + page = _with_chart_dates(_v2_content_page("page2", *charts, bare_ids=True), charts) + return _v2_document_part([_v2_cover_page(), page]) + + +def test_charts_that_apply_the_period_as_the_dashboard_does_pass() -> None: + evaluation = _evaluate(_dashboard_document_part(), expected={"chart_periods": _CHART_PERIODS}) + assert evaluation.failures == [] + assert evaluation.strict_checks["document_chart_periods"] is True + + +@pytest.mark.parametrize( + ("dates", "failure"), + [ + ( + {**_CHART_PERIODS, _SEASONALITY: "date"}, + "chart 'seasonality' applies the period on 'date', expected it to ignore the period", + ), + ( + {**_CHART_PERIODS, _GOAL: "date"}, + "chart 'sales_goal' applies the period on 'date', expected it on 'customer_created_date'", + ), + ( + {**_CHART_PERIODS, _GOAL: None}, + "chart 'sales_goal' ignores the period, expected it on 'customer_created_date'", + ), + ({**_CHART_PERIODS, _REVENUE_TREND: None}, "chart 'revenue_trend' ignores the period, expected it on 'date'"), + ], + ids=["unreached-chart-dated", "other-date", "reached-chart-ignores", "ignores-on-its-own-date"], +) +def test_a_chart_that_applies_the_period_elsewhere_fails(dates: dict, failure: str) -> None: + evaluation = _evaluate(_dashboard_document_part(dates), expected={"chart_periods": _CHART_PERIODS}) + assert evaluation.strict_checks["document_chart_periods"] is False + assert evaluation.failures == [failure] + + +def test_a_chart_left_out_of_the_document_fails_its_period_check() -> None: + dates = {_REVENUE_TREND: "date", _GOAL: "customer_created_date"} + evaluation = _evaluate(_dashboard_document_part(dates), expected={"chart_periods": _CHART_PERIODS}) + assert evaluation.strict_checks["document_chart_periods"] is False + assert evaluation.failures == [ + "chart 'seasonality' is not in the document, so where it applies the period is unknown" + ] + + +def test_every_placement_of_a_chart_must_apply_the_period_as_stated() -> None: + reached = _with_chart_dates(_v2_content_page("page2", _SEASONALITY, bare_ids=True), {_SEASONALITY: None}) + again = _with_chart_dates(_v2_content_page("page3", _SEASONALITY, bare_ids=True), {_SEASONALITY: "date"}) + part = _v2_document_part([_v2_cover_page(), reached, again]) + evaluation = _evaluate(part, expected={"chart_periods": {_SEASONALITY: None}}) + assert evaluation.strict_checks["document_chart_periods"] is False + + +def test_a_chart_only_on_a_hidden_page_is_not_in_the_document() -> None: + hidden_page = _v2_content_page("page3", _SEASONALITY, bare_ids=True) + hidden = {**_with_chart_dates(hidden_page, {_SEASONALITY: None}), "visible": False} + part = _v2_document_part([_v2_cover_page(), _v2_content_page("page2", _REVENUE_TREND), hidden]) + evaluation = _evaluate(part, expected={"chart_periods": {_SEASONALITY: None}}) + assert evaluation.strict_checks["document_chart_periods"] is False + + +def test_chart_periods_are_not_scored_on_a_v1_document() -> None: + evaluation = _evaluate(_document_part(), expected={"chart_periods": {_REVENUE_TREND: "date"}}) + assert evaluation.strict_checks["document_chart_periods"] is False + assert evaluation.failures == ["where a chart applies the period is scored on aac-v2 documents only"] + + +def test_chart_periods_are_published_only_when_stated() -> None: + assert "document_chart_periods" not in _evaluate(_dashboard_document_part()).strict_checks + + +@pytest.mark.parametrize( + ("chart_periods", "message"), + [ + ({}, "chart_periods must map chart ids"), + ([_REVENUE_TREND], "chart_periods must map chart ids"), + ({" ": "date"}, "every chart_periods key must be a chart id"), + ({_REVENUE_TREND: ""}, r"chart_periods\['revenue_trend'\] must be a date dataset id or null"), + ({_REVENUE_TREND: 1}, r"chart_periods\['revenue_trend'\] must be a date dataset id or null"), + ], + ids=["empty", "list", "blank-chart", "blank-dataset", "not-a-dataset"], +) +def test_unusable_chart_periods_are_rejected(chart_periods: Any, message: str) -> None: + with pytest.raises(ValueError, match=message): + _validate_expectation({"chart_periods": chart_periods}) def test_a_period_that_is_not_an_object_fails_instead_of_crashing() -> None: