From 2d66d0980562596bc231aec1b50c1f725c76b2ac Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 9 Sep 2026 18:15:40 +0200 Subject: [PATCH 1/5] fix(gooddata-eval): stop the adhoc-visualization fallback from failing validation When `create_adhoc_visualization` fails (e.g. the data source is unreachable), `_build_chat_result` falls back to the agent's raw tool-call arguments so the turn can still be scored on the visualization the agent *intended*. But those arguments describe a definition, not a persisted object, so they carry no `id` -- and `CreatedVisualization.id` is required with no default. The result is that the fallback path could never validate. Every turn that reached it raised ValidationError: 1 validation error for ChatResult createdVisualizations.objects.0.id Field required and was recorded as a hard error, which is precisely the outcome the fallback was added to prevent: a stalled data source scored as an agent content failure. Observed on 10 of 56 runs in one recent visualization eval batch. Synthesize a sentinel id on the fallback path only. An `id` present in the arguments still wins. Both existing fallback tests hand-wrote an `id` into the tool arguments, which real agent calls do not have -- so CI passed while production failed 100% of the time this path was taken. The added test uses realistic arguments and fails against the unfixed parser with the exact error above. Co-Authored-By: Claude Opus 5 --- .../src/gooddata_eval/core/chat/sse_client.py | 11 +++++++++- .../gooddata-eval/tests/test_sse_client.py | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py index 27ca91d26..6600cd86d 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py @@ -34,6 +34,9 @@ _RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 502, 503, 504}) _METADATA_SYNC_MARKER = "METADATA_SYNC_IN_PROGRESS" +# Stands in for the `id` a persisted visualization would carry, on the fallback path +# where the agent's create_adhoc_visualization call failed and only its arguments survive. +_ADHOC_VIZ_ID = "adhoc-visualization-not-persisted" _KNOWN_PART_TYPES: frozenset[str] = frozenset( { @@ -247,8 +250,14 @@ def _build_chat_result(acc: _SseAccumulator) -> ChatResult: # Fallback: the agent produced a correct visualization definition via # create_adhoc_visualization but the call failed (e.g. data source not # accessible). The last attempt is the agent's best answer. + # + # These are raw tool-call arguments, so they carry no `id` -- nothing was + # ever persisted. CreatedVisualization requires one, so synthesize a + # sentinel rather than letting the whole ChatResult fail to validate: + # dropping the turn entirely would score a stalled data source as a + # content failure, which is exactly what this fallback exists to prevent. payload["createdVisualizations"] = { - "objects": [acc.adhoc_viz_args[-1]], + "objects": [{"id": _ADHOC_VIZ_ID, **acc.adhoc_viz_args[-1]}], "reasoning": "\n".join(acc.viz_reasoning_parts), } result = ChatResult.model_validate(payload) diff --git a/packages/gooddata-eval/tests/test_sse_client.py b/packages/gooddata-eval/tests/test_sse_client.py index cdb348e9f..16e34cb33 100644 --- a/packages/gooddata-eval/tests/test_sse_client.py +++ b/packages/gooddata-eval/tests/test_sse_client.py @@ -362,6 +362,28 @@ def test_parse_sse_lines_falls_back_to_adhoc_viz_when_multipart_viz_is_null(): assert result.created_visualizations.objects[0].type == "line_chart" +def test_parse_sse_lines_adhoc_fallback_synthesizes_id_when_args_have_none(): + """A create_adhoc_visualization definition carries no `id` -- nothing was persisted. + + CreatedVisualization requires one, so without a synthesized stand-in the whole + ChatResult fails to validate and the turn is lost. Regression test: real agent + tool arguments have no `id`, unlike the hand-written fixtures above. + """ + viz_def = { + "type": "line_chart", + "query": {"fields": {"m": {"using": "metric/total_sales"}}, "filter_by": {}}, + "metrics": ["m"], + } + lines = [ + f'data: {{"item": {{"role": "assistant", "content": {{"type": "toolCall", "callId": "c1", "name": "create_adhoc_visualization", "arguments": {{"visualization": {json.dumps(viz_def)}}}}}}}}}', + 'data: {"item": {"role": "assistant", "content": {"type": "multipart", "parts": [{"type": "visualization", "visualization": null}]}}}', + ] + result = parse_sse_lines(lines) + assert result.created_visualizations is not None + assert result.created_visualizations.objects[0].id == "adhoc-visualization-not-persisted" + assert result.created_visualizations.objects[0].type == "line_chart" + + def test_parse_sse_lines_counts_reasoning_steps(): lines = [ 'data: {"item": {"role": "assistant", "content": {"type": "reasoning", "summary": "step one"}}}', From 0256f158662b021c57f540cf7089d9b66bb1f066 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 9 Sep 2026 18:19:17 +0200 Subject: [PATCH 2/5] fix(gooddata-eval): retry transient gen-ai 500s instead of hard-failing the run `_RETRYABLE_STATUS_CODES` omitted 500, so a mid-stream `statusCode: 500` from gen-ai raised a terminal `ChatError` with zero retry attempts -- the same shape as the `RemoteProtocolError` gap already documented just below it. Evidence that these are transient rather than deterministic server bugs: in one visualization eval batch, 10 of 56 runs died this way, and *every* affected question scored normally when the same question/model ran again the next day. Not one reproduced. A genuinely deterministic 500 still terminates the run, just after the bounded backoff (`GOODDATA_EVAL_CHAT_MAX_RETRIES`, default 5) rather than immediately. Two existing tests used 500 as a generic "hard error" code. Since 500 now raises `TransientChatError` -- a `ChatError` subclass -- they would still have passed while no longer testing the terminal path they were written for, so they move to 400. Co-Authored-By: Claude Opus 5 --- .../src/gooddata_eval/core/chat/sse_client.py | 6 +++++- packages/gooddata-eval/tests/test_sse_client.py | 9 +++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py index 6600cd86d..084467891 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py @@ -32,7 +32,11 @@ # gen-ai's last event, only if at least one item was already emitted (conversations_controller.py). _RESPONSE_ENDED_EVENT = "response_ended" -_RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 502, 503, 504}) +# 500 is here on evidence, not on principle: in one visualization eval batch it hard-failed +# 10 of 56 runs with zero retry attempts, and every affected question scored normally when the +# same question/model ran again the next day -- i.e. transient gen-ai faults, not deterministic +# server bugs. A genuinely deterministic 500 still terminates, just after the bounded backoff. +_RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 500, 502, 503, 504}) _METADATA_SYNC_MARKER = "METADATA_SYNC_IN_PROGRESS" # Stands in for the `id` a persisted visualization would carry, on the fallback path # where the agent's create_adhoc_visualization call failed and only its arguments survive. diff --git a/packages/gooddata-eval/tests/test_sse_client.py b/packages/gooddata-eval/tests/test_sse_client.py index 16e34cb33..a9e7e8eaa 100644 --- a/packages/gooddata-eval/tests/test_sse_client.py +++ b/packages/gooddata-eval/tests/test_sse_client.py @@ -18,8 +18,9 @@ def test_parse_sse_lines_collects_text_and_visualization(fixtures_dir): def test_parse_sse_lines_raises_on_error_event(): - lines = ['data: {"statusCode": 500, "detail": "boom"}'] - with pytest.raises(RuntimeError, match="SSE error 500"): + # 400: a code outside _RETRYABLE_STATUS_CODES, so this exercises the terminal path. + lines = ['data: {"statusCode": 400, "detail": "boom"}'] + with pytest.raises(RuntimeError, match="SSE error 400"): parse_sse_lines(lines) @@ -51,7 +52,7 @@ def test_parse_sse_lines_error_carries_partial_result_with_tool_calls_already_se } ), "", - json.dumps({"statusCode": 500, "detail": "boom"}), + json.dumps({"statusCode": 400, "detail": "boom"}), ] lines = [f"data: {line}" if line else line for line in lines] with pytest.raises(ChatError) as ei: @@ -472,7 +473,7 @@ def test_parse_sse_lines_has_no_alert_proposals_by_default(): assert parse_sse_lines(lines).alert_proposals == [] -@pytest.mark.parametrize("code", [429, 502, 503, 504]) +@pytest.mark.parametrize("code", [429, 500, 502, 503, 504]) def test_parse_sse_lines_transient_status_codes(code): with pytest.raises(TransientChatError) as ei: parse_sse_lines([f'data: {{"statusCode": {code}, "detail": null}}']) From 12d522046590de8122d2c72e8a65b3ee79f62daa Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 9 Sep 2026 21:21:09 +0200 Subject: [PATCH 3/5] fix(gooddata-eval): score relative and absolute date filters as equivalent The agent may express "last month" either relatively (`granularity: MONTH, from: -1, to: -1`) or absolutely (`from: 2026-08-01, to: 2026-08-31`). `_normalize_date_filter` passed `from`/`to`/`granularity` straight through, so the two encodings could never compare equal -- a correct answer in whichever encoding the fixture did not happen to use was scored as a wrong date period. Both forms now resolve to the inclusive absolute span they denote, for DAY, MONTH, QUARTER and YEAR. The WEEK family deliberately falls back to literal comparison: its start-of-week convention varies (WEEK vs WEEK_US), and guessing would trade a false negative for a false positive. Resolution uses a single `today` anchor threaded through `check_filters`, so both sides resolve against the same date even if a run straddles midnight. Note this changes the rendering of `detail.expected_filters` / `actual_filters` for resolvable granularities: an entry now carries an absolute `from`/`to` and no `granularity` key. That is also more legible when diagnosing a date-score failure, which is what those fields exist for. Verified against a real eval batch: two runs scored as date-period failures are correctly matches. Genuinely different windows still fail -- an absolute range spanning two months does not match a one-month relative filter, and an off-by-one day count still differs. Co-Authored-By: Claude Opus 5 --- .../src/gooddata_eval/core/scoring.py | 99 ++++++++++++++++--- packages/gooddata-eval/tests/test_scoring.py | 57 +++++++++++ .../tests/test_visualization_evaluator.py | 9 +- 3 files changed, 152 insertions(+), 13 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py index 0d554cd85..05791508a 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py @@ -1,8 +1,10 @@ # (C) 2026 GoodData Corporation """Visualization scoring — ported from gdc-nas tavern-e2e app/vis_assertions/metrics.py.""" +import calendar import json from dataclasses import dataclass +from datetime import date, timedelta from gooddata_eval.core.models import AacBucketRef, AacQueryField, CreatedVisualization @@ -106,13 +108,81 @@ def validate_cross_references(viz: CreatedVisualization) -> tuple[bool, list[str return len(errors) == 0, errors -def _normalize_date_filter(filter_dict: dict, _fields: dict) -> dict: +def _shift_month(anchor: date, offset: int) -> tuple[date, date]: + """First and last day of the calendar month ``offset`` months from ``anchor``.""" + total = anchor.year * 12 + (anchor.month - 1) + offset + year, month = divmod(total, 12) + return date(year, month + 1, 1), date(year, month + 1, calendar.monthrange(year, month + 1)[1]) + + +def _absolute_span(granularity: str, start_offset: int, end_offset: int, today: date) -> tuple[date, date] | None: + """Resolve a relative date filter to the inclusive absolute span it denotes. + + Returns None for granularities this cannot resolve unambiguously -- notably the + WEEK family, whose start-of-week convention varies (WEEK vs WEEK_US vs ...). + Guessing there would trade a false negative for a false positive. + """ + gran = granularity.upper() + if gran == "DAY": + return today + timedelta(days=start_offset), today + timedelta(days=end_offset) + if gran == "MONTH": + return _shift_month(today, start_offset)[0], _shift_month(today, end_offset)[1] + if gran == "QUARTER": + q_start_month = (today.month - 1) // 3 * 3 + 1 + anchor = date(today.year, q_start_month, 1) + return _shift_month(anchor, start_offset * 3)[0], _shift_month(anchor, end_offset * 3 + 2)[1] + if gran == "YEAR": + return date(today.year + start_offset, 1, 1), date(today.year + end_offset, 12, 31) + return None + + +def _as_date(value: object) -> date | None: + if isinstance(value, str): + try: + return date.fromisoformat(value[:10]) + except ValueError: + return None + return None + + +def _normalize_date_filter(filter_dict: dict, _fields: dict, today: date | None = None) -> dict: + """Canonicalize a date filter, resolving relative offsets to an absolute span. + + The agent may answer "last month" either relatively (``granularity: MONTH, + from: -1, to: -1``) or absolutely (``from: 2026-08-01, to: 2026-08-31``). + Compared literally these never match, so a correct answer in the encoding the + fixture did not happen to use was scored as a wrong date period. Both forms + collapse to the same absolute span here. + + Resolution is relative to today, which is the same "today" the agent resolved + against -- scoring runs in the same process as the turn. Re-scoring an archived + result at a later date would therefore drift; nothing currently does that. + """ + raw_from, raw_to = filter_dict.get("from"), filter_dict.get("to") + granularity = filter_dict.get("granularity") + span: tuple[date, date] | None = None + + if isinstance(raw_from, int) and isinstance(raw_to, int) and isinstance(granularity, str): + span = _absolute_span(granularity, raw_from, raw_to, today or date.today()) + else: + start, end = _as_date(raw_from), _as_date(raw_to) + if start and end: + span = (start, end) + + if span is not None: + return { + "type": "date_filter", + "dataset_uri": filter_dict.get("using", ""), + "from": span[0].isoformat(), + "to": span[1].isoformat(), + } + # Unresolvable (e.g. the WEEK family): fall back to literal comparison. return { "type": "date_filter", "dataset_uri": filter_dict.get("using", ""), - "from": filter_dict.get("from"), - "to": filter_dict.get("to"), - "granularity": filter_dict.get("granularity"), + "from": raw_from, + "to": raw_to, + "granularity": granularity, } @@ -171,7 +241,9 @@ def _normalize_attribute_filter(filter_dict: dict, _fields: dict) -> dict: } -def _split_and_normalize_filters(viz: CreatedVisualization) -> tuple[set[str], set[str], set[str]]: +def _split_and_normalize_filters( + viz: CreatedVisualization, today: date | None = None +) -> tuple[set[str], set[str], set[str]]: date_set: set[str] = set() ranking_set: set[str] = set() attr_set: set[str] = set() @@ -180,7 +252,7 @@ def _split_and_normalize_filters(viz: CreatedVisualization) -> tuple[set[str], s for filter_dict in viz.query.filter_by.values(): ft = filter_dict.get("type") if ft == "date_filter": - date_set.add(json.dumps(_normalize_date_filter(filter_dict, fields), sort_keys=True)) + date_set.add(json.dumps(_normalize_date_filter(filter_dict, fields, today), sort_keys=True)) elif ft == "ranking_filter": ranking_set.add(json.dumps(_normalize_ranking_filter(filter_dict, fields, sole_dim_uri), sort_keys=True)) elif ft == "attribute_filter": @@ -188,7 +260,7 @@ def _split_and_normalize_filters(viz: CreatedVisualization) -> tuple[set[str], s return date_set, ranking_set, attr_set -def normalized_filters(viz: CreatedVisualization) -> dict[str, list[str]]: +def normalized_filters(viz: CreatedVisualization, today: date | None = None) -> dict[str, list[str]]: """A visualization's filters exactly as `check_filters` compares them. Grouped by the three categories it scores separately and sorted for stable output. @@ -197,13 +269,18 @@ def normalized_filters(viz: CreatedVisualization) -> dict[str, list[str]]: a `filter_date_score` of False otherwise gives no clue whether the period differed, the granularity did, or the dataset the filter hangs off did. """ - date_set, ranking_set, attr_set = _split_and_normalize_filters(viz) + date_set, ranking_set, attr_set = _split_and_normalize_filters(viz, today) return {"date": sorted(date_set), "ranking": sorted(ranking_set), "attribute": sorted(attr_set)} -def check_filters(expected: CreatedVisualization, actual: CreatedVisualization) -> FilterScores: - exp_date, exp_rank, exp_attr = _split_and_normalize_filters(expected) - act_date, act_rank, act_attr = _split_and_normalize_filters(actual) +def check_filters( + expected: CreatedVisualization, actual: CreatedVisualization, today: date | None = None +) -> FilterScores: + # One anchor for both sides: resolving each against its own date.today() would + # score inconsistently for a run that straddles midnight. + today = today or date.today() + exp_date, exp_rank, exp_attr = _split_and_normalize_filters(expected, today) + act_date, act_rank, act_attr = _split_and_normalize_filters(actual, today) return FilterScores( date_ok=act_date == exp_date, ranking_ok=act_rank == exp_rank, diff --git a/packages/gooddata-eval/tests/test_scoring.py b/packages/gooddata-eval/tests/test_scoring.py index 873e30628..dea702761 100644 --- a/packages/gooddata-eval/tests/test_scoring.py +++ b/packages/gooddata-eval/tests/test_scoring.py @@ -1,4 +1,6 @@ # (C) 2026 GoodData Corporation +from datetime import date + from gooddata_eval.core.models import CreatedVisualization from gooddata_eval.core.scoring import ( check_filters, @@ -205,3 +207,58 @@ def test_normalized_filters_is_empty_per_category_when_unfiltered(): } ) assert normalized_filters(viz) == {"date": [], "ranking": [], "attribute": []} + + +# --- relative vs absolute date filters denote the same period --- +# +# The agent answers "last month" either relatively (granularity MONTH, from -1, to -1) +# or absolutely (from 2026-08-01, to 2026-08-31). Compared literally these never match, +# so a correct answer in whichever encoding the fixture did not happen to use was scored +# as a wrong date period. Both forms now collapse to the same absolute span. + +_TODAY = date(2026, 9, 9) + + +def _date_viz(**overrides): + f = {"using": "dataset/dt_transactions", "type": "date_filter"} + f.update(overrides) + return _viz(query={"fields": {}, "filter_by": {"f_d": f}}) + + +def test_check_filters_relative_and_absolute_last_month_agree(): + expected = _date_viz(**{"from": -1, "to": -1, "granularity": "MONTH"}) + actual = _date_viz(**{"from": "2026-08-01", "to": "2026-08-31", "granularity": None}) + assert check_filters(expected, actual, _TODAY).date_ok is True + + +def test_check_filters_absolute_spanning_two_months_still_differs_from_one(): + """Normalization must not flatten a genuinely wrong window into a match.""" + expected = _date_viz(**{"from": -1, "to": -1, "granularity": "MONTH"}) + actual = _date_viz(**{"from": "2026-07-01", "to": "2026-08-31", "granularity": None}) + assert check_filters(expected, actual, _TODAY).date_ok is False + + +def test_check_filters_day_offsets_resolve_and_off_by_one_still_fails(): + expected = _date_viz(**{"from": -89, "to": 0, "granularity": "DAY"}) + assert check_filters(expected, _date_viz(**{"from": "2026-06-12", "to": "2026-09-09"}), _TODAY).date_ok is True + assert check_filters(expected, _date_viz(**{"from": -90, "to": 0, "granularity": "DAY"}), _TODAY).date_ok is False + + +def test_check_filters_quarter_and_year_offsets_resolve(): + q = _date_viz(**{"from": -1, "to": -1, "granularity": "QUARTER"}) + assert check_filters(q, _date_viz(**{"from": "2026-04-01", "to": "2026-06-30"}), _TODAY).date_ok is True + y = _date_viz(**{"from": 0, "to": 0, "granularity": "YEAR"}) + assert check_filters(y, _date_viz(**{"from": "2026-01-01", "to": "2026-12-31"}), _TODAY).date_ok is True + + +def test_check_filters_week_granularity_falls_back_to_literal_comparison(): + """WEEK start-of-week convention varies, so it is compared literally rather than guessed.""" + expected = _date_viz(**{"from": -2, "to": -1, "granularity": "WEEK"}) + assert check_filters(expected, _date_viz(**{"from": -2, "to": -1, "granularity": "WEEK"}), _TODAY).date_ok is True + assert check_filters(expected, _date_viz(**{"from": -13, "to": 0, "granularity": "DAY"}), _TODAY).date_ok is False + + +def test_check_filters_date_still_distinguishes_the_dataset_it_hangs_off(): + expected = _date_viz(**{"from": -1, "to": -1, "granularity": "MONTH"}) + actual = _date_viz(using="dataset/dt_date", **{"from": -1, "to": -1, "granularity": "MONTH"}) + assert check_filters(expected, actual, _TODAY).date_ok is False diff --git a/packages/gooddata-eval/tests/test_visualization_evaluator.py b/packages/gooddata-eval/tests/test_visualization_evaluator.py index d99d364d6..b16b00626 100644 --- a/packages/gooddata-eval/tests/test_visualization_evaluator.py +++ b/packages/gooddata-eval/tests/test_visualization_evaluator.py @@ -1,4 +1,6 @@ # (C) 2026 GoodData Corporation +import re + from gooddata_eval.core.evaluators import get_evaluator from gooddata_eval.core.models import ChatResult, DatasetItem @@ -158,8 +160,11 @@ def test_detail_reports_the_filters_that_were_compared(): assert result.detail["filter_date_score"] is False expected, actual = result.detail["expected_filters"], result.detail["actual_filters"] - assert '"from": -11' in expected["date"][0] - assert '"from": -12' in actual["date"][0] + # Relative offsets are reported as the absolute span they resolve to, so the two + # periods are legible side by side and visibly different -- which is the point. + assert re.search(r'"from": "\d{4}-\d{2}-\d{2}"', expected["date"][0]) + assert re.search(r'"from": "\d{4}-\d{2}-\d{2}"', actual["date"][0]) + assert expected["date"] != actual["date"] assert expected["ranking"] == actual["ranking"] == [] assert expected["attribute"] == actual["attribute"] == [] From 118c27bada8e98001f2a946c80b8977dda53add4 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 9 Sep 2026 21:31:22 +0200 Subject: [PATCH 4/5] fix(gooddata-eval): resolve WEEK_US date filters too WEEK_US names a Sunday-start week, so unlike a bare WEEK it can be resolved to an absolute span without guessing a convention -- and a bare WEEK is not in the AAC granularity enum at all (the convertor rewrites WEEK_US to AFM's WEEK). Without this, a fixture pinned to WEEK_US could never match an agent that answered the same period absolutely. Co-Authored-By: Claude Opus 5 --- .../gooddata-eval/src/gooddata_eval/core/scoring.py | 10 +++++++--- packages/gooddata-eval/tests/test_scoring.py | 12 ++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py index 05791508a..1e59e0947 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py @@ -118,13 +118,17 @@ def _shift_month(anchor: date, offset: int) -> tuple[date, date]: def _absolute_span(granularity: str, start_offset: int, end_offset: int, today: date) -> tuple[date, date] | None: """Resolve a relative date filter to the inclusive absolute span it denotes. - Returns None for granularities this cannot resolve unambiguously -- notably the - WEEK family, whose start-of-week convention varies (WEEK vs WEEK_US vs ...). - Guessing there would trade a false negative for a false positive. + Returns None for granularities this cannot resolve unambiguously -- notably a bare + ``WEEK``, which is not in the AAC granularity enum and states no start-of-week + convention. ``WEEK_US`` *is* well defined (Sunday-start), so it resolves. + Guessing the bare case would trade a false negative for a false positive. """ gran = granularity.upper() if gran == "DAY": return today + timedelta(days=start_offset), today + timedelta(days=end_offset) + if gran == "WEEK_US": + sunday = today - timedelta(days=(today.weekday() + 1) % 7) + return sunday + timedelta(weeks=start_offset), sunday + timedelta(weeks=end_offset, days=6) if gran == "MONTH": return _shift_month(today, start_offset)[0], _shift_month(today, end_offset)[1] if gran == "QUARTER": diff --git a/packages/gooddata-eval/tests/test_scoring.py b/packages/gooddata-eval/tests/test_scoring.py index dea702761..ba7ccc80e 100644 --- a/packages/gooddata-eval/tests/test_scoring.py +++ b/packages/gooddata-eval/tests/test_scoring.py @@ -251,8 +251,16 @@ def test_check_filters_quarter_and_year_offsets_resolve(): assert check_filters(y, _date_viz(**{"from": "2026-01-01", "to": "2026-12-31"}), _TODAY).date_ok is True -def test_check_filters_week_granularity_falls_back_to_literal_comparison(): - """WEEK start-of-week convention varies, so it is compared literally rather than guessed.""" +def test_check_filters_week_us_resolves_to_a_sunday_start_week(): + """WEEK_US is well defined (Sunday-start), unlike a bare WEEK.""" + expected = _date_viz(**{"from": 0, "to": 0, "granularity": "WEEK_US"}) + # 2026-09-09 is a Wednesday; its WEEK_US bucket runs Sun 09-06 .. Sat 09-12. + assert check_filters(expected, _date_viz(**{"from": "2026-09-06", "to": "2026-09-12"}), _TODAY).date_ok is True + assert check_filters(expected, _date_viz(**{"from": "2026-09-06", "to": "2026-09-09"}), _TODAY).date_ok is False + + +def test_check_filters_bare_week_granularity_falls_back_to_literal_comparison(): + """A bare WEEK is not in the AAC enum and names no convention, so it is not guessed.""" expected = _date_viz(**{"from": -2, "to": -1, "granularity": "WEEK"}) assert check_filters(expected, _date_viz(**{"from": -2, "to": -1, "granularity": "WEEK"}), _TODAY).date_ok is True assert check_filters(expected, _date_viz(**{"from": -13, "to": 0, "granularity": "DAY"}), _TODAY).date_ok is False From d7e9a7d14b55e69dab138a42225048bf54638862 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 9 Sep 2026 22:44:05 +0200 Subject: [PATCH 5/5] fix(gooddata-eval): guard out-of-range date offsets and share one scoring anchor Two defects in relative-date scoring, both found in review of this PR. An offset far enough out to leave the representable date range raised out of _absolute_span instead of resolving. Every granularity can be pushed past it: a YEAR offset of -today.year lands on year 0, large DAY/WEEK_US offsets exceed timedelta's magnitude limit, and large MONTH/QUARTER offsets leave 1..9999. The exception escaped through _normalize_date_filter and check_filters, failing the whole item over one malformed filter. It now resolves to None, putting the filter back on the literal-comparison path an unresolvable span already takes. Separately, check_filters shared one anchor across its two sides, but the evaluator resolved the filters it reports through separate normalized_filters calls, each taking its own date.today(). A run straddling midnight could report periods the verdict was never computed from -- a detail contradicting its own score. The anchor is now captured once in _evaluate_visualization and passed to check_filters and both normalized_filters calls; _evaluate_against_candidates captures one too, so candidates are never ranked on spans from different days. Co-Authored-By: Claude Opus 5 --- .../core/evaluators/visualization.py | 21 ++++++++--- .../src/gooddata_eval/core/scoring.py | 35 +++++++++++------- packages/gooddata-eval/tests/test_scoring.py | 24 +++++++++++++ .../tests/test_visualization_evaluator.py | 36 ++++++++++++++++++- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py index a6e197d34..32a807972 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py @@ -2,6 +2,7 @@ """Agentic visualization evaluator — ported from gdc-nas tavern-e2e app/vis_agentic.py.""" from dataclasses import dataclass +from datetime import date from gooddata_eval.core.evaluators.base import ItemEvaluation from gooddata_eval.core.models import ( @@ -85,7 +86,13 @@ def _evaluate_visualization( expected: CreatedVisualization, actual: CreatedVisualization | None, skill_activated: bool = False, + today: date | None = None, ) -> EvaluationResult: + # One anchor for the scoring and for the filters reported beside it. check_filters + # already shares an anchor across its two sides, but resolving the reported filters + # separately would let a run that straddles midnight report periods the score was + # never computed from -- a detail that contradicts its own verdict. + today = today or date.today() exp_metric_uris = get_metric_uri_set(expected) exp_dim_uris = get_dimension_uri_set(expected) if actual is None: @@ -105,13 +112,13 @@ def _evaluate_visualization( actual_metric_uris=set(), expected_dim_uris=exp_dim_uris, actual_dim_uris=set(), - expected_filters=normalized_filters(expected), + expected_filters=normalized_filters(expected, today), actual_filters={category: values.copy() for category, values in _NO_FILTERS.items()}, ) cross_ref_valid, cross_ref_errors = validate_cross_references(actual) act_metric_uris = get_metric_uri_set(actual) act_dim_uris = get_dimension_uri_set(actual) - filter_scores = check_filters(expected, actual) + filter_scores = check_filters(expected, actual, today) return EvaluationResult( visualization_created=True, cross_ref_valid=cross_ref_valid, @@ -128,8 +135,8 @@ def _evaluate_visualization( actual_metric_uris=act_metric_uris, expected_dim_uris=exp_dim_uris, actual_dim_uris=act_dim_uris, - expected_filters=normalized_filters(expected), - actual_filters=normalized_filters(actual), + expected_filters=normalized_filters(expected, today), + actual_filters=normalized_filters(actual, today), ) @@ -137,8 +144,12 @@ def _evaluate_against_candidates( expected_outputs: list[CreatedVisualization], actual: CreatedVisualization | None, skill_activated: bool = False, + today: date | None = None, ) -> tuple[EvaluationResult, CreatedVisualization]: - pairs = [(_evaluate_visualization(exp, actual, skill_activated), exp) for exp in expected_outputs] + # Anchored once here too: the candidates are ranked against each other, so letting + # each resolve its own today could rank them on spans from different days. + today = today or date.today() + pairs = [(_evaluate_visualization(exp, actual, skill_activated, today), exp) for exp in expected_outputs] best_result, best_expected = max(pairs, key=lambda p: (p[0].strict_pass, p[0].strict_checks_passed_count)) return best_result, best_expected diff --git a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py index 1e59e0947..d7b88674a 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py @@ -122,21 +122,30 @@ def _absolute_span(granularity: str, start_offset: int, end_offset: int, today: ``WEEK``, which is not in the AAC granularity enum and states no start-of-week convention. ``WEEK_US`` *is* well defined (Sunday-start), so it resolves. Guessing the bare case would trade a false negative for a false positive. + + An offset far enough out to leave the representable date range resolves to None + rather than raising: every granularity here can be pushed past it (a YEAR offset of + ``-today.year`` alone lands on year 0), and letting that escape would abort scoring + for the whole item over one malformed filter. None puts the filter back on the + literal-comparison path, which is what an unresolvable span already does. """ gran = granularity.upper() - if gran == "DAY": - return today + timedelta(days=start_offset), today + timedelta(days=end_offset) - if gran == "WEEK_US": - sunday = today - timedelta(days=(today.weekday() + 1) % 7) - return sunday + timedelta(weeks=start_offset), sunday + timedelta(weeks=end_offset, days=6) - if gran == "MONTH": - return _shift_month(today, start_offset)[0], _shift_month(today, end_offset)[1] - if gran == "QUARTER": - q_start_month = (today.month - 1) // 3 * 3 + 1 - anchor = date(today.year, q_start_month, 1) - return _shift_month(anchor, start_offset * 3)[0], _shift_month(anchor, end_offset * 3 + 2)[1] - if gran == "YEAR": - return date(today.year + start_offset, 1, 1), date(today.year + end_offset, 12, 31) + try: + if gran == "DAY": + return today + timedelta(days=start_offset), today + timedelta(days=end_offset) + if gran == "WEEK_US": + sunday = today - timedelta(days=(today.weekday() + 1) % 7) + return sunday + timedelta(weeks=start_offset), sunday + timedelta(weeks=end_offset, days=6) + if gran == "MONTH": + return _shift_month(today, start_offset)[0], _shift_month(today, end_offset)[1] + if gran == "QUARTER": + q_start_month = (today.month - 1) // 3 * 3 + 1 + anchor = date(today.year, q_start_month, 1) + return _shift_month(anchor, start_offset * 3)[0], _shift_month(anchor, end_offset * 3 + 2)[1] + if gran == "YEAR": + return date(today.year + start_offset, 1, 1), date(today.year + end_offset, 12, 31) + except (ValueError, OverflowError): + return None return None diff --git a/packages/gooddata-eval/tests/test_scoring.py b/packages/gooddata-eval/tests/test_scoring.py index ba7ccc80e..f5aa20649 100644 --- a/packages/gooddata-eval/tests/test_scoring.py +++ b/packages/gooddata-eval/tests/test_scoring.py @@ -1,6 +1,7 @@ # (C) 2026 GoodData Corporation from datetime import date +import pytest from gooddata_eval.core.models import CreatedVisualization from gooddata_eval.core.scoring import ( check_filters, @@ -270,3 +271,26 @@ def test_check_filters_date_still_distinguishes_the_dataset_it_hangs_off(): expected = _date_viz(**{"from": -1, "to": -1, "granularity": "MONTH"}) actual = _date_viz(using="dataset/dt_date", **{"from": -1, "to": -1, "granularity": "MONTH"}) assert check_filters(expected, actual, _TODAY).date_ok is False + + +@pytest.mark.parametrize( + ("granularity", "offset"), + [ + ("YEAR", -_TODAY.year), # lands on year 0 + ("DAY", -(10**9)), # past timedelta's magnitude limit + ("WEEK_US", -(10**8)), + ("MONTH", -30000), + ("QUARTER", -10000), + ], +) +def test_check_filters_out_of_range_offsets_fall_back_instead_of_raising(granularity, offset): + """An offset outside the representable date range must not abort scoring. + + Every granularity can be pushed past date's 1..9999 year range (or timedelta's + magnitude limit). Letting the ValueError/OverflowError escape would fail the whole + item on one malformed filter, so the span resolves to None and the filter goes back + to literal comparison -- which still matches an identically malformed expectation. + """ + expected = _date_viz(**{"from": offset, "to": 0, "granularity": granularity}) + assert check_filters(expected, _date_viz(**{"from": offset, "to": 0, "granularity": granularity}), _TODAY).date_ok + assert not check_filters(expected, _date_viz(**{"from": -1, "to": -1, "granularity": "MONTH"}), _TODAY).date_ok diff --git a/packages/gooddata-eval/tests/test_visualization_evaluator.py b/packages/gooddata-eval/tests/test_visualization_evaluator.py index b16b00626..bd6ae3d89 100644 --- a/packages/gooddata-eval/tests/test_visualization_evaluator.py +++ b/packages/gooddata-eval/tests/test_visualization_evaluator.py @@ -1,8 +1,10 @@ # (C) 2026 GoodData Corporation import re +from datetime import date from gooddata_eval.core.evaluators import get_evaluator -from gooddata_eval.core.models import ChatResult, DatasetItem +from gooddata_eval.core.evaluators.visualization import _evaluate_visualization +from gooddata_eval.core.models import ChatResult, CreatedVisualization, DatasetItem def _item(expected_viz) -> DatasetItem: @@ -175,3 +177,35 @@ def test_detail_filters_are_empty_when_no_visualization_was_created(): result = ev.evaluate(_item(_dated("MONTH", -11, 0)), empty) assert result.detail["actual_filters"] == {"date": [], "ranking": [], "attribute": []} assert len(result.detail["expected_filters"]["date"]) == 1 + + +def _date_filtered_viz(): + return { + "id": "x", + "type": "table", + "query": { + "fields": {"m_rev": {"using": "metric/revenue"}}, + "filter_by": { + "f": {"type": "date_filter", "using": "dataset/dt", "from": -1, "to": -1, "granularity": "MONTH"} + }, + }, + "metrics": ["m_rev"], + } + + +def test_reported_filters_use_the_same_date_anchor_as_the_score(): + """The filters reported in detail must be resolved against the anchor the score used. + + check_filters shares one anchor across its two sides, but the reported filters were + resolved by separate normalized_filters calls, each taking its own date.today(). A run + straddling midnight could therefore report periods the verdict was never computed from. + Pinning an explicit anchor here fails unless it reaches every one of those calls. + """ + viz = CreatedVisualization.model_validate(_date_filtered_viz()) + result = _evaluate_visualization(viz, viz, today=date(2026, 3, 15)) + + # -1 MONTH from 2026-03-15 is February 2026, not whatever month the suite runs in. + assert result.filter_date_score is True + assert '"from": "2026-02-01"' in result.expected_filters["date"][0] + assert '"to": "2026-02-28"' in result.expected_filters["date"][0] + assert result.expected_filters["date"] == result.actual_filters["date"]