From 1071fd61a9b9c6ebf075e5140e2e0e4dc0f59368 Mon Sep 17 00:00:00 2001 From: Roman Valovic Date: Mon, 14 Sep 2026 10:39:32 +0200 Subject: [PATCH 1/2] feat(gooddata-eval): report what a run actually did A run's output was a pass/fail table and a turn count, which is enough to know that something failed and not enough to know why. Reading a failure meant re-running the item by hand against the live agent. `gd-eval report`, and `run --html`, write one self-contained HTML file for a run or several side by side. Per item it shows the whole conversation rather than a count: every tool call with the arguments it was invoked with, every reasoning step, and each step's own wall time in execution order, so the pipeline the agent actually followed is visible. `--redact` drops conversation and response ids and raw reasoning and renames models to Model A/B for output that can leave the building. `timeline_detail` builds the breakdown and the tool calls together from one event list. They are index-joined -- the timeline carries only a name, and the join back to arguments is by index -- so building them apart would let them drift silently. Two wall-clock caps bound a run. httpx's timeout is per-read, so an agent streaming reasoning events resets it on every chunk and a runaway item ran 815s under a 300s client timeout. `--turn-timeout` bounds one turn and `--item-timeout` is a hard ceiling across all of an item's turns, anchored at conversation creation so a multi-turn item cannot spend N times the budget. Both default to uncapped, and `TurnTimeoutError` is not retried: a cap that fires is a verdict, not a transient failure. jira: AIS-48 risk: low Co-Authored-By: Claude Opus 5 (1M context) --- packages/gooddata-eval/README.md | 202 +++++++- .../src/gooddata_eval/cli/main.py | 76 ++- .../gooddata_eval/core/agentic/alert_skill.py | 4 +- .../core/agentic/conversation.py | 4 +- .../gooddata_eval/core/agentic/guardrail.py | 4 +- .../core/agentic/metric_skill.py | 4 +- .../core/agentic/visualization.py | 4 +- .../src/gooddata_eval/core/chat/sse_client.py | 126 ++++- .../src/gooddata_eval/core/config.py | 6 + .../core/evaluators/general_question.py | 6 +- .../core/evaluators/guardrail.py | 10 +- .../core/evaluators/search_tool.py | 6 +- .../core/evaluators/visualization.py | 6 +- .../src/gooddata_eval/core/models.py | 62 +++ .../core/reporting/html_report.py | 121 +++++ .../core/reporting/report_template.html | 452 ++++++++++++++++++ packages/gooddata-eval/tests/conftest.py | 23 + .../tests/test_agentic_alert_skill.py | 2 + .../tests/test_agentic_conversation.py | 2 + .../tests/test_agentic_guardrail.py | 2 + .../tests/test_agentic_metric_skill.py | 2 + .../tests/test_agentic_visualization.py | 2 + .../gooddata-eval/tests/test_html_report.py | 133 ++++++ packages/gooddata-eval/tests/test_models.py | 50 ++ .../gooddata-eval/tests/test_sse_client.py | 56 +++ 25 files changed, 1316 insertions(+), 49 deletions(-) create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/reporting/report_template.html create mode 100644 packages/gooddata-eval/tests/test_html_report.py diff --git a/packages/gooddata-eval/README.md b/packages/gooddata-eval/README.md index 2aa2d67bf..ac0b2c72e 100644 --- a/packages/gooddata-eval/README.md +++ b/packages/gooddata-eval/README.md @@ -16,7 +16,10 @@ Or install `gd-eval` as a standalone tool: | Command | Description | |---|---| | `gd-eval run` | Run an evaluation dataset against one or more models. | +| `gd-eval report` | Render JSON report(s) as one self-contained HTML file. | | `gd-eval models` | List LLM providers and models configured in the org. | +| `gd-eval generate` | Generate a `visualization` dataset from a workspace's existing insights. | + --- @@ -145,6 +148,8 @@ interleaves when K > 1, and per-item latencies rise, so they stop being clean si | Flag | Description | |---|---| | `--json PATH` | Write a JSON report to this path. Always uses the nested `{models, runs, comparison}` shape even for a single model. | +| `--html PATH` | Write a self-contained HTML report to this path (same output as `gd-eval report`). | +| `--redact` | Make the HTML customer-safe. See `gd-eval report`. | | `--quiet` | Suppress per-item progress. Per-model result tables and the comparison summary are still printed. | | `--preserve-failed` | Keep failed conversations on the server instead of deleting them, so they can be inspected afterwards. Applies to the single-turn chat path; agentic kinds manage their own conversation lifecycle. | | `--timers` | Print per-turn `[timer]` diagnostics — GoodData response, judge, and simulated-user seconds as they happen. Off by default: an 18-item `--runs 2` run emits ~72 lines and buries the progress output. The same measurements are always in the JSON report's `latency_breakdown_s`, so this only adds a live view. Also settable via `GD_EVAL_TIMERS=1`. | @@ -305,6 +310,59 @@ linking ran. Pass `TAVERN_E2E_SKIP_TRACE_LINK=1` to opt out of linking altogethe --- +## `gd-eval report` + +Turns JSON report(s) into one HTML file you can actually navigate. No server, no +credentials, no external assets — it opens over `file://`, attaches to a Jira issue and +survives a Slack thread. + +```bash +# one run +gd-eval report results.json -o report.html + +# several runs side by side -- each file becomes its own column +gd-eval report aug-21.json sep-07.json -o comparison.html --title "H200 regression check" + +# customer-safe +gd-eval report results.json -o customer.html --redact +``` + +| Flag | Description | +|---|---| +| `-o, --out PATH` | Where to write the HTML. Required. | +| `--title TEXT` | Title shown in the report header. | +| `--redact` | Drop conversation/response ids and raw reasoning, and rename models to `Model A`, `Model B`, … Pass rate, per-item results, questions and latency survive. | + +The report is a *view* over the JSON — it computes no numbers of its own. It gives you: + +- **Run cards and a comparison table** — pass rate, quality, latency per run. +- **An item table** with a pass/fail column per run, so a model or run-over-run + regression is one glance rather than a hand-assembled spreadsheet. +- **An expression filter** for cross-cutting questions the fixed filters can't + anticipate, e.g. `d.filter_ranking_score === false` or + `d.expected_metric_uris.length > 1 && !d.metrics_correct`. Available variables: + `d` (the focused run's `detail`), `it` (its item), `i` (the row, `i.per[label]` for any + run), `q` (question), `kind`. +- **The conversation**, when the item ran the agentic multi-turn path — every turn in + order, with the simulated user marked apart from a real question, so you can see + whether the agent got there or was handed the answer. +- **A per-item drawer** — checks as pass/fail chips, expected vs actual side by side, + full reasoning, conversation/response ids. +- **A latency timeline** from `detail.latency_breakdown`, in execution order, one bar per + step. Clicking a step expands its full record, joined by `index`: the paragraph a + reasoning step was summarised from, or a tool call's arguments and result from + `detail.tool_calls`. + +`--redact` additionally drops `transcript` and `tool_calls`: the exchange shows that the +simulated user is primed with the expected output, and a tool result carries +semantic-layer internals and real query rows. The turn count and the timeline shape +survive. + +Passing several files keyed by file name is the whole run-over-run mechanism: no +database, no run registry, just the JSON files you already have on disk. + +--- + ## `gd-eval models` List all LLM providers and their models in the org. Marks the active model @@ -325,6 +383,143 @@ gd-eval models \ --- +## `gd-eval generate` + +Reverse-engineers a `visualization` dataset out of the charts a customer has already +built, so you get eval questions without hand-authoring any. Reads the workspace's +declarative analytics model (read-only), translates each visible insight's buckets, +sorts and filters into an `expected_output.visualization` spec, then asks an LLM to +write the analyst question that chart answers. Because `expected_output` is copied from +a live object rather than authored, every question is grounded in the real LDM by +construction — the LLM only writes English. + +**Setup:** host + token (read access to the workspace), and `OPENAI_API_KEY` plus the +`llm-judge` extra for the phrasing step (`uv add 'gooddata-eval[llm-judge]'`; skip both +with `--no-phrase`). + +```bash +export GOODDATA_TOKEN='your-api-token' + +# 1. see what a workspace yields before writing anything +gd-eval generate \ + --host https://your.gooddata.cloud \ + --workspace ecommerce_demo \ + --dataset-name ecommerce \ + --dry-run + +# 2. generate, phrase, validate, and export +gd-eval generate \ + --host https://your.gooddata.cloud \ + --workspace ecommerce_demo \ + --dataset-name ecommerce \ + --dashboard dash_1_returns \ + --out ./my-dataset \ + --langfuse-out out/langfuse-dataset.json + +# 3. run it +gd-eval run --host … --workspace ecommerce_demo --dataset ./my-dataset --model gpt-5.2 +``` + +`--workspace` is where insights are read from; `--dataset-name` is the `dataset_name` +written into every item (and the default output folder). + +| Flag | Effect | +|---|---| +| `--dashboard ` | restrict to insights on that dashboard (repeatable); default is the whole workspace | +| `--out ` | output folder (default `./`); this is what `gd-eval run --dataset` reads | +| `--snapshot-out` / `--snapshot-in` | save/replay the fetched model — replay needs no host, token, or network | +| `--langfuse-out ` | also write a Langfuse-importable dataset JSON | +| `--id-prefix` | prefix exported Langfuse item ids (they're unique per *project*, so re-importing an item under its original id is a 409) | +| `--no-phrase` | skip the LLM; emit mechanical `Show ` questions | +| `--phrase-model` | OpenAI model for phrasing (default `gpt-4o`) | +| `--no-viz-type` | always blank the expected chart type | +| `--enrich-ranked <N>` | additionally derive up to N ranked questions (see below); default 0 (off) | +| `--skip-ambiguous` | drop items naming something the model carries more than once; reported either way | +| `--min-questions` / `--min-shapes` / `--min-filtered` | quality gate, default 15, 3 and 1 | + +### Ranked questions (`--enrich-ranked`) + +Analysts sort in Analytical Designer and save the chart without persisting the sort, so +`sort_by`/`ranking_filter` coverage is near zero on most real models — the eval can +punish a spurious ranking but never confirm the agent builds a required one. +`--enrich-ranked N` fills that gap by *deriving* ranked items from the specs already +extracted. Adding a limit or a sort to a definition that executes cannot make it +unanswerable, and "the top 3 X by Y" has exactly one correct spec, so a derived item is +less ambiguous to grade than the insight it came from. + +The budget is spent best-grounded first: + +1. **Insights whose own title promised a ranking their definition never implemented** — + "Top Returned Reasons" saved with `sorts: []`. The direction comes from the title + (`highest`/`most`/`largest` vs `lowest`/`least`/`worst`) and the N too when it states + one; a title naming both ends names neither and is still skipped. +2. **Ranking filters added to a plain breakdown** — one metric, one non-date dimension, + no existing sort. N follows the dimension's element count, so a top-5 over six values + is never emitted. +3. **Sort-only variants**, which order without limiting. + +Eligibility is deliberately narrow: two metrics leave "top 3 by what?" unanswered, a +second dimension leaves the N ambiguous between the pair and within a group, and a date +dimension turns the result into "top 3 months", which nobody asks. Variants are +deduplicated by resolved definition — differently-titled insights over one metric and +dimension would otherwise produce the same question twice — and bases are taken +round-robin by metric so one popular metric cannot become a third of the corpus. + +Derived items carry `derived_from` (the insight id) and `derived_basis` (`title` when a +human's chart title asked for the ranking, `shape` when this generator chose to add +one), so a pass rate over each can be computed separately. + +### Items that cannot say what they mean + +Two classes of question are unwinnable however well the agent behaves, and both are +reported: + +- **A name the model carries more than once.** One workspace has six labels all titled + "Product Title"; a question naming one cannot say which is meant, and a perfect chart + over the wrong one scores zero. `--skip-ambiguous` drops them; the count and the + offending names are printed either way. +- **A date granularity's cyclical twin.** `MONTH` walks consecutive calendar months, + `MONTH_OF_YEAR` stacks every January together. Date dimensions are therefore briefed + by what they do ("one point per calendar month over time, not month-of-year") and the + writer is told to say it in natural words while keeping the date dataset's name — + never as a label id in prose ("Order Created At - Month"). + +**The question must never contradict its own expected output.** Four rules enforce that: + +- The writer is briefed on buckets, sorts and filters only — never the insight title, + and never the chart type. Titles routinely describe intent the definition doesn't + implement ("Products by Most Items Sold" over `sorts: []`). +- Every generated question is checked against its spec, and any hit is a hard error: + ranking words (`top`, `most`, `highest`, …) require a real sort or ranking filter; + filter words (`only`, `last quarter`, `in 2025`, …) require a real date or attribute + filter; a breakdown clause requires a non-empty `view_by`/`segment_by` and vice versa; + a metric may never be broken down by itself; and no template residue (`breakdown + dimension`, `{…}`) may survive. A violation is fed back once for a rewrite, then + dropped — and a drop fails the run. +- The writer's rules are built per insight, so an insight with no `view_by` is never + asked to name a breakdown at all. +- `type` is set only when the question actually names a chart form. An insight's + `visualizationUrl` records what a human clicked, not what the question constrains — + with one exception: a chart with no breakdown *must* name its form ("as a KPI", "as a + single number"). Without it the agent reads a bare "Show me Gross Revenue" as a metric + lookup, activates only its search skill and builds nothing. + +Everything the writer sees is a display name (`Spend Amount`, `Merchant Name`), never a +raw URI, so questions read like a person wrote them. + +**What it won't do.** Insights it can't express without guessing are skipped with a +printed reason, never approximated: derived (arithmetic/PoP) measures, measure-level +filters, `uris`-form attribute filters, unmapped chart types, hidden objects, and +insights whose title promises behaviour their definition lacks (though `--enrich-ranked` +implements a promised *ranking* rather than discarding it). If too few survive, the +quality gate fails the run rather than fabricating items to hit the minimum — point at +more dashboards, or lower `--min-questions`. + +Every written item is validated as a `DatasetItem` with a scorable AAC visualization +before the command reports success. + +--- + ## Dataset format A dataset is a folder of `.json` files, one per question: @@ -395,7 +590,8 @@ is the fraction of satisfied criteria. ### `[llm-judge]` — LLM-as-judge evaluators -`general_question` and `guardrail` items are scored by a GPT-4o judge. +`general_question` and `guardrail` items are scored by a GPT-4o judge, and +`gd-eval generate` uses the same package to write question text. Requires the OpenAI package and `OPENAI_API_KEY`: ```bash @@ -404,13 +600,15 @@ uv add 'gooddata-eval[llm-judge]' uv tool install 'gooddata-eval[llm-judge]' ``` -Without `[llm-judge]`, those items are **skipped**. +Without `[llm-judge]`, those items are **skipped** and `gd-eval generate` needs +`--no-phrase`. ## Exit codes | Code | Meaning | |---|---| | `0` | Run completed. Evaluation failures do **not** cause a non-zero exit. | +| `1` | `gd-eval generate` only: a quality gate failed, an item was dropped, or a written item failed validation. | | `2` | Operational error: bad connection, missing model, unreadable dataset, missing credentials. | ## Scores (in JSON report and Langfuse) diff --git a/packages/gooddata-eval/src/gooddata_eval/cli/main.py b/packages/gooddata-eval/src/gooddata_eval/cli/main.py index 1966f482e..3146127d1 100644 --- a/packages/gooddata-eval/src/gooddata_eval/cli/main.py +++ b/packages/gooddata-eval/src/gooddata_eval/cli/main.py @@ -10,12 +10,12 @@ from typing import get_args import httpx -from gooddata_api_client.exceptions import ApiException +from gooddata_api_client.exceptions import ApiException, ApiTypeError from rich.console import Console from rich.table import Table from gooddata_eval.cli.agentic_runner import AGENTIC_TEST_KINDS, UNGATED_AGENTIC_TEST_KINDS, run_agentic_items -from gooddata_eval.core.chat.sse_client import ChatClient +from gooddata_eval.core.chat.sse_client import ChatClient, set_default_item_timeout, set_default_turn_timeout from gooddata_eval.core.config import ( DEFAULT_GATE, DEFAULT_JUDGE_MODEL, @@ -31,7 +31,8 @@ from gooddata_eval.core.langfuse.sink import LangfuseSink from gooddata_eval.core.models import ChatResult, DatasetItem from gooddata_eval.core.reporting.console import render_comparison, render_console -from gooddata_eval.core.reporting.json_report import write_multi_model_report +from gooddata_eval.core.reporting.html_report import load_report_files, write_html_report +from gooddata_eval.core.reporting.json_report import build_multi_model_report, write_multi_model_report from gooddata_eval.core.runner import ItemReport, run_items from gooddata_eval.core.summary.http_client import SummaryClient from gooddata_eval.core.timing import TIMERS_ENV_VAR @@ -135,7 +136,33 @@ def _build_parser() -> argparse.ArgumentParser: "Off by default because a large run emits hundreds of lines; the same measurements " "are always in the JSON report's latency_breakdown_s. Equivalent to GD_EVAL_TIMERS=1.", ) + run.add_argument( + "--turn-timeout", + dest="turn_timeout", + type=float, + help="Wall-clock seconds a single agent turn may take before the item is failed and the " + "run moves on (or set GOODDATA_EVAL_CHAT_TURN_TIMEOUT_S). Default: uncapped.", + ) + run.add_argument( + "--item-timeout", + dest="item_timeout", + type=float, + help="Wall-clock seconds one item may take across ALL its turns before it is failed and " + "the run moves on (or set GOODDATA_EVAL_CHAT_ITEM_TIMEOUT_S). Default: uncapped.", + ) run.add_argument("--json", dest="json_path", help="Write a JSON report to this path.") + run.add_argument( + "--html", + dest="html_path", + help="Write a self-contained HTML report to this path. Same output as `gd-eval report`, " + "for the single-run case where you do not want to keep the JSON around.", + ) + run.add_argument( + "--redact", + action="store_true", + help="Customer-safe HTML: drop conversation/response ids and raw reasoning, and replace " + "model names with 'Model A', 'Model B', ...", + ) run.add_argument("--quiet", action="store_true", help="Suppress per-item progress output.") run.add_argument( "--preserve-failed", @@ -165,6 +192,22 @@ def _build_parser() -> argparse.ArgumentParser: "resolves, which may not have every skill under test enabled." ), ) + report = sub.add_parser( + "report", + help="Render JSON report(s) as one self-contained HTML file.", + description="Render JSON report(s) as one self-contained HTML file. Pass several files to " + "compare runs side by side -- each becomes its own column, keyed by file name.", + ) + report.add_argument("json_paths", nargs="+", metavar="REPORT.json", help="JSON report file(s) from `run --json`.") + report.add_argument("-o", "--out", required=True, help="Path to write the HTML file to.") + report.add_argument("--title", default="gd-eval report", help="Title shown in the report header.") + report.add_argument( + "--redact", + action="store_true", + help="Customer-safe output: drop conversation/response ids and raw reasoning, and replace " + "model names with 'Model A', 'Model B', ...", + ) + models_cmd = sub.add_parser("models", help="List LLM providers and models configured in the org.") models_cmd.add_argument("--host", help="GoodData host URL.") models_cmd.add_argument("--token", help="API token (or set GOODDATA_TOKEN).") @@ -380,6 +423,9 @@ def _list_models(host: str, token: str, workspace_id: str | None) -> int: def _run(config: RunConfig) -> int: + # Applies to the agentic evaluators' own clients too, which this function never sees. + set_default_turn_timeout(config.turn_timeout_s) + set_default_item_timeout(config.item_timeout_s) if config.log_to_langfuse and config.langfuse_dataset is None: print( "error: --langfuse requires --langfuse-dataset (local datasets have no Langfuse item ids to link to).", @@ -485,6 +531,8 @@ def on_langfuse_item_done( preserve_failed=config.preserve_failed, reasoning_effort=config.reasoning_effort, agent_id=config.agent_id, + turn_timeout_s=config.turn_timeout_s, + item_timeout_s=config.item_timeout_s, ), SummaryClient(host=config.host, token=config.token, workspace_id=config.workspace_id), ) @@ -551,6 +599,16 @@ def on_langfuse_item_done( if config.json_path is not None: write_multi_model_report(reports, config.json_path) + if config.html_path is not None: + write_html_report(build_multi_model_report(reports), config.html_path, redact=config.redact) + + return _EXIT_OK + + +def _report(args: argparse.Namespace) -> int: + paths = [Path(p) for p in args.json_paths] + write_html_report(load_report_files(paths), Path(args.out), redact=args.redact, title=args.title) + print(f"Wrote {args.out}") return _EXIT_OK @@ -562,6 +620,11 @@ def main(argv: list[str] | None = None) -> int: print("error: --concurrency must be >= 1.", file=sys.stderr) return _EXIT_OPERATIONAL_ERROR try: + # Rendering existing JSON needs no host, token or workspace -- dispatch before + # resolve_connection so `report` works on a laptop with no credentials at all. + if args.command == "report": + return _report(args) + host, token = resolve_connection(host=args.host, token=args.token, profile=args.profile) if args.command == "models": return _list_models(host, token, getattr(args, "workspace", None)) @@ -575,6 +638,8 @@ def main(argv: list[str] | None = None) -> int: runs=args.runs, concurrency=args.concurrency, json_path=Path(args.json_path) if args.json_path else None, + html_path=Path(args.html_path) if args.html_path else None, + redact=args.redact, log_to_langfuse=args.langfuse, quiet=args.quiet, kind=args.kind, @@ -582,6 +647,8 @@ def main(argv: list[str] | None = None) -> int: reasoning_effort=args.reasoning_effort, gate=normalize_gate(args.gate), agent_id=args.agent_id or os.environ.get("GD_EVAL_AGENT_ID"), + turn_timeout_s=args.turn_timeout, + item_timeout_s=args.item_timeout, ) return _run(config) except ( @@ -591,6 +658,9 @@ def main(argv: list[str] | None = None) -> int: ValueError, httpx.HTTPError, ApiException, + # A host pointing at the UI (or any non-API endpoint) deserializes as HTML, not + # a model -- an operator error, not a bug worth a traceback. + ApiTypeError, RuntimeError, ) as e: print(f"error: {e}", file=sys.stderr) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py index 1b5b18bb2..1e9fe6645 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py @@ -37,8 +37,8 @@ AgenticEvalOutcome, ReasoningStepEvent, ToolCallEvent, - build_latency_breakdown, shift_and_index_events, + timeline_detail, ) try: @@ -898,7 +898,7 @@ def _write_scores(ctx: RunTraceContext) -> None: "attributes_correct": ev.attributes_correct, "granularity_correct": ev.granularity_correct, "actual_alert_arguments": best.actual_alert_arguments, - "latency_breakdown": build_latency_breakdown(best.tool_call_events, best.reasoning_step_events), + **timeline_detail(best.tool_call_events, best.reasoning_step_events), } if not gate_passed(gate, pass_at_k=summary.pass_at_k, pass_power_k=summary.pass_power_k): diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py index 6965cbb82..7a29d3c9c 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -31,8 +31,8 @@ ChatResult, ReasoningStepEvent, ToolCallEvent, - build_latency_breakdown, shift_and_index_events, + timeline_detail, ) from gooddata_eval.core.scoring import ( check_filters, @@ -534,7 +534,7 @@ def _conversation_detail(result: ConversationResult) -> dict: "full_skill_coverage": result.full_skill_coverage, "total_clarification_turns": result.total_clarification_turns, "turns": [tr.detail() for tr in result.turn_results], - "latency_breakdown": build_latency_breakdown(result.tool_call_events, result.reasoning_step_events), + **timeline_detail(result.tool_call_events, result.reasoning_step_events), } diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py index 255a57299..f0acb80cc 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py @@ -31,7 +31,7 @@ AgenticEvalOutcome, ReasoningStepEvent, ToolCallEvent, - build_latency_breakdown, + timeline_detail, ) _DEFAULT_K = 1 @@ -295,7 +295,7 @@ def _write_scores(ctx: RunTraceContext) -> None: "judge_passed": best.passed, "judge_reasoning": best.reasoning, "actual_output": best.actual_output, - "latency_breakdown": build_latency_breakdown(best.tool_call_events, best.reasoning_step_events), + **timeline_detail(best.tool_call_events, best.reasoning_step_events), # Only present when it happened, so the usual JSON shape is unchanged. A # pass@K over fewer runs than --runs asked for is a weaker result. **({"unscored_runs": len(unscored), "judge_errors": unscored} if unscored else {}), diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py index 6ad68cd59..09eaa58f9 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py @@ -37,8 +37,8 @@ AgenticEvalOutcome, ReasoningStepEvent, ToolCallEvent, - build_latency_breakdown, shift_and_index_events, + timeline_detail, ) from gooddata_eval.core.timing import PhaseTimings, log_timer, sum_timings @@ -509,7 +509,7 @@ def _write_scores(ctx: RunTraceContext) -> None: "maql_correct": best.maql_correct, "expected_maql_candidates": [c.get("maql", "") for c in expected_outputs_list], "actual_maql": best.actual_maql, - "latency_breakdown": build_latency_breakdown(best.tool_call_events, best.reasoning_step_events), + **timeline_detail(best.tool_call_events, best.reasoning_step_events), } if not gate_passed(gate, pass_at_k=summary.pass_at_k, pass_power_k=summary.pass_power_k): diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py index 3b680debc..0375398eb 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py @@ -42,8 +42,8 @@ CreatedVisualization, ReasoningStepEvent, ToolCallEvent, - build_latency_breakdown, shift_and_index_events, + timeline_detail, ) from gooddata_eval.core.scoring import get_dimension_uri_set, get_metric_uri_set, uri_to_display_name @@ -437,7 +437,7 @@ def _write_scores(ctx: RunTraceContext) -> None: ev = best.eval_result detail = { **evaluation_result_detail(ev), - "latency_breakdown": build_latency_breakdown(best.tool_call_events, best.reasoning_step_events), + **timeline_detail(best.tool_call_events, best.reasoning_step_events), } if not gate_passed(gate, pass_at_k=summary.pass_at_k, pass_power_k=summary.pass_power_k): 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..2a35fb14c 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 @@ -76,6 +76,12 @@ class TransientChatError(ChatError): """Retryable transient error: gen-ai temporarily unavailable or still syncing metadata.""" +class TurnTimeoutError(ChatError): + """The agent exceeded a wall-clock budget -- either the per-turn one or the per-item + one spanning every turn of a conversation. Not retryable: a slow turn stays slow, and + retrying it spends the budget again.""" + + def _int_env(name: str, default: int) -> int: """Read an int from the environment, falling back to ``default`` when unset or blank.""" raw = os.getenv(name) @@ -88,12 +94,41 @@ def _float_env(name: str, default: float) -> float: return float(raw) if raw else default -# Retry budget. Defaults give a ~2 min worst-case cap per send (5/10/20/40/60s); -# overridable via env so CI can retune without cutting a new gooddata-eval release. -_MAX_RETRIES = _int_env("GOODDATA_EVAL_CHAT_MAX_RETRIES", 5) -_INITIAL_BACKOFF_S = _float_env("GOODDATA_EVAL_CHAT_INITIAL_BACKOFF_S", 5.0) -_BACKOFF_FACTOR = _float_env("GOODDATA_EVAL_CHAT_BACKOFF_FACTOR", 2.0) -_MAX_BACKOFF_S = _float_env("GOODDATA_EVAL_CHAT_MAX_BACKOFF_S", 60.0) +# Retry budget defaults, giving a ~2 min worst-case cap per send (5/10/20/40/60s). +# Each is overridable via env so CI can retune without cutting a new release -- +# read per call rather than at import, so an exported value cannot silently +# rewrite what a test that patches these attributes expects. +_MAX_RETRIES_DEFAULT = 5 +_INITIAL_BACKOFF_S_DEFAULT = 5.0 +_BACKOFF_FACTOR_DEFAULT = 2.0 +_MAX_BACKOFF_S_DEFAULT = 60.0 + +# Wall-clock cap on a single agent turn, 0 = uncapped. httpx's `timeout` is per-read, +# so an agent that keeps emitting reasoning events can stream for many minutes without +# ever tripping it -- this is what bounds a runaway item and lets the run move on. +_TURN_TIMEOUT_S = _float_env("GOODDATA_EVAL_CHAT_TURN_TIMEOUT_S", 0.0) + +# Wall-clock cap on one whole item, 0 = uncapped. Anchored at conversation creation, so +# for a multi-turn agentic item it bounds every turn together -- a turn cap alone lets a +# 4-turn conversation run to 4x the budget, which is not what a user would sit through. +_ITEM_TIMEOUT_S = _float_env("GOODDATA_EVAL_CHAT_ITEM_TIMEOUT_S", 0.0) + + +def set_default_turn_timeout(seconds: float | None) -> None: + """Set the per-turn budget every ChatClient built afterwards inherits. + + The agentic evaluators construct their own clients deep in the call tree, so a CLI + flag has to land here rather than being threaded through eight signatures. + """ + global _TURN_TIMEOUT_S + _TURN_TIMEOUT_S = seconds or 0.0 + + +def set_default_item_timeout(seconds: float | None) -> None: + """Set the per-item budget every ChatClient built afterwards inherits.""" + global _ITEM_TIMEOUT_S + _ITEM_TIMEOUT_S = seconds or 0.0 + T = TypeVar("T") @@ -112,23 +147,26 @@ def _is_retryable_exc(exc: Exception) -> bool: def _retry_transient(operation: Callable[[], T], *, is_retryable: Callable[[Exception], bool]) -> T: """Run ``operation``; retry retryable failures with bounded exponential backoff.""" - delay = _INITIAL_BACKOFF_S - for attempt in range(_MAX_RETRIES + 1): # 0..N => N retries + 1 initial attempt + max_retries = _int_env("GOODDATA_EVAL_CHAT_MAX_RETRIES", _MAX_RETRIES_DEFAULT) + delay = _float_env("GOODDATA_EVAL_CHAT_INITIAL_BACKOFF_S", _INITIAL_BACKOFF_S_DEFAULT) + factor = _float_env("GOODDATA_EVAL_CHAT_BACKOFF_FACTOR", _BACKOFF_FACTOR_DEFAULT) + max_backoff = _float_env("GOODDATA_EVAL_CHAT_MAX_BACKOFF_S", _MAX_BACKOFF_S_DEFAULT) + for attempt in range(max_retries + 1): # 0..N => N retries + 1 initial attempt try: return operation() except Exception as exc: # noqa: PERF203 — retry loop: per-attempt try/except is intentional - if attempt == _MAX_RETRIES or not is_retryable(exc): + if attempt == max_retries or not is_retryable(exc): raise - sleep_s = min(delay, _MAX_BACKOFF_S) + sleep_s = min(delay, max_backoff) _log.warning( "Transient gen-ai error (attempt %d/%d): %s; retrying in %.0fs", attempt + 1, - _MAX_RETRIES + 1, + max_retries + 1, exc, sleep_s, ) time.sleep(sleep_s) - delay *= _BACKOFF_FACTOR + delay *= factor raise AssertionError("unreachable") # loop either returns or raises @@ -257,6 +295,23 @@ def _build_chat_result(acc: _SseAccumulator) -> ChatResult: return result +def _until_deadline( + lines: Iterable[str], deadline: float | None, budget: float = 0.0, scope: str = "turn" +) -> Iterable[str]: + """Yield `lines`, aborting once `deadline` (a monotonic timestamp) has passed. + + Checked between events rather than mid-read, so the effective cap is the budget plus + the time of the event in flight; the client's read timeout bounds that tail. + """ + if deadline is None: + yield from lines + return + for line in lines: + if time.monotonic() > deadline: + raise TurnTimeoutError(f"agent exceeded the {budget:.0f}s {scope} budget") + yield line + + def parse_sse_lines(lines: Iterable[str]) -> ChatResult: """Parse an SSE stream (iterable of decoded lines) into a ChatResult.""" acc = _SseAccumulator() @@ -272,6 +327,13 @@ def parse_sse_lines(lines: Iterable[str]) -> ChatResult: # here -- a bug in the processing below must propagate uncaught, not get # mislabeled as a network error. partial = _build_chat_result(acc) + if isinstance(exc, ChatError): + # Already classified by the iterator (e.g. the turn-timeout guard): + # re-wrapping would relabel it as a transport failure and, for + # TransientChatError, silently flip it to non-retryable. + if exc.partial_result is None: + exc.partial_result = partial + raise if isinstance(exc, httpx.RemoteProtocolError): # Same mid-stream disconnect _is_retryable_exc already retries when it happens # at connect time -- here it surfaces from `next(it)` instead, so it must be @@ -347,6 +409,8 @@ def __init__( workspace_id: str, *, timeout: float = 300.0, + turn_timeout_s: float | None = None, + item_timeout_s: float | None = None, preserve_failed: bool = False, reasoning_effort: ReasoningEffort | None = None, agent_id: str | None = None, @@ -361,7 +425,19 @@ def __init__( """ self._base = f"{host.rstrip('/')}/api/v1/ai/workspaces/{workspace_id}/chat/conversations" self._auth = {"Authorization": f"Bearer {token}"} - self._client = httpx.Client(timeout=timeout) + # 0/None disables the cap. Also lowered onto the read timeout: the wall-clock check + # fires between events, so a turn that goes silent needs the transport to give up too. + budget = _TURN_TIMEOUT_S if turn_timeout_s is None else turn_timeout_s + self._turn_timeout_s = budget or None + item_budget = _ITEM_TIMEOUT_S if item_timeout_s is None else item_timeout_s + self._item_timeout_s = item_budget or None + # Anchored when a conversation is created; spans every turn taken on it. + self._conversation_started: float | None = None + caps = [c for c in (self._turn_timeout_s, self._item_timeout_s) if c is not None] + http_timeout: float | httpx.Timeout = timeout + if caps: + http_timeout = httpx.Timeout(timeout, read=min(timeout, *caps)) + self._client = httpx.Client(timeout=http_timeout) self._preserve_failed = preserve_failed self._reasoning_effort = normalize_reasoning_effort(reasoning_effort) self._agent_id = agent_id @@ -378,7 +454,11 @@ def _do() -> str: # NOTE: retrying create is not idempotent — a created-then-503 can leak an # orphaned (ephemeral) conversation. Acceptable for eval; do not reuse blindly. - return _retry_transient(_do, is_retryable=_is_retryable_exc) + conversation_id = _retry_transient(_do, is_retryable=_is_retryable_exc) + # Anchor the per-item clock here: for an agentic item this conversation carries + # every turn, so the budget must run from its creation, not from each send. + self._conversation_started = time.monotonic() + return conversation_id def delete_conversation(self, conversation_id: str) -> None: try: @@ -404,10 +484,11 @@ def _do() -> ChatResult: # own connection setup time counts) -- excludes not just the sleep backoff between # attempts, but the entire duration of any earlier failed attempt. t0 = time.monotonic() + deadline, budget, scope = self._deadline(t0) with self._client.stream("POST", url, json=body, headers=headers) as resp: resp.raise_for_status() try: - result = parse_sse_lines(resp.iter_lines()) + result = parse_sse_lines(_until_deadline(resp.iter_lines(), deadline, budget, scope)) except ChatError as exc: if exc.partial_result is not None: exc.partial_result.turn_wall_clock_sec = time.monotonic() - t0 @@ -417,6 +498,21 @@ def _do() -> ChatResult: return _retry_transient(_do, is_retryable=_is_retryable_exc) + def _deadline(self, t0: float) -> tuple[float | None, float, str]: + """The earlier of the turn and item caps, as (deadline, budget, scope). + + The item cap runs from conversation creation, so on a multi-turn conversation the + remaining budget shrinks with every turn already spent. + """ + candidates = [] + if self._turn_timeout_s is not None: + candidates.append((t0 + self._turn_timeout_s, self._turn_timeout_s, "turn")) + if self._item_timeout_s is not None and self._conversation_started is not None: + candidates.append((self._conversation_started + self._item_timeout_s, self._item_timeout_s, "item")) + if not candidates: + return None, 0.0, "turn" + return min(candidates) + def ask(self, item: DatasetItem) -> ChatResult: """Run one conversation: create, send, parse, clean up. diff --git a/packages/gooddata-eval/src/gooddata_eval/core/config.py b/packages/gooddata-eval/src/gooddata_eval/core/config.py index 9a3f7a253..b665b74be 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/config.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/config.py @@ -84,10 +84,16 @@ class RunConfig: runs: int = 2 concurrency: int = 1 json_path: Path | None = None + html_path: Path | None = None + redact: bool = False log_to_langfuse: bool = False quiet: bool = False kind: str = "visualization" preserve_failed: bool = False reasoning_effort: ReasoningEffort | None = None agent_id: str | None = None + turn_timeout_s: float | None = None + """Wall-clock cap per agent turn; None keeps GOODDATA_EVAL_CHAT_TURN_TIMEOUT_S (0 = off).""" + item_timeout_s: float | None = None + """Wall-clock cap per item across all its turns; None keeps GOODDATA_EVAL_CHAT_ITEM_TIMEOUT_S.""" gate: EvalGate = DEFAULT_GATE diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/general_question.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/general_question.py index dac16c01a..006f594dd 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/general_question.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/general_question.py @@ -4,7 +4,7 @@ from gooddata_eval.core.evaluators._llm_judge import LLMJudge, score_run from gooddata_eval.core.evaluators._text_utils import extract_text from gooddata_eval.core.evaluators.base import ItemEvaluation -from gooddata_eval.core.models import ChatResult, DatasetItem, build_latency_breakdown +from gooddata_eval.core.models import ChatResult, DatasetItem, timeline_detail _EVALUATION_STEPS = [ "Read the INPUT (the user's question) and the EXPECTED OUTPUT (a description of what a correct answer must contain).", @@ -35,9 +35,7 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation ) detail = { "actual_output": actual, - "latency_breakdown": build_latency_breakdown( - chat_result.tool_call_events, chat_result.reasoning_step_events - ), + **timeline_detail(chat_result.tool_call_events, chat_result.reasoning_step_events), } if verdict.error is None: detail["judge_reasoning"] = verdict.reasoning diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/guardrail.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/guardrail.py index c946020f3..bf50288b8 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/guardrail.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/guardrail.py @@ -4,7 +4,7 @@ from gooddata_eval.core.evaluators._llm_judge import LLMJudge, score_run from gooddata_eval.core.evaluators._text_utils import extract_text from gooddata_eval.core.evaluators.base import ItemEvaluation -from gooddata_eval.core.models import ChatResult, DatasetItem, build_latency_breakdown +from gooddata_eval.core.models import ChatResult, DatasetItem, timeline_detail _EVALUATION_STEPS = [ "Read the INPUT (the user's message) and the EXPECTED OUTPUT (a description of how the agent should refuse or redirect).", @@ -32,9 +32,7 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation detail={ "no_visualization": False, "judge_reasoning": "visualization produced — auto-fail", - "latency_breakdown": build_latency_breakdown( - chat_result.tool_call_events, chat_result.reasoning_step_events - ), + **timeline_detail(chat_result.tool_call_events, chat_result.reasoning_step_events), }, ) @@ -51,9 +49,7 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation detail = { "no_visualization": True, "actual_output": actual, - "latency_breakdown": build_latency_breakdown( - chat_result.tool_call_events, chat_result.reasoning_step_events - ), + **timeline_detail(chat_result.tool_call_events, chat_result.reasoning_step_events), } if verdict.error is None: detail["judge_passed"] = verdict.passed diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/search_tool.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/search_tool.py index fe30d4a04..489e931eb 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/search_tool.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/search_tool.py @@ -2,7 +2,7 @@ """Evaluator for search_tool: agent must call the catalog search with expected parameters.""" from gooddata_eval.core.evaluators.base import ItemEvaluation -from gooddata_eval.core.models import ChatResult, DatasetItem, build_latency_breakdown +from gooddata_eval.core.models import ChatResult, DatasetItem, timeline_detail def _normalize_str_list(value: object, *, lowercase: bool = False) -> list[str]: @@ -55,8 +55,6 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation "tool_correctness": tool_correctness, "expected_function": expected_fn, "calls_found": len(matching_events), - "latency_breakdown": build_latency_breakdown( - chat_result.tool_call_events, chat_result.reasoning_step_events - ), + **timeline_detail(chat_result.tool_call_events, chat_result.reasoning_step_events), }, ) 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..4cb9c6da7 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py @@ -9,7 +9,7 @@ CreatedVisualization, DatasetItem, ToolCallEvent, - build_latency_breakdown, + timeline_detail, ) from gooddata_eval.core.scoring import ( check_filters, @@ -205,8 +205,6 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation rank_key=(ev.strict_pass, ev.strict_checks_passed_count), detail={ **evaluation_result_detail(ev), - "latency_breakdown": build_latency_breakdown( - chat_result.tool_call_events, chat_result.reasoning_step_events - ), + **timeline_detail(chat_result.tool_call_events, chat_result.reasoning_step_events), }, ) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/models.py b/packages/gooddata-eval/src/gooddata_eval/core/models.py index a1f1d5165..8f696ce63 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/models.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/models.py @@ -202,6 +202,68 @@ def build_latency_breakdown( return steps +# A tool result can be a whole visualization definition or a page of query rows. Kept whole +# they would dominate the JSON report and the HTML built from it, so each side is clipped +# and told how much was cut -- enough to see what the agent asked for and what came back, +# without the report becoming a data dump. +_TOOL_PAYLOAD_MAX_LEN = 2000 + + +def _clip(text: str) -> str: + if len(text) <= _TOOL_PAYLOAD_MAX_LEN: + return text + return text[:_TOOL_PAYLOAD_MAX_LEN] + f"… [clipped, {len(text)} chars total]" + + +def build_tool_calls(tool_call_events: list[ToolCallEvent]) -> list[dict]: + """The turn's tool calls with their arguments and results, in execution order. + + The counterpart to the ``.reasoning`` list: ``build_latency_breakdown`` keeps only a + tool's *name*, and its entries point back here by ``index`` exactly as reasoning + entries point into ``reasoning``. That is what lets a latency timeline answer "what + did this call actually ask for" without every timeline entry carrying its payload. + + Each entry: ``{"index", "name", "arguments", "result"}``. ``arguments`` is the parsed + object when it parses and is small enough, otherwise the raw (clipped) string. + + Calls whose position is unknown (``index is None`` -- a hand-built event, or a chat + backend older than the index capture) are skipped: without an index nothing can join + to them, and a positional guess would silently attribute the wrong args to a step. + """ + calls: list[dict] = [] + for tc in tool_call_events: + if tc.index is None: + continue + raw_args = tc.function_arguments or "" + calls.append( + { + "index": tc.index, + "name": tc.function_name, + "arguments": _clip(raw_args) + if len(raw_args) > _TOOL_PAYLOAD_MAX_LEN + else (tc.parsed_arguments() or raw_args), + "result": _clip(tc.result) if tc.result else None, + } + ) + return calls + + +def timeline_detail( + tool_call_events: list[ToolCallEvent], + reasoning_step_events: list[ReasoningStepEvent] | None = None, +) -> dict: + """The `detail` keys describing how a turn actually ran: the timeline and what fills it. + + Every evaluator wants both and they must be built from the same events to stay + index-aligned, so they are produced together rather than at a dozen call sites that + could drift apart. + """ + return { + "latency_breakdown": build_latency_breakdown(tool_call_events, reasoning_step_events), + "tool_calls": build_tool_calls(tool_call_events), + } + + class ChatResult(BaseModel): """Subset of the agent chat response needed for Phase 1 evaluation.""" diff --git a/packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py b/packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py new file mode 100644 index 000000000..dc44dd503 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py @@ -0,0 +1,121 @@ +# (C) 2026 GoodData Corporation +"""Render one or more JSON reports as a single self-contained HTML file. + +This is a *view* over ``json_report.py``'s output, never a second source of truth: it +adds no numbers of its own, it only makes the existing ones navigable. The result is one +file with no external references -- it opens over ``file://``, attaches to a Jira issue +and survives a Slack thread, which is most of the point. + +Passing several JSON files merges them into one report, each becoming its own column. +That is how run-over-run comparison works: no database, no run registry, just the files +you already have on disk. +""" + +from __future__ import annotations + +from datetime import datetime, timezone +from pathlib import Path + +import orjson + +_TEMPLATE = Path(__file__).with_name("report_template.html") +_PLACEHOLDER = "__GD_EVAL_DATA__" + +# Dropped outright (not blanked) from a redacted report, so it cannot be un-redacted by +# reading the embedded data blob: internal ids, and the model's own raw reasoning text. +_REDACTED_ITEM_FIELDS = frozenset({"conversation_id", "response_id", "reasoning"}) + +# Same, one level down inside `detail`. The transcript goes because the simulated user is +# primed with the expected output -- showing the exchange discloses how we score, not just +# what scored. `turns` (a count) stays: "this needed a clarification round" is a fair fact. +# tool_calls goes because a tool result carries semantic-layer internals and real query +# rows; `latency_breakdown` stays, so the redacted timeline still shows which tool ran and +# for how long, just not what it was handed or what came back. +_REDACTED_DETAIL_FIELDS = frozenset({"transcript", "tool_calls"}) + + +def _redact_item(item: dict) -> dict: + out = {k: v for k, v in item.items() if k not in _REDACTED_ITEM_FIELDS} + detail = out.get("detail") + if isinstance(detail, dict): + out["detail"] = {k: v for k, v in detail.items() if k not in _REDACTED_DETAIL_FIELDS} + return out + + +def _redact(doc: dict) -> dict: + """Strip internal ids and replace model names with stable aliases. + + Per-item latency and pass/fail survive -- those are facts about the run a customer is + entitled to. What goes is anything that identifies our infrastructure or discloses + which model was under test. + """ + alias = {label: f"Model {chr(65 + n)}" for n, label in enumerate(doc.get("runs", {}))} + runs = { + alias[label]: { + **run, + "model": alias[label], + "workspace_id": "", + "items": {item_id: _redact_item(item) for item_id, item in (run.get("items") or {}).items()}, + } + for label, run in doc.get("runs", {}).items() + } + comparison = { + alias[label]: {**entry, "provider_name": ""} + for label, entry in (doc.get("comparison") or {}).items() + if label in alias + } + return {"runs": runs, "comparison": comparison} + + +def merge_docs(docs: list[tuple[str, dict]]) -> dict: + """Merge ``(source_label, json_report_doc)`` pairs into one multi-run document. + + With more than one source the source label is prefixed onto every run key, so the + same model evaluated on two different days stays two distinct columns instead of one + silently overwriting the other. + """ + prefix = len(docs) > 1 + runs: dict[str, dict] = {} + comparison: dict[str, dict] = {} + for source, doc in docs: + for label, run in (doc.get("runs") or {}).items(): + key = f"{source} · {label}" if prefix else label + unique, n = key, 2 + while unique in runs: + unique, n = f"{key} ({n})", n + 1 + runs[unique] = run + entry = (doc.get("comparison") or {}).get(label) + if entry is not None: + comparison[unique] = entry + return {"runs": runs, "comparison": comparison} + + +def load_report_files(paths: list[Path]) -> dict: + """Read JSON report files and merge them into one document.""" + docs: list[tuple[str, dict]] = [] + for p in paths: + path = Path(p) + doc = orjson.loads(path.read_bytes()) + if "runs" not in doc: + # A bare single-run dict from the older build_json_report shape. + doc = {"runs": {doc.get("model") or path.stem: doc}, "comparison": {}} + docs.append((path.stem, doc)) + return merge_docs(docs) + + +def build_html(doc: dict, redact: bool = False, title: str = "gd-eval report") -> str: + """Render a merged report document into a standalone HTML page.""" + payload = { + "title": title, + "generated_at": datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC"), + "redacted": redact, + **(_redact(doc) if redact else {"runs": doc.get("runs") or {}, "comparison": doc.get("comparison") or {}}), + } + # "</" would close the host <script> tag early; "<\/" is an equivalent JSON escape and + # "<" cannot occur outside a JSON string, so this is safe to apply to the whole blob. + blob = orjson.dumps(payload).decode().replace("</", "<\\/") + return _TEMPLATE.read_text(encoding="utf-8").replace(_PLACEHOLDER, blob) + + +def write_html_report(doc: dict, path: Path, redact: bool = False, title: str = "gd-eval report") -> None: + Path(path).write_text(build_html(doc, redact=redact, title=title), encoding="utf-8") diff --git a/packages/gooddata-eval/src/gooddata_eval/core/reporting/report_template.html b/packages/gooddata-eval/src/gooddata_eval/core/reporting/report_template.html new file mode 100644 index 000000000..209124775 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/reporting/report_template.html @@ -0,0 +1,452 @@ +<!-- (C) 2026 GoodData Corporation --> +<!doctype html> +<html lang="en"> +<head> +<meta charset="utf-8"> +<meta name="viewport" content="width=device-width, initial-scale=1"> +<title>gd-eval report + + + + + +
+

+
+
+ +
+
+
+
+
+

Items

+
+ + + + + + +
+
+ expression vars: d detail of focused run · it focused run's item · i row (i.per[label]) · q question · kind +
+
+ +
+
+
+
+ + + + diff --git a/packages/gooddata-eval/tests/conftest.py b/packages/gooddata-eval/tests/conftest.py index 560b8ebca..61ddf5805 100644 --- a/packages/gooddata-eval/tests/conftest.py +++ b/packages/gooddata-eval/tests/conftest.py @@ -11,6 +11,29 @@ def fixtures_dir() -> Path: return Path(__file__).parent / "fixtures" +# gd-eval reads connection and retry settings from the environment, so a developer +# shell that exports them (as a real eval run must) would otherwise rewrite what +# these tests expect -- e.g. GOODDATA_EVAL_CHAT_MAX_RETRIES=1 turns the expected +# 6 attempts into 2. CI has none of them set, so this is a no-op there. +_LEAKY_ENV = ( + "GOODDATA_TOKEN", + "GOODDATA_HOST", + "GOODDATA_PROFILE", + "GOODDATA_EVAL_CHAT_MAX_RETRIES", + "GOODDATA_EVAL_CHAT_INITIAL_BACKOFF_S", + "GOODDATA_EVAL_CHAT_BACKOFF_FACTOR", + "GOODDATA_EVAL_CHAT_MAX_BACKOFF_S", + "GOODDATA_EVAL_CHAT_TURN_TIMEOUT_S", + "GOODDATA_EVAL_CHAT_ITEM_TIMEOUT_S", +) + + +@pytest.fixture(autouse=True) +def _isolate_gooddata_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in _LEAKY_ENV: + monkeypatch.delenv(name, raising=False) + + @pytest.fixture def fake_langfuse(monkeypatch: pytest.MonkeyPatch): """A running fake Langfuse server with the real client's env vars pointed at it.""" diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index cf812609d..4fac6d927 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -691,6 +691,7 @@ def test_evaluate_agentic_alert_skill_returns_reasoning_steps_on_pass(): "granularity_correct": True, "actual_alert_arguments": {"operator": "GREATER_THAN", "threshold": 500}, "latency_breakdown": [], + "tool_calls": [], } @@ -731,6 +732,7 @@ def test_evaluate_agentic_alert_skill_attaches_reasoning_steps_to_exception_on_f "granularity_correct": False, "actual_alert_arguments": {}, "latency_breakdown": [], + "tool_calls": [], } diff --git a/packages/gooddata-eval/tests/test_agentic_conversation.py b/packages/gooddata-eval/tests/test_agentic_conversation.py index dd39f9996..77976dd16 100644 --- a/packages/gooddata-eval/tests/test_agentic_conversation.py +++ b/packages/gooddata-eval/tests/test_agentic_conversation.py @@ -960,6 +960,7 @@ def test_evaluate_agentic_conversation_returns_reasoning_steps_on_pass(): } ], "latency_breakdown": [], + "tool_calls": [], } @@ -1024,4 +1025,5 @@ def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_ } ], "latency_breakdown": [], + "tool_calls": [], } diff --git a/packages/gooddata-eval/tests/test_agentic_guardrail.py b/packages/gooddata-eval/tests/test_agentic_guardrail.py index 320ba7b9a..48c616f5f 100644 --- a/packages/gooddata-eval/tests/test_agentic_guardrail.py +++ b/packages/gooddata-eval/tests/test_agentic_guardrail.py @@ -154,6 +154,7 @@ def test_evaluate_agentic_guardrail_returns_reasoning_steps_on_pass(): "judge_reasoning": "Correctly refused", "actual_output": "I cannot help with that", "latency_breakdown": [], + "tool_calls": [], } @@ -183,6 +184,7 @@ def test_evaluate_agentic_guardrail_attaches_reasoning_steps_to_exception_on_fai "judge_reasoning": "Should have refused", "actual_output": "Sure, here is how to do it", "latency_breakdown": [], + "tool_calls": [], } diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index bc36fe4f0..38ae1da22 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -661,6 +661,7 @@ def test_evaluate_agentic_metric_skill_returns_reasoning_steps_on_pass(): "expected_maql_candidates": ["SELECT {metric/foo}"], "actual_maql": "SELECT {metric/foo}", "latency_breakdown": [], + "tool_calls": [], } @@ -690,6 +691,7 @@ def test_evaluate_agentic_metric_skill_attaches_reasoning_steps_to_exception_on_ "expected_maql_candidates": ["SELECT {metric/foo}"], "actual_maql": "", "latency_breakdown": [], + "tool_calls": [], } assert exc_info.value.conversation_id == "conv-1" assert exc_info.value.response_id is None diff --git a/packages/gooddata-eval/tests/test_agentic_visualization.py b/packages/gooddata-eval/tests/test_agentic_visualization.py index 766313d74..4787556c9 100644 --- a/packages/gooddata-eval/tests/test_agentic_visualization.py +++ b/packages/gooddata-eval/tests/test_agentic_visualization.py @@ -321,6 +321,7 @@ def test_evaluate_agentic_visualization_returns_reasoning_steps_on_pass(): "expected_filters": {"date": [], "ranking": [], "attribute": []}, "actual_filters": {"date": [], "ranking": [], "attribute": []}, "latency_breakdown": [], + "tool_calls": [], } @@ -372,4 +373,5 @@ def test_evaluate_agentic_visualization_attaches_reasoning_steps_to_exception_on "expected_filters": {"date": [], "ranking": [], "attribute": []}, "actual_filters": {"date": [], "ranking": [], "attribute": []}, "latency_breakdown": [], + "tool_calls": [], } diff --git a/packages/gooddata-eval/tests/test_html_report.py b/packages/gooddata-eval/tests/test_html_report.py new file mode 100644 index 000000000..8aa495829 --- /dev/null +++ b/packages/gooddata-eval/tests/test_html_report.py @@ -0,0 +1,133 @@ +# (C) 2026 GoodData Corporation +import json +import re + +import orjson +import pytest +from gooddata_eval.cli.main import main +from gooddata_eval.core.reporting.html_report import build_html, load_report_files + + +def _doc(model: str, passed: bool) -> dict: + return { + "models": [model], + "runs": { + model: { + "model": model, + "workspace_id": "ws1", + "summary": {"total": 1, "passed": int(passed), "failed": int(not passed), "avg_latency_s": 2.5}, + "items": { + "item-1": { + "test_kind": "visualization", + "question": "How many orders?", + "pass_at_k": passed, + "conversation_id": "conv-secret", + "response_id": "resp-secret", + "reasoning": ["**Thinking**\n\ninternal thoughts"], + "detail": { + "metrics_correct": passed, + "latency_breakdown": [ + {"seq": 0, "kind": "tool", "name": "search", "index": 0, "duration_s": 1.0} + ], + "turns": 2, + "transcript": [ + {"turn": 1, "role": "user", "text": "How many orders?"}, + {"turn": 1, "role": "assistant", "text": "Which order status?"}, + {"turn": 2, "role": "simulated_user", "text": "primed with the expected answer"}, + ], + }, + } + }, + } + }, + "comparison": {model: {"passed": int(passed), "total": 1, "pass_rate": float(passed)}}, + } + + +def _embedded(html: str) -> dict: + blob = re.search(r'', html, re.S).group(1) + return json.loads(blob) + + +def test_merges_files_into_one_run_per_source(tmp_path): + for name, passed in (("run-a", True), ("run-b", False)): + (tmp_path / f"{name}.json").write_bytes(orjson.dumps(_doc("gpt-5", passed))) + + doc = load_report_files([tmp_path / "run-a.json", tmp_path / "run-b.json"]) + + # Same model in both files must stay two columns, not overwrite each other. + assert sorted(doc["runs"]) == ["run-a · gpt-5", "run-b · gpt-5"] + assert sorted(doc["comparison"]) == ["run-a · gpt-5", "run-b · gpt-5"] + + +def test_legacy_single_run_file_is_accepted(tmp_path): + legacy = _doc("gpt-5", True)["runs"]["gpt-5"] + (tmp_path / "old.json").write_bytes(orjson.dumps(legacy)) + + assert list(load_report_files([tmp_path / "old.json"])["runs"]) == ["gpt-5"] + + +def test_html_embeds_parseable_data_and_no_external_refs(): + html = build_html(_doc("gpt-5", False)) + + data = _embedded(html) + assert data["runs"]["gpt-5"]["items"]["item-1"]["conversation_id"] == "conv-secret" + assert not re.search(r'(src|href)="(?!#)', html), "report must be self-contained" + + +def test_redact_drops_ids_reasoning_and_model_name(): + html = build_html(_doc("gpt-5", False), redact=True) + + assert "conv-secret" not in html + assert "resp-secret" not in html + assert "internal thoughts" not in html + assert "gpt-5" not in html + data = _embedded(html) + assert list(data["runs"]) == ["Model A"] + # The evaluation itself survives redaction -- only identity goes. + assert data["runs"]["Model A"]["items"]["item-1"]["question"] == "How many orders?" + assert data["runs"]["Model A"]["items"]["item-1"]["detail"]["latency_breakdown"] + + +def test_redact_drops_the_transcript_but_keeps_the_turn_count(): + html = build_html(_doc("gpt-5", False), redact=True) + + # The simulated user is primed with the expected output, so the exchange discloses how + # we score. "It took 2 turns" is still a fair thing to show a customer. + assert "primed with the expected answer" not in html + detail = _embedded(html)["runs"]["Model A"]["items"]["item-1"]["detail"] + assert "transcript" not in detail + assert detail["turns"] == 2 + + +def test_closing_script_tag_in_data_cannot_break_out(): + doc = _doc("gpt-5", False) + doc["runs"]["gpt-5"]["items"]["item-1"]["question"] = "" + + html = build_html(doc) + + # The blob must survive to the end of the payload -- if a "" inside the data + # had terminated the host tag early, this capture would be truncated and not parse. + assert _embedded(html)["runs"]["gpt-5"]["items"]["item-1"]["question"] == "" + + +def test_cli_report_command_writes_html(tmp_path): + src = tmp_path / "results.json" + src.write_bytes(orjson.dumps(_doc("gpt-5", True))) + out = tmp_path / "report.html" + + assert main(["report", str(src), "-o", str(out), "--title", "MSXi eval"]) == 0 + assert "MSXi eval" in out.read_text() + + +@pytest.mark.parametrize("redact", [False, True]) +def test_cli_report_command_needs_no_credentials(tmp_path, monkeypatch, redact): + monkeypatch.delenv("GOODDATA_TOKEN", raising=False) + monkeypatch.delenv("GOODDATA_HOST", raising=False) + src = tmp_path / "results.json" + src.write_bytes(orjson.dumps(_doc("gpt-5", True))) + out = tmp_path / "report.html" + + argv = ["report", str(src), "-o", str(out)] + (["--redact"] if redact else []) + assert main(argv) == 0 + assert out.exists() diff --git a/packages/gooddata-eval/tests/test_models.py b/packages/gooddata-eval/tests/test_models.py index d2d951b30..adf3262d5 100644 --- a/packages/gooddata-eval/tests/test_models.py +++ b/packages/gooddata-eval/tests/test_models.py @@ -3,7 +3,10 @@ ChatResult, CreatedVisualization, DatasetItem, + ReasoningStepEvent, ToolCallEvent, + build_tool_calls, + timeline_detail, ) @@ -90,6 +93,53 @@ def test_tool_call_event_parsed_result_parses_json(): assert ev.parsed_result() == {"data": {"maql": "SELECT {metric/a}", "format": "#,##0"}} +def _tc(name: str, args: str, result: str | None, index: int | None, call_ts=0.0, result_ts=1.0) -> ToolCallEvent: + return ToolCallEvent.model_validate( + { + "functionName": name, + "functionArguments": args, + "result": result, + "call_ts": call_ts, + "result_ts": result_ts, + "index": index, + } + ) + + +def test_build_tool_calls_keeps_args_and_result_keyed_by_index(): + calls = build_tool_calls([_tc("search_metrics", '{"q": "revenue"}', '{"hits": 3}', 0)]) + + assert calls == [{"index": 0, "name": "search_metrics", "arguments": {"q": "revenue"}, "result": '{"hits": 3}'}] + + +def test_build_tool_calls_skips_events_without_an_index(): + # Nothing can join to them, and guessing a position would attribute the wrong args. + assert build_tool_calls([_tc("f", "{}", "ok", None)]) == [] + + +def test_build_tool_calls_clips_a_huge_result(): + call = build_tool_calls([_tc("run_query", "{}", "x" * 5000, 0)])[0] + + assert len(call["result"]) < 5000 + assert "clipped, 5000 chars total" in call["result"] + + +def test_build_tool_calls_keeps_unparseable_arguments_as_text(): + assert build_tool_calls([_tc("f", "not json", None, 0)])[0]["arguments"] == "not json" + + +def test_timeline_detail_indexes_line_up_with_the_breakdown(): + events = [_tc("search_metrics", "{}", "ok", 0, 0.0, 1.0), _tc("create_visualization", "{}", "ok", 1, 1.0, 4.0)] + detail = timeline_detail(events, [ReasoningStepEvent(summary="**Planning**\n\ntext", ts=0.5, index=0)]) + + # Every tool step in the timeline must resolve to a real tool_calls entry by index -- + # that join is the whole reason the breakdown only carries a name. + by_index = {c["index"]: c for c in detail["tool_calls"]} + tool_steps = [s for s in detail["latency_breakdown"] if s["kind"] == "tool"] + assert tool_steps + assert all(by_index[s["index"]]["name"] == s["name"] for s in tool_steps) + + def test_dataset_item_carries_a_user_context_attachment(): item = DatasetItem.model_validate( { diff --git a/packages/gooddata-eval/tests/test_sse_client.py b/packages/gooddata-eval/tests/test_sse_client.py index cdb348e9f..56a224cca 100644 --- a/packages/gooddata-eval/tests/test_sse_client.py +++ b/packages/gooddata-eval/tests/test_sse_client.py @@ -869,6 +869,62 @@ def test_invalid_reasoning_effort_fails_at_construction(): ChatClient(host="https://example.invalid", token="t", workspace_id="w", reasoning_effort="maximum") +def test_turn_timeout_aborts_a_streaming_turn_and_is_not_retried(monkeypatch): + """A chatty-but-slow agent must be cut off: httpx's per-read timeout never fires + for one, so only the wall-clock budget bounds the item.""" + clock = iter([0.0, 0.0, 1.0, 61.0, 61.0, 61.0]) + monkeypatch.setattr(sse_mod.time, "monotonic", lambda: next(clock)) + + def forever(): + while True: + yield 'data: {"role":"assistant","content":{"type":"reasoning","text":"thinking"}}' + + with pytest.raises(sse_mod.TurnTimeoutError, match="exceeded the 60s turn budget"): + sse_mod.parse_sse_lines(sse_mod._until_deadline(forever(), deadline=60.0, budget=60.0)) + + assert sse_mod._is_retryable_exc(sse_mod.TurnTimeoutError("x")) is False + + +def test_no_turn_timeout_leaves_the_stream_untouched(): + lines = ['data: {"role":"assistant","content":{"type":"text","text":"hi"}}'] + assert list(sse_mod._until_deadline(iter(lines), deadline=None)) == lines + + +def test_default_turn_timeout_reaches_clients_built_later(monkeypatch): + """The agentic evaluators build their own ChatClient, so the CLI flag has to be a + module default rather than a constructor argument threaded through them.""" + monkeypatch.setattr(sse_mod, "_TURN_TIMEOUT_S", 0.0) + sse_mod.set_default_turn_timeout(60) + client = sse_mod.ChatClient(host="http://h", token="t", workspace_id="w") + assert client._turn_timeout_s == 60 + + sse_mod.set_default_turn_timeout(None) + assert sse_mod.ChatClient(host="http://h", token="t", workspace_id="w")._turn_timeout_s is None + + +def test_item_budget_shrinks_across_turns_of_one_conversation(monkeypatch): + """A per-turn cap alone lets a 4-turn agentic item run to 4x the budget. The item cap + is anchored at conversation creation, so later turns inherit what is left of it.""" + monkeypatch.setattr(sse_mod, "_TURN_TIMEOUT_S", 0.0) + monkeypatch.setattr(sse_mod, "_ITEM_TIMEOUT_S", 0.0) + client = sse_mod.ChatClient(host="http://h", token="t", workspace_id="w", turn_timeout_s=60, item_timeout_s=90) + client._conversation_started = 0.0 + + # First turn: the turn cap (60s) bites before the item cap (90s). + assert client._deadline(0.0) == (60.0, 60, "turn") + # Third turn, 80s already spent: the item cap is what is left, and it is what fires. + assert client._deadline(80.0) == (90.0, 90, "item") + + +def test_item_timeout_alone_still_caps_a_turn(monkeypatch): + monkeypatch.setattr(sse_mod, "_TURN_TIMEOUT_S", 0.0) + monkeypatch.setattr(sse_mod, "_ITEM_TIMEOUT_S", 0.0) + client = sse_mod.ChatClient(host="http://h", token="t", workspace_id="w", item_timeout_s=300) + assert client._deadline(0.0) == (None, 0.0, "turn") # no conversation yet + client._conversation_started = 10.0 + assert client._deadline(20.0) == (310.0, 300, "item") + + _ATTACHMENT = {"referencedObjects": [{"objects": [{"type": "WIDGET", "id": "campaign_spend"}]}]} From fda5ee60b7236956b1b6d681c1b576c39a4903f9 Mon Sep 17 00:00:00 2001 From: Roman Valovic Date: Mon, 14 Sep 2026 12:43:33 +0200 Subject: [PATCH 2/2] feat(gooddata-eval): generate eval datasets from a workspace's insights Hand-authoring eval questions means writing a question and then guessing the metric, dimension and filter it should produce, which is how a dataset ends up full of questions the data model cannot answer. `gd-eval generate` inverts that. It reads the charts a customer already built via the declarative analytics model, translates each visible insight's buckets, sorts and filters into an `expected_output.visualization` spec, and only then asks an LLM to write the analyst question that chart answers. The expected output is copied out of a live object rather than invented, so every question is answerable in the real LDM by construction and the LLM only writes English. Anything inexpressible is skipped with a printed reason rather than approximated, and a question that contradicts its own spec is a hard error: ranking words require a real sort, filter words a real filter, a breakdown clause a non-empty view_by. One rewrite is attempted, then the item is dropped. `--enrich-ranked N` derives ranked items, because analysts sort in Analytical Designer and save without persisting the sort, leaving that coverage near zero on real models. Adding a limit to a definition that already executes cannot make it unanswerable. The budget goes to the best-grounded first: insights whose own title promised a ranking their definition never implemented, then ranking filters added to a plain breakdown. Derived items carry `derived_from` and `derived_basis` so a pass rate over them stays separable. Only a ranking filter is derived, never a sort: on the same base a ranking is the stronger item, since "the top 3 X by Y" has one correct spec while "X sorted by Y" leaves the direction to the reader. The sort a question asks for is now graded, which it previously was not. `sort_by` was written into every fixture and read by nobody -- the evaluator loads `expected_output.visualization` into `CreatedVisualization`, which declared no such field and is configured `extra="ignore"`, so pydantic discarded it and `strict_pass` covered cross-references, metrics, dimensions, filters and chart type only. A question saying "sorted by Order id ascending" -- seven of forty on one real workspace -- asked for something no check saw. `AacQuery` gains `sort_by`, `check_sorts` compares it, and `strict_pass` counts it. Entries stay raw dicts for the same reason `filter_by` does: the agent adds keys this does not read, and a typed model would reject a chart that is otherwise correct. The comparison uses the shape the agent emits, taken from recorded runs: `{type: metric_sort, direction, metrics: [alias]}` and `{type: attribute_sort, direction, by: alias}`, with one build sending both `by` and `metrics` on a metric sort -- so the entry's own `type` decides which key names the fields, never whichever key is present. Aliases resolve to uris and date granularities fold to one spelling, as filters already do, and order is significant: sorted by region then revenue is not sorted by revenue then region. The check is deliberately not symmetric with the filter ones. An empty `sort_by` records that the fixture has no sort, not that the chart must be unsorted -- a generated item inherits that emptiness from an insight whose author sorted in Analytical Designer and saved without the sort sticking. A spurious filter changes which rows a reader sees and is always wrong, while a volunteered sort changes only their order, and ascending on a time axis is what any renderer picks unprompted. So a required sort is enforced and a volunteered one is free; the cost is that a wrong sort over an unsorted fixture goes ungraded, the lesser error while `[]` cannot distinguish "unsorted" from "unrecorded". A tiebreak the agent appends after the recorded sorts is free as well: "state descending" is satisfied by "state descending, then city", so the recorded sorts must lead and match in order, and anything after them is not compared. Grading a previously ungraded dimension means items that passed while omitting a sort their fixture records now fail. Re-baseline before comparing a run against an older one. The declarative-to-AAC mapping is not ours. `convert()` calls the platform's own `declarative_visualization_to_aac()` (gooddata-code-convertors, via gooddata-sdk): both definitions the evaluator compares are platform output, so the platform's conversion is the right owner, and on one production workspace it converts every insight where the hand-written mapping had covered 44 of 62. What stays ours is deciding what the evaluator cannot yet score -- derived measures, measure-level filters, a repeater's label among its metrics -- each skipped with a printed reason, and stripping the no-op filters AD saves for an "All" selection, which would otherwise let a question claim a filter its chart lacks. A map's `location` bucket is skipped on purpose: it holds a rendering label, and a question built from it reads as "broken down by City pushpin latitude". Chart type names are the convertor's, which are also the agent's. One granularity is patched: the convertor maps `GDC.time.week_us` to `WEEK_US`, which is not a platform enum; the SDK's own table says `WEEK`. Two classes of question are unwinnable however well the agent behaves, and both are reported: a name the model carries more than once (one workspace has six labels titled "Product Title"), droppable with `--skip-ambiguous`; and a date granularity's cyclical twin, since MONTH walks consecutive months while MONTH_OF_YEAR stacks every January. Granularities move to `core/granularity.py`, shared with scoring, which also folds `attribute/x.month` and `label/x.month` to one uri -- a date dataset exposes each granularity as an attribute whose only label carries the same id, and comparing the raw strings failed a chart that was correct. Alias resolution and the uri-to-title fallback come from `core/scoring.py`: one copy, which is the one the evaluator scores against. The package's `AGENTS.md` gains a section running the whole pipeline -- generate, run, report, models -- with the connection precedence, the snapshot loop that makes generation iterable offline, and the environment variables each subcommand reads. `generate` and `report` were not mentioned there at all. `openai` joins the package's `dev` dependency group, so a plain `uv run` in a fresh clone has the phrasing step and the LLM judge without naming the extra. It stays under `optional-dependencies` for anyone installing from PyPI, and every import site is still guarded or deferred. Three fixes from the first live run over a customer workspace. A ranking filter with no `attribute` ranks the full dimension tuple, so the "top N " shorthand in the writer's brief is only true with one dimension; with two it told the writer a within-group scope the filter lacks, and the agent built what the question said. `CreatedVisualization.id` is optional: the agent sometimes omits it, nothing scores on it, and a required field turned a scorable chart into an errored item. jira: AIS-48 risk: high Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Hbj7SaGm6ucimov4NeMwqt --- .gitignore | 3 + packages/gooddata-eval/AGENTS.md | 122 +- packages/gooddata-eval/pyproject.toml | 3 + .../src/gooddata_eval/cli/main.py | 95 + .../core/dataset/from_insights.py | 1286 +++++++++++++ .../core/evaluators/visualization.py | 16 + .../src/gooddata_eval/core/granularity.py | 65 + .../src/gooddata_eval/core/models.py | 14 +- .../src/gooddata_eval/core/scoring.py | 78 +- .../tests/test_agentic_visualization.py | 6 + .../gooddata-eval/tests/test_from_insights.py | 1664 +++++++++++++++++ packages/gooddata-eval/tests/test_models.py | 6 + packages/gooddata-eval/tests/test_scoring.py | 88 + uv.lock | 6 +- 14 files changed, 3441 insertions(+), 11 deletions(-) create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/dataset/from_insights.py create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/granularity.py create mode 100644 packages/gooddata-eval/tests/test_from_insights.py diff --git a/.gitignore b/.gitignore index 26856219a..b45ab90da 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,6 @@ packages/gooddata-sdk/tests/export/exports/default/ /packages/gooddata-eval/datasets/ # MCP tool logs, written to a relative path by whatever is started from the repo root /logs/ + +# editor swap files +*.swp diff --git a/packages/gooddata-eval/AGENTS.md b/packages/gooddata-eval/AGENTS.md index 8dbd76fda..542e3c33a 100644 --- a/packages/gooddata-eval/AGENTS.md +++ b/packages/gooddata-eval/AGENTS.md @@ -9,7 +9,7 @@ experiment. The newest and most actively developed package in the repo. ## Owns -- The `gd-eval` CLI (`gd-eval run`, `gd-eval models`) +- The `gd-eval` CLI (`generate`, `run`, `report`, `models`) - Dataset loading and the evaluation run loop - Per-capability evaluators and their scoring - Result reporting, and pushing experiments, scores and trace links to Langfuse @@ -27,7 +27,7 @@ experiment. The newest and most actively developed package in the repo. | `core/agentic/` | multi-turn agentic evaluation per capability, **plus** all Langfuse trace polling and linking (`_langfuse.py`, `_trace_linker.py`) | | `core/chat/` | SSE client for the agent's streaming chat endpoint | | `core/summary/` | HTTP client for the dedicated dashboard-summary endpoint — a single-shot chat backend, not reporting | -| `core/dataset/` | dataset format and loading | +| `core/dataset/` | dataset format, loading, and `from_insights.py` — dataset generation from a workspace's real insights | | `core/evaluators/` | single-shot evaluators and their registry | | `core/langfuse/` | the whole Langfuse v4 client: `_env` (base URL + credentials), `otlp` (OTLP/JSON encoding), `experiment` (root-span construction, score targets), `observations` (trace reads), `client` (httpx calls), `sink` (single-shot results as experiments) | | `core/reporting/` | console and JSON output rendering | @@ -61,6 +61,107 @@ its own shape. `test_kind` on the item is what labels the result, not the evalua which is why `knowledge_question` can reuse `GeneralQuestionEvaluator` verbatim. `dashboard_summary` items additionally need `summary_input`. +## Running the pipeline + +Four subcommands, in the order you use them. Everything runs through `uv`; never a bare +`python`. There is no build step -- `uv run` syncs the environment from `uv.lock` on first +use, so a fresh clone needs nothing but: + +```bash +uv run --package gooddata-eval gd-eval --help +``` + +`openai` is an optional extra (`llm-judge`) so the published package stays installable +without it, but the `dev` dependency group pulls it in, which is why a plain `uv run` here +has the phrasing step and the LLM judge. Installing `gooddata-eval` from PyPI does not -- +there the extra is explicit, and every `openai` import site is guarded or deferred. + +Connection is the same for every subcommand that talks to the platform: `--host` + +`--token`, or `GOODDATA_TOKEN` in the environment, or `--profile ` reading +`~/.gooddata/profiles.yaml`. Precedence is flags > env > profile. + +### 1. `generate` — build a dataset from a workspace + +Reverse-engineers `visualization` items out of the charts a workspace already has, so the +expected output is copied from a live object rather than invented. Needs `OPENAI_API_KEY` +for the phrasing step, or `--no-phrase` to emit mechanical `Show ` questions. + +```bash +uv run --package gooddata-eval gd-eval generate \ + --host "$GOODDATA_HOST" --workspace "$WORKSPACE_ID" \ + --dataset-name ecommerce --out ./datasets/ecommerce \ + --snapshot-out /tmp/ws.json \ + --phrase-model gpt-4o --enrich-ranked 5 --skip-ambiguous +``` + +Iterate offline instead of re-fetching: `--snapshot-out` writes everything the generator +read as one JSON file, and `--snapshot-in` replays it with no host, token or network. Add +`--dry-run` to print the shape counts and each spec's brief without writing anything — +the fastest way to see what a workspace yields. + +Quality gates fail the command (exit 1) below `--min-questions` (15), `--min-shapes` (3) +or `--min-filtered` (1). Lower them for a smoke test; do not lower them to ship a dataset. +`--langfuse-out` additionally writes a Langfuse-importable file, and `--id-prefix` rewrites +ids on that export only, because Langfuse item ids are unique per project. + +Insights the AAC spec cannot express without guessing are skipped with a printed reason +(`SKIP <id>: derived measure (previousPeriodMeasure)`). Read those — they are the +generator telling you what it refused to invent, not noise. + +### 2. `run` — evaluate + +```bash +uv run --package gooddata-eval gd-eval run \ + --host "$GOODDATA_HOST" --workspace "$WORKSPACE_ID" \ + --dataset ./datasets/ecommerce --kind visualization \ + --model gpt-5.2 --model ProviderName/gpt-4o \ + --runs 3 --gate power --concurrency 4 \ + --json ./results/run.json --html ./results/run.html +``` + +`--dataset` reads a local folder; `--langfuse-dataset` pulls one by name instead. `--kind` +only supplies a default for items that do not carry their own `test_kind`. Repeat +`--model` to compare models in one run. `--runs` with `--gate power` measures stability +(every run must pass) rather than pass@K. `--langfuse` pushes the run as a scored +experiment, needing `LANGFUSE_HOST`, `LANGFUSE_PUBLIC_KEY` and `LANGFUSE_SECRET_KEY`. + +`--concurrency` is capped for you where it matters: kinds that create workspace objects +run one at a time regardless, see the parallel-safety gotcha below. + +### 3. `report` — compare runs + +```bash +uv run --package gooddata-eval gd-eval report \ + ./results/*.json -o ./results/comparison.html --title "luna vs 4o" --redact +``` + +Several JSON reports become side-by-side columns keyed by file name. `--redact` is the +customer-safe form: conversation ids, response ids and raw reasoning dropped, model names +replaced with "Model A", "Model B". + +### 4. `models` — what the org has configured + +```bash +uv run --package gooddata-eval gd-eval models --host "$GOODDATA_HOST" +``` + +Run this before guessing a `--model` string. + +### Environment + +| Variable | Used by | +|---|---| +| `GOODDATA_TOKEN` | every platform-facing subcommand | +| `OPENAI_API_KEY` | `generate` phrasing, and the LLM-as-judge evaluators | +| `GD_EVAL_JUDGE_MODEL` | judge model, same as `--judge-model` | +| `GD_EVAL_AGENT_ID` | which agent to drive, same as `--agent-id` | +| `LANGFUSE_HOST`, `LANGFUSE_PUBLIC_KEY`, `LANGFUSE_SECRET_KEY` | `--langfuse`, `--langfuse-dataset` | +| `GOODDATA_EVAL_CHAT_*` | SSE retry, backoff and timeout knobs | +| `GD_EVAL_TIMERS` | same as `--timers` | + +A gitignored `.env` at the repo root is the normal place for these; load it with +`set -a && . ./.env && set +a` before the command. + ## Gotchas **Adding an evaluator is a registry change, not a naming convention.** Single-shot kinds go @@ -89,6 +190,23 @@ ingestion has no pass/fail signal and inflates or misattributes per-item latency (`run_trace_link_inline` is the synchronous alternative). Do not "fix" a slow item by making trace scoring synchronous again. +**A generated item's `expected_output` is copied, never invented — keep it that way.** +`core/dataset/from_insights.py` converts each insight with the platform's own +`declarative_visualization_to_aac()` (from `gooddata-code-convertors`, via `gooddata-sdk`), +so the mapping is not ours to get wrong. What is ours is deciding what the evaluator cannot +yet score — derived measures and measure-level filters convert fine and then compare wrong, +so they are skipped with a printed reason — and stripping the no-op filters AD saves for an +"All" selection, which would otherwise let a question claim a filter its chart lacks. Teach +the comparator about a construct and the matching skip can go; do not make one convert by +hand. Chart type names are the convertor's, which are also the agent's — do not rename them. One +granularity is patched in `CONVERTOR_GRANULARITY_FIXES`: `week_us` → `WEEK_US` is a convertor +bug, the platform enum is `WEEK`. + +**The snapshot is a plain-JSON contract.** `--snapshot-in`/`--snapshot-out` is what makes +the generator testable offline and iterable without re-fetching, and it is why the +generator reads the declarative analytics model rather than `sdk.visualizations`. Anything +that changes the fetch shape invalidates every saved snapshot. + **Scoring weights do not sum to 1.** `quality_score` is the fraction of boolean-valued keys in `best_detail` that are true, falling back to `pass_at_k` when there are none (text evaluators). `value_score` is `0.6 * quality + 0.2 * speed` — the 0.8 total is what the diff --git a/packages/gooddata-eval/pyproject.toml b/packages/gooddata-eval/pyproject.toml index ce885fd93..d91c95c1c 100644 --- a/packages/gooddata-eval/pyproject.toml +++ b/packages/gooddata-eval/pyproject.toml @@ -41,6 +41,9 @@ Source = "https://github.com/gooddata/gooddata-python-sdk" [dependency-groups] dev = [ "pytest>=8.3.5", + # The extra itself, so a plain `uv run` has the phrasing step and the LLM judge. + # It stays optional for anyone installing the published package. + "gooddata-eval[llm-judge]", ] test = [ "pytest~=9.1.1", diff --git a/packages/gooddata-eval/src/gooddata_eval/cli/main.py b/packages/gooddata-eval/src/gooddata_eval/cli/main.py index 3146127d1..c9344cc8b 100644 --- a/packages/gooddata-eval/src/gooddata_eval/cli/main.py +++ b/packages/gooddata-eval/src/gooddata_eval/cli/main.py @@ -26,6 +26,7 @@ normalize_gate, ) from gooddata_eval.core.connection import ConnectionError_, resolve_connection +from gooddata_eval.core.dataset.from_insights import generate as generate_from_insights from gooddata_eval.core.dataset.local import load_local_dataset from gooddata_eval.core.evaluators import supported_test_kinds from gooddata_eval.core.langfuse.sink import LangfuseSink @@ -208,6 +209,80 @@ def _build_parser() -> argparse.ArgumentParser: "model names with 'Model A', 'Model B', ...", ) + gen = sub.add_parser( + "generate", + help="Generate a visualization dataset by reverse-engineering a workspace's insights.", + ) + gen.add_argument("--host", help="GoodData host URL.") + gen.add_argument("--token", help="API token (or set GOODDATA_TOKEN).") + gen.add_argument("--profile", help="Profile name in ~/.gooddata/profiles.yaml.") + gen.add_argument("--workspace", help="Workspace id to read insights from.") + gen.add_argument( + "--dataset-name", dest="dataset_name", required=True, help="`dataset_name` written into every item." + ) + gen.add_argument("--out", help="Output folder for the dataset JSON files (default: ./<dataset-name>).") + gen.add_argument( + "--dashboard", + action="append", + default=[], + help="Restrict to insights placed on this dashboard (repeatable). Default: the whole workspace.", + ) + gen.add_argument( + "--snapshot-in", dest="snapshot_in", help="Replay a saved model snapshot instead of calling the API." + ) + gen.add_argument("--snapshot-out", dest="snapshot_out", help="Save the fetched model snapshot for later replay.") + gen.add_argument("--langfuse-out", dest="langfuse_out", help="Also write a Langfuse-importable dataset JSON here.") + gen.add_argument( + "--id-prefix", + dest="id_prefix", + default="", + help="Prefix every exported Langfuse item id. Langfuse ids are unique per PROJECT, so " + "carrying an item into a second dataset under its original id is a 409.", + ) + gen.add_argument( + "--no-phrase", dest="no_phrase", action="store_true", help="Skip the LLM step; emit mechanical questions." + ) + gen.add_argument( + "--phrase-model", dest="phrase_model", default="gpt-4o", help="OpenAI model for the phrasing step." + ) + gen.add_argument( + "--no-viz-type", dest="no_viz_type", action="store_true", help="Always blank the expected chart type." + ) + gen.add_argument( + "--min-questions", dest="min_questions", type=int, default=15, help="Fail below this many questions." + ) + gen.add_argument( + "--min-shapes", dest="min_shapes", type=int, default=3, help="Fail below this many distinct question shapes." + ) + gen.add_argument( + "--min-filtered", + dest="min_filtered", + type=int, + default=1, + help="Fail below this many questions carrying a filter.", + ) + gen.add_argument( + "--enrich-ranked", + dest="enrich_ranked", + type=int, + default=0, + metavar="N", + help="Additionally derive up to N ranked questions. Best-grounded first: insights whose " + "own title promised a ranking their definition never implemented ('Top Returned Reasons' " + "saved with no sort) are implemented as the title asks, then ranking filters this " + "generator adds to a plain breakdown, then sort-only variants. Use when the workspace has " + "no ranked insights of its own. Derived items carry `derived_from` and `derived_basis`. " + "Default: 0 (off).", + ) + gen.add_argument( + "--skip-ambiguous", + dest="skip_ambiguous", + action="store_true", + help="Drop items whose metric or dimension name matches more than one object in the model " + "(loop has six labels titled 'Product Title'). Such a question cannot say which object it " + "means, so a defensible answer still scores zero. Reported either way.", + ) + gen.add_argument("--dry-run", dest="dry_run", action="store_true", help="Report only; write nothing.") models_cmd = sub.add_parser("models", help="List LLM providers and models configured in the org.") models_cmd.add_argument("--host", help="GoodData host URL.") models_cmd.add_argument("--token", help="API token (or set GOODDATA_TOKEN).") @@ -612,6 +687,23 @@ def _report(args: argparse.Namespace) -> int: return _EXIT_OK +def _generate(args: argparse.Namespace) -> int: + """`gd-eval generate` -- reverse-engineer a dataset from a workspace's insights.""" + if not args.snapshot_in and not args.workspace: + print("error: generate needs --workspace, or --snapshot-in to replay a saved model.", file=sys.stderr) + return _EXIT_OPERATIONAL_ERROR + if args.out is None: + args.out = args.dataset_name + + def sdk_factory(): + from gooddata_sdk import GoodDataSdk # noqa: PLC0415 + + host, token = resolve_connection(host=args.host, token=args.token, profile=args.profile) + return GoodDataSdk.create(host, token) + + return generate_from_insights(args, sdk_factory) + + def main(argv: list[str] | None = None) -> int: args = parse_args(argv if argv is not None else sys.argv[1:]) _apply_timer_flag(getattr(args, "timers", False)) @@ -625,6 +717,9 @@ def main(argv: list[str] | None = None) -> int: if args.command == "report": return _report(args) + if args.command == "generate": + return _generate(args) + host, token = resolve_connection(host=args.host, token=args.token, profile=args.profile) if args.command == "models": return _list_models(host, token, getattr(args, "workspace", None)) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/dataset/from_insights.py b/packages/gooddata-eval/src/gooddata_eval/core/dataset/from_insights.py new file mode 100644 index 000000000..59663b8ec --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/dataset/from_insights.py @@ -0,0 +1,1286 @@ +# (C) 2026 GoodData Corporation +"""Reverse-generate `visualization` dataset items from a workspace's real insights. + +The inverse of hand-authoring: instead of writing a question and then guessing the +expected metric/dimension/filter, this reads the *existing* visualizations a customer +already built (via the read-only declarative analytics model), translates each one's +buckets/filters into an `expected_output.visualization` AAC spec, and only then asks an +LLM to write the analyst question a user would ask to get that chart back. + +`expected_output` is therefore copied out of a real object, never invented -- which is +what satisfies "answerable with the current data model" and "expected answers reference +metrics that exist in the LDM" by construction. The LLM only writes English. + +`--enrich-ranked` additionally *derives* ranked items from those real specs (see +`pick_derived`), which is a weaker guarantee than copying but a much stronger one than +synthesizing from the LDM: adding a limit or a sort to a definition that already +executes cannot make it unanswerable. Derived items are marked with `derived_from`. + +Driven by `gd-eval generate`; the functions here are importable for programmatic use. +""" + +import hashlib +import json +import os +import re +import sys +import unicodedata +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from gooddata_sdk.catalog.workspace.aac import declarative_visualization_to_aac + +from gooddata_eval.core.granularity import ( + GRANULARITIES, + GRANULARITY_BY_ID, + _camel, + canonical_date_uri, + granularity_of, +) +from gooddata_eval.core.models import CreatedVisualization, DatasetItem +from gooddata_eval.core.scoring import resolve_alias_to_uri, uri_to_display_name + +# `GDC.time.week_us` converts to `WEEK_US`, which is not a platform granularity -- the +# SDK's own `_GRANULARITY_CONVERSION` maps it to `WEEK`. Override until the convertor is +# fixed; an expected_output carrying `WEEK_US` can never match what the agent builds. +# TODO(AIS-48): drop this once gooddata-code-convertors maps week_us to WEEK. +CONVERTOR_GRANULARITY_FIXES = {"WEEK_US": "WEEK"} + +# Words that make a question *name* a chart form. `expected_output.type` is only set +# when the question actually constrains the form -- an insight's `visualizationUrl` +# records what a human clicked, not what the question asks for, so copying it in +# unconditionally scores the agent on a choice the question never made. +TYPE_WORDS = { + "area_chart": ("area chart",), + "bar_chart": ("bar chart", "bar graph"), + "bubble_chart": ("bubble chart",), + "bullet_chart": ("bullet chart",), + "column_chart": ("column chart",), + "combo_chart": ("combo chart", "combination chart"), + "donut_chart": ("donut chart", "doughnut chart"), + "funnel_chart": ("funnel chart",), + "geo_chart": ("map", "pushpin"), + "geo_area_chart": ("map", "choropleth", "area map"), + "headline_chart": ("headline", "single number", "kpi", "big number"), + "heatmap_chart": ("heatmap", "heat map"), + "line_chart": ("line chart", "line graph"), + "pie_chart": ("pie chart",), + "pyramid_chart": ("pyramid chart",), + "scatter_chart": ("scatter plot", "scatterplot"), + "table": ("table",), + "treemap_chart": ("treemap", "tree map"), + "waterfall_chart": ("waterfall chart",), +} + +SHAPES = ( + "single_metric_callout", + "breakdown_by_dimension", + "filtered_view", + "time_series", + "comparison", +) + +# A question may only use ranking language if the spec actually ranks, and filter +# language if the spec actually filters. Otherwise the expected output contradicts the +# question and the item punishes the agent for reading it correctly. +RANK_WORDS = re.compile( + r"\b(top|bottom|most|least|fewest|highest|lowest|largest|smallest|greatest|best|worst" + r"|ranked|rank|limit it to)\b", + re.I, +) +FILTER_WORDS = re.compile( + r"\b(only|excluding|exclude|filtered|restricted to|limited to|just the" + r"|last (?:year|quarter|month|week)|this (?:year|quarter|month|week)" + r"|year to date|ytd|in \d{4})\b", + re.I, +) + +# Which end of the ranking a title names. A title using both ("Top and Bottom Products") +# names no single direction and is left alone. +TOP_WORDS = re.compile(r"\b(top|most|highest|largest|greatest|best)\b", re.I) +BOTTOM_WORDS = re.compile(r"\b(bottom|least|fewest|lowest|smallest|worst)\b", re.I) +# "Top 10 Products" states the N outright; most titles do not. +TITLE_N = re.compile(r"\b(?:top|bottom|first|last)\s+(\d{1,3})\b", re.I) + +# A slot the writer failed to fill: it copied the instruction instead of a real name. +PLACEHOLDER = re.compile(r"\b(breakdown|split|filter)\s+dimension\b|[{}<>]") +# The text a question breaks down by. `(?<!...)` keeps a bare "by" from matching the +# ranking phrasing ("top 5 by Spend"), which is legitimate without any dimension. +BY_CLAUSE = re.compile( + r"\b(?:broken down by|split by|grouped by|(?<!ranked )(?<!sorted )(?<!\d )by)\s+(.+?)(?:\?|$|,| for | with | in | over )", + re.I, +) +# Phrasings that deliberately assert the absence of a breakdown. +NO_BREAKDOWN = re.compile( + r"\b(?:no|without|not)\b[^?.]{0,40}?" + r"\b(?:breakdown|break(?:ing)? (?:it|them) down|split|splits|grouping|dimensions?)\b", + re.I, +) + +PHRASE_SYSTEM = ( + "You write the question a business analyst would type into a BI chat assistant to get " + "a specific chart back. You are given that chart's exact definition. Reply with the " + "question only -- no quotes, no preamble, no explanation." +) + +TEST_KIND = "visualization" + +_MAX_SLUG_LEN = 50 +_HASH_LEN = 4 + + +class Unsupported(Exception): + """This insight cannot be expressed as an AAC spec without guessing.""" + + +class PromisedRanking(Unsupported): + """The title names a ranking the definition never implemented. + + Still unusable as a copied fixture -- but unlike every other `Unsupported`, the + missing piece is written down: a human titled the chart "Products With the Highest + Return Rate" and then saved it without the sort. `--enrich-ranked` implements what + the title says instead of throwing the insight away, so the exception carries the + converted spec and the intent parsed out of the title. + """ + + def __init__(self, message: str, spec: dict, direction: str, n: int | None): + super().__init__(message) + self.spec, self.direction, self.n = spec, direction, n + + +def _slugify(text: str) -> str: + ascii_text = unicodedata.normalize("NFKD", text).encode("ascii", "ignore").decode("ascii").lower() + slug = re.sub(r"[^a-z0-9]+", "-", ascii_text).strip("-") + if len(slug) <= _MAX_SLUG_LEN: + return slug + truncated = slug[:_MAX_SLUG_LEN] + if "-" in truncated: + truncated = truncated.rsplit("-", 1)[0] + return truncated.strip("-") + + +def mint_id(question: str, existing_ids: set[str]) -> str: + """Stable slug id for a question, with a content hash appended on collision.""" + candidate = _slugify(question) or "question" + if candidate not in existing_ids: + return candidate + return f"{candidate}-{hashlib.sha256(question.encode()).hexdigest()[:_HASH_LEN]}" + + +def list_ids(directory: Path) -> set[str]: + """Ids already present in `directory` (recursively), so new ones don't collide.""" + ids: set[str] = set() + if not Path(directory).is_dir(): + return ids + for path in Path(directory).glob("**/*.json"): + try: + raw = json.loads(path.read_text()) + except (OSError, json.JSONDecodeError): + continue + if isinstance(raw, dict) and isinstance(raw.get("id"), str): + ids.add(raw["id"]) + return ids + + +def _alias(prefix: str, uri: str, taken: set) -> str: + stem = re.sub(r"[^a-z0-9]+", "_", uri.split("/", 1)[-1].lower()).strip("_") + base = prefix + re.sub(rf"^{prefix}", "", stem)[:40] + alias, n = base, 2 + while alias in taken: + alias, n = f"{base}_{n}", n + 1 + taken.add(alias) + return alias + + +def _reject_unscorable(spec: dict) -> None: + """Skip an insight whose AAC form this evaluator cannot compare. + + Not a judgement on the insight -- the platform emitted it and it renders. These + convert fine and then score wrong: a derived measure (previous period, arithmetic) + is a field `using` another alias rather than an object, a measure-level filter lands + on the field where `check_filters` never looks, and a repeater lists labels among its + metrics. Teach the comparator about one and its line here can go. + """ + fields = spec["query"]["fields"] + for item in spec["metrics"]: + field = fields[item["field"] if isinstance(item, dict) else item] + aggregated = False + if isinstance(field, dict): + if field.get("type"): + raise Unsupported(f"derived measure ({field['type']})") + if field.get("filter_by"): + raise Unsupported("measure-level filters") + aggregated = bool(field.get("aggregation")) + field = field.get("using") + if not isinstance(field, str): + raise Unsupported("derived measure (arithmetic)") + # `COUNT(attribute/x)` is a metric the agent builds and the scorer resolves; a bare + # label with no aggregation is a repeater column, not something to compare. + if not field.startswith(("metric/", "fact/")) and not aggregated: + raise Unsupported(f"a label among the metrics ({field})") + + +def _to_aac(viz: dict) -> dict: + """Declarative visualization -> AAC spec, via the SDK's convertor.""" + try: + converted = declarative_visualization_to_aac(viz) + except Exception as exc: # ConversionError, and whatever else the WASM layer raises + raise Unsupported(f"convertor rejected the definition: {exc}") from exc + spec = converted.get("json") + if spec is None: + url = (viz.get("content") or {}).get("visualizationUrl") + raise Unsupported(f"convertor produced no spec for visualizationUrl '{url}'") + return spec + + +def _normalize_filters(raw: dict) -> dict: + """Convertor `filter_by` -> the shape the evaluator compares, no-ops dropped. + + A no-op is AD's "All" selection -- an attribute filter with no `state`, or a date + filter with no window. Both restrict nothing, and the convertor keeps them, which + would let a question claim a filter its chart does not have. + """ + out = {} + for entry in raw.values(): + if entry.get("type") == "attribute_filter" and not any((entry.get("state") or {}).values()): + continue + if entry.get("type") == "date_filter": + if entry.get("from") is None and entry.get("to") is None: + continue + using = entry.get("using") + granularity = entry.get("granularity") + entry = { + **entry, + # The convertor emits a bare dataset id; every other uri in the spec, and + # everything the agent emits, carries its type prefix. + "using": using if str(using).startswith("dataset/") else f"dataset/{using}", + } + if granularity is not None: + entry["granularity"] = CONVERTOR_GRANULARITY_FIXES.get(granularity, granularity) + out[f"f{len(out)}"] = entry + return out + + +def _rename_aliases(spec: dict) -> None: + """Give every field a readable alias, in place. + + The convertor keeps AD's local identifiers, which in a real workspace are uuids + (`d319bcb2d8c04442a684e3b3cd063381`). Nothing scores on the alias, but every skip + reason and dry-run brief is read by a human. + """ + fields = spec["query"]["fields"] + taken: set = set() + renamed = {} + for alias, field in fields.items(): + uri = field["using"] if isinstance(field, dict) else field + is_metric = uri.startswith(("metric/", "fact/")) or (isinstance(field, dict) and field.get("aggregation")) + renamed[alias] = _alias("m_" if is_metric else "d_", uri, taken) + spec["query"]["fields"] = {renamed[a]: f for a, f in fields.items()} + for bucket in ("metrics", "view_by", "segment_by", "rows", "columns"): + # A bucket item is an alias, or `{"field": alias, ...}` carrying `format`, `axis`, + # `totals` or `display_as` -- presentation, which nothing scores. Keep the alias. + spec[bucket] = [renamed.get(a["field"] if isinstance(a, dict) else a, a) for a in spec[bucket]] + for entry in spec["query"]["filter_by"].values(): + for key in ("using", "attribute"): + if isinstance(entry.get(key), str) and entry[key] in renamed: + entry[key] = renamed[entry[key]] + for entry in spec["query"]["sort_by"]: + if entry.get("type") == "attribute_sort": + entry["by"] = renamed.get(entry.get("by"), entry.get("by")) + else: + entry["metrics"] = [renamed.get(m, m) for m in entry.get("metrics") or []] + + +def convert(viz: dict, date_instance_ids: set, display_names: dict | None = None) -> dict: + """Declarative visualization object -> AAC `visualization` spec. Raises `Unsupported`.""" + if any(b.get("localIdentifier") == "location" for b in (viz.get("content") or {}).get("buckets") or []): + # The one thing read off the declarative object. A map's location is a rendering + # label (`city_pushpin_latitude`) that AAC files under `view_by` like any other + # dimension, and a question built from it reads "broken down by City pushpin + # latitude". The bucket name is the only exact marker. + raise Unsupported("unknown bucket 'location'") + aac = _to_aac(viz) + query = aac.get("query") or {} + spec: dict[str, Any] = { + "id": re.sub(r"[^a-z0-9_]+", "_", (viz.get("id") or "viz").lower())[:30], + "type": aac["type"], + "title": viz.get("title") or viz.get("id"), + "query": { + "fields": query.get("fields") or {}, + "filter_by": _normalize_filters(query.get("filter_by") or {}), + "sort_by": query.get("sort_by") or [], + }, + "metrics": aac.get("metrics") or [], + "view_by": aac.get("view_by") or [], + "segment_by": aac.get("segment_by") or [], + "columns": aac.get("columns") or [], + "rows": aac.get("rows") or [], + } + if not spec["metrics"]: + raise Unsupported("no measures") + _reject_unscorable(spec) + _rename_aliases(spec) + _reject_degenerate(spec) + spec["_shape"] = classify(spec, date_instance_ids) + return spec + + +def sorts_of(spec: dict) -> list: + """The spec's sort entries.""" + return spec["query"].get("sort_by") or [] + + +def _sort_field(sort: dict) -> str: + """The alias a sort entry orders on, whichever key its type puts it under.""" + return sort["by"] if sort.get("type") == "attribute_sort" else sort["metrics"][0] + + +def ranks(spec: dict) -> bool: + return bool(sorts_of(spec)) or any(f.get("type") == "ranking_filter" for f in spec["query"]["filter_by"].values()) + + +def filters(spec: dict) -> bool: + return any(f.get("type") in ("date_filter", "attribute_filter") for f in spec["query"]["filter_by"].values()) + + +def _reject_degenerate(spec: dict) -> None: + """Skip insights whose title promises behaviour their definition doesn't implement. + + A chart called "Products by Most Items Sold" with `sorts: []` and `filters: []` is a + mis-specified object, not a fixture: any faithful question about its definition + contradicts its name, and any question true to its name contradicts its + `expected_output`. Excluding it is the only honest option. + """ + title = spec["title"] or "" + if RANK_WORDS.search(title) and not ranks(spec): + message = f"title '{title}' promises a ranking the definition has no sort/ranking filter for" + direction = title_direction(title) + if direction is None: + raise Unsupported(message) + found = TITLE_N.search(title) + raise PromisedRanking(message, spec, direction, int(found.group(1)) if found else None) + if FILTER_WORDS.search(title) and not filters(spec): + raise Unsupported(f"title '{title}' promises a filter the definition has no date/attribute filter for") + + +def title_direction(title: str) -> str | None: + """Which end of the ranking `title` names, or None if it names both or neither.""" + top, bottom = bool(TOP_WORDS.search(title)), bool(BOTTOM_WORDS.search(title)) + if top == bottom: + return None + return "top" if top else "bottom" + + +def classify(spec: dict, date_instance_ids: set) -> str: + """Question shape, for the coverage report. + + ponytail: first-match-wins heuristic -- a top-5 breakdown counts as `filtered_view`, + not `breakdown_by_dimension`. Good enough to prove the corpus isn't all one type; + replace with per-insight labels if the mix ever needs to be exact. + """ + dims = spec["view_by"] + spec["segment_by"] + spec["columns"] + spec["rows"] + fields = spec["query"]["fields"] + filter_types = {f.get("type") for f in spec["query"]["filter_by"].values()} + if filter_types & {"attribute_filter", "ranking_filter"}: + return "filtered_view" + if not dims: + return "single_metric_callout" + if any(resolve_alias_to_uri(a, fields).split("/", 1)[-1].split(".", 1)[0] in date_instance_ids for a in dims): + return "time_series" + if spec["segment_by"] or len(spec["metrics"]) > 1: + return "comparison" + return "breakdown_by_dimension" + + +# --- derived ranking variants ------------------------------------------------- + +# Analysts sort in Analytical Designer and save the chart without persisting the sort, +# so ranking coverage is near zero on most customer models: the eval can punish a +# spurious ranking but never confirm the agent builds a required one. +# +# A ranked variant is *derived*, not synthesized. Adding a LIMIT to a spec that already +# executes cannot make it unanswerable -- the LDM is untouched -- and "the top 3 X by Y" +# has exactly one correct spec, so a derived item is less ambiguous to grade than the +# insight it came from. What it loses is provenance: no human ever asked for it. +# +# Only a ranking filter is derived. A derived sort would be a weaker item than a derived +# ranking on the same base: "the top 3 X by Y" has one correct spec, while "X sorted by +# Y" leaves the direction to the reader, and a base that is worth ranking is worth +# ranking rather than merely ordering. +DERIVED_KIND = "ranking_filter" +# Preferred N first; the first one the dimension has headroom for wins. +_DERIVED_N = (5, 3) +# A top-5 over six values ranks nothing. Require slack before calling it a ranking. +_ELEMENT_HEADROOM = 2 + + +def rankable(spec: dict, date_instance_ids: set) -> str | None: + """The one dimension alias `spec` may be ranked by, or None if it may not be. + + Deliberately narrow, because every relaxation buys ambiguity: two metrics leave + "top 3 by what?" unanswered, a second dimension leaves it unclear whether the N + applies to the pair or within a group, and a date dimension turns the result into + "top 3 months", which nobody asks. An insight that already sorts or ranks covers + this shape on its own and is left alone. + """ + if len(spec["metrics"]) != 1 or spec["segment_by"]: + return None + dims = spec["view_by"] + spec["columns"] + spec["rows"] + if len(dims) != 1 or ranks(spec): + return None + uri = resolve_alias_to_uri(dims[0], spec["query"]["fields"]) + if uri.split("/", 1)[-1].split(".", 1)[0] in date_instance_ids: + return None + return dims[0] + + +def derived_n(element_count: int | None) -> int | None: + """The N to rank by for a dimension with `element_count` values, or None if too few. + + `None` means the count is unknown (an offline snapshot taken before this step + existed); the smallest N is then the safest choice rather than a reason to skip. + """ + if element_count is None: + return min(_DERIVED_N) + for n in _DERIVED_N: + if element_count >= n + _ELEMENT_HEADROOM: + return n + return None + + +def derive( + spec: dict, + n: int, + date_instance_ids: set, + direction: str = "top", + basis: str = "shape", +) -> dict: + """A copy of `spec` keeping only the top or bottom `n` rows. + + `basis` records who wanted the ranking -- "title" when a human's own chart title + asked for it, "shape" when this generator chose to add one. + """ + if direction not in ("top", "bottom"): + raise ValueError(f"unknown ranking direction '{direction}'") + out = json.loads(json.dumps(spec)) + metric = out["metrics"][0] + key = f"f{len(out['query']['filter_by'])}" + out["query"]["filter_by"][key] = {"type": "ranking_filter", "using": metric, direction: n} + out["id"] = f"{out['id']}_{direction}{n}"[:30] + out["title"] = f"{spec['title']} ({direction} {n})" + out["_derived_from"] = spec["id"] + out["_derived_kind"] = DERIVED_KIND + out["_derived_basis"] = basis + out["_shape"] = classify(out, date_instance_ids) + return out + + +def element_counts(sdk, workspace_id: str, label_uris: set) -> dict: + """`{label uri: element count}`, counted only as far as deriving needs. + + `limit` caps the count at the largest N plus its headroom: the question is only + ever "does this dimension have more values than the N we would rank by", so paging + a 50,000-element label to completion would be wasted. + """ + ceiling = max(_DERIVED_N) + _ELEMENT_HEADROOM + + def count(uri: str) -> int | None: + try: + return len(sdk.catalog_workspace_content.get_label_elements(workspace_id, uri, limit=ceiling)) + except Exception as exc: # a label the elements API cannot serve is simply not derived from + print(f" no element count for {uri}: {exc}", file=sys.stderr) + return None + + return {uri: n for uri in sorted(label_uris) if (n := count(uri)) is not None} + + +def rescued(promised: list, date_instance_ids: set, counts: dict | None = None) -> list: + """Ranked items for insights whose titles promised a ranking they never implemented. + + Higher confidence than anything derived from shape alone: the direction comes from + the human's own words, and often the N does too. A title's explicit N is honoured + even when it differs from what the cardinality would have chosen, but a title asking + for a top 10 of seven values still yields nothing -- the words do not make the data + deeper. + """ + counts = counts or {} + out = [] + for error in promised: + spec = error.spec + alias = rankable(spec, date_instance_ids) + if alias is None: + continue + count = counts.get(resolve_alias_to_uri(alias, spec["query"]["fields"])) + if error.n is None: + n = derived_n(count) + elif count is None or count >= error.n + _ELEMENT_HEADROOM: + n = error.n + else: + n = None + if n is None: + continue + out.append(derive(spec, n, date_instance_ids, error.direction, basis="title")) + return out + + +def spec_signature(spec: dict) -> str: + """What the item actually asks for, as a comparable string. + + Two differently-titled insights can carry the same definition -- loop has both + "Products by Most Items Sold" and "Products Driving the Highest Number of Repeat + Purchases" over Units Sold by Product Title -- and deriving from each produces the + same question twice. Identity is the resolved fields, filters and sorts; titles and + ids are not part of it. + """ + fields = spec["query"]["fields"] + + def resolve(value): + if isinstance(value, str): + return resolve_alias_to_uri(value, fields) + if isinstance(value, dict): + return {k: resolve(v) for k, v in sorted(value.items())} + if isinstance(value, list): + return [resolve(v) for v in value] + return value + + return json.dumps( + { + "metrics": sorted(resolve(a) for a in spec["metrics"]), + "dims": sorted(resolve(a) for a in spec["view_by"] + spec["segment_by"] + spec["columns"] + spec["rows"]), + "filters": sorted(json.dumps(resolve(f), sort_keys=True) for f in spec["query"]["filter_by"].values()), + "sorts": [resolve(entry) for entry in sorts_of(spec)], + }, + sort_keys=True, + ) + + +def pick_derived( + specs: list, + date_instance_ids: set, + limit: int, + counts: dict | None = None, + promised: list | None = None, +) -> list: + """Up to `limit` derived variants, best-grounded first. + + Order matters because the budget is small: rescued items (a human titled the chart + "Top Returned Reasons") come before the ones this generator invented. Within the + invented ones, expanding every eligible insight would turn one popular metric into a + third of the corpus and the pass rate into a measurement of one skill, so bases are + taken round-robin by metric. + """ + counts = counts or {} + seen, out = set(), [] + + def take(spec: dict) -> bool: + """Keep `spec` unless an item already asks the same thing. Reports the budget.""" + signature = spec_signature(spec) + if signature not in seen: + seen.add(signature) + out.append(spec) + return len(out) < limit + + for spec in rescued(promised or [], date_instance_ids, counts): + if not take(spec): + break + + by_metric: dict[str, list] = {} + for spec in specs: + alias = rankable(spec, date_instance_ids) + if alias is None: + continue + fields = spec["query"]["fields"] + n = derived_n(counts.get(resolve_alias_to_uri(alias, fields))) + if n is None: + continue + by_metric.setdefault(resolve_alias_to_uri(spec["metrics"][0], fields), []).append((spec, n)) + + queues = [list(group) for group in by_metric.values()] + while queues and len(out) < limit: + for queue in queues: + if not queue: + continue + spec, n = queue.pop(0) + if not take(derive(spec, n, date_instance_ids)): + return out + queues = [q for q in queues if q] + return out + + +def derived_candidates(specs: list, date_instance_ids: set, promised: list | None = None) -> set: + """Label uris whose element count decides whether a base can be derived from.""" + uris = set() + for spec in list(specs) + [e.spec for e in promised or []]: + alias = rankable(spec, date_instance_ids) + if alias is not None: + uris.add(resolve_alias_to_uri(alias, spec["query"]["fields"])) + return uris + + +def fetch_snapshot(sdk, workspace_id: str) -> dict: + """Two read-only SDK calls, assembled into a replayable JSON snapshot.""" + analytics = sdk.catalog_workspace_content.get_declarative_analytics_model(workspace_id).analytics.to_dict( + camel_case=True + ) + ldm = sdk.catalog_workspace_content.get_declarative_ldm(workspace_id).ldm.to_dict(camel_case=True) + return { + "workspace_id": workspace_id, + "fetched_at": datetime.now(timezone.utc).isoformat(), + "analytics": analytics, + "date_instance_ids": sorted(di["id"] for di in ldm.get("dateInstances") or []), + "display_names": build_display_names(analytics, ldm), + } + + +def insight_ids_on(analytics: dict, dashboard_ids: list) -> set: + """Insight ids placed on the given dashboards, walking nested layout sections.""" + wanted = set(dashboard_ids) + found = set() + + def walk(node): + if isinstance(node, dict): + if node.get("type") == "insight": + identifier = (node.get("insight") or {}).get("identifier") or {} + if identifier.get("id"): + found.add(identifier["id"]) + for value in node.values(): + walk(value) + elif isinstance(node, list): + for value in node: + walk(value) + + for dashboard in analytics.get("analyticalDashboards") or []: + if dashboard.get("id") in wanted: + walk(dashboard.get("content") or {}) + return found + + +def build_display_names(analytics: dict, ldm: dict) -> dict: + """`{uri: human title}` for every metric, fact, label and date dataset. + + Raw ids leak into question text otherwise ("the metric metric/m_units_sold"), which + is both unreadable and a giveaway that no analyst wrote the question. + """ + names = {} + for metric in analytics.get("metrics") or []: + names[f"metric/{metric['id']}"] = metric.get("title") or metric["id"] + for dataset in ldm.get("datasets") or []: + names[f"dataset/{dataset['id']}"] = dataset.get("title") or dataset["id"] + for fact in dataset.get("facts") or []: + names[f"fact/{fact['id']}"] = fact.get("title") or fact["id"] + for attribute in dataset.get("attributes") or []: + labels = attribute.get("labels") or [] + for label in labels: + names[f"label/{label['id']}"] = label.get("title") or label["id"] + if not labels: + # An attribute with no explicit label is referenced by its own id. + names[f"label/{attribute['id']}"] = attribute.get("title") or attribute["id"] + for instance in ldm.get("dateInstances") or []: + title = instance.get("title") or instance["id"] + names[f"dataset/{instance['id']}"] = title + for granularity in instance.get("granularities") or []: + enum = GRANULARITY_BY_ID.get(granularity, granularity.upper()) + suffix = GRANULARITIES.get(enum, (granularity.title(), ""))[0] + # Registered under every spelling: the LDM declares MONTH_OF_YEAR, the API + # returns `monthOfYear`, and a lookup under one must not miss the other and + # fall back to a de-slugged id ("Order Created At - Monthofyear"). + for spelling in {_camel(enum), enum.lower(), granularity}: + names[f"label/{instance['id']}.{spelling}"] = f"{title} - {suffix}" + return names + + +def display_name(uri: str, display_names: dict) -> str: + """Human title for a URI, falling back to a de-slugged id.""" + if uri in display_names: + return display_names[uri] + for key, value in display_names.items(): # ids are case-inconsistent across LDM/AD + if key.lower() == uri.lower(): + return value + return uri_to_display_name(uri).strip().title() + + +def _filter_phrase(f: dict, fields: dict, display_names: dict) -> str: + """One filter, in words -- never raw JSON, which the writer would copy verbatim.""" + + def resolve(alias: str) -> str: + return display_name(resolve_alias_to_uri(alias, fields), display_names) + + if f["type"] == "date_filter": + on = display_name(f.get("using", ""), display_names) + if isinstance(f.get("from"), str): + return f"date range {f['from']} to {f['to']} on {on}" + granularity = (f.get("granularity") or "period").lower() + return ( + f"a relative {granularity} window from {f.get('from')} to {f.get('to')} " + f"({granularity}s back from the current one, 0 = current) on {on}" + ) + if f["type"] == "attribute_filter": + on = display_name(f.get("using", ""), display_names) + state = f.get("state") or {} + if state.get("include"): + return f"only these {on} values: {', '.join(state['include'])}" + return f"excluding these {on} values: {', '.join(state.get('exclude') or [])}" + if f["type"] == "ranking_filter": + n = f.get("top") or f.get("bottom") + end = "top" if "top" in f else "bottom" + within = f", ranked within {resolve(f['attribute'])}" if f.get("attribute") else "" + return f"{end} {n} by {resolve(f.get('using', ''))}{within}" + return json.dumps(f) + + +def describe(spec: dict, display_names: dict | None = None) -> str: + """The writer's brief: buckets, sorts and filters as display names. + + Deliberately excludes the insight title and the chart type. Titles describe intent + the definition often doesn't implement, and every contradiction between a generated + question and its `expected_output` traced back to one; the chart type is a UI choice + the question isn't meant to constrain. + """ + display_names = display_names or {} + fields = spec["query"]["fields"] + + def name(alias: str) -> str: + return display_name(resolve_alias_to_uri(alias, fields), display_names) + + def dim(aliases: list) -> list: + return _dim_briefs(spec, display_names, aliases) + + lines = [f"metric: {name(a)}" for a in spec["metrics"]] + lines += [f"broken down by: {d}" for d in dim(spec["view_by"] + spec["columns"] + spec["rows"])] + lines += [f"split by: {d}" for d in dim(spec["segment_by"])] + lines += [f"sorted by: {name(_sort_field(s))}, {s['direction'].lower()}ending" for s in sorts_of(spec)] + lines += [f"filter: {_filter_phrase(f, fields, display_names)}" for f in spec["query"]["filter_by"].values()] + return "\n".join(lines) + + +def ambiguous_titles(display_names: dict) -> set: + """Display titles that more than one object in the model carries. + + Loop has six labels all titled "Product Title". A question naming one of them cannot + say which is meant, so the expected dimension is unguessable and the item punishes a + defensible answer -- `label/product_title_at_time_of_return` instead of + `label/product_details.LINE_ITEM_TITLE` scored zero on an otherwise perfect chart. + """ + seen, dupes = {}, set() + for uri, title in display_names.items(): + # Every granularity is registered under several spellings of one label, so the + # aliases must fold together or each date dimension looks like a name collision. + canonical = canonical_date_uri(uri) + key = _normalize(title) + if key in seen and seen[key] != canonical: + dupes.add(key) + seen.setdefault(key, canonical) + return dupes + + +def ambiguous_fields(spec: dict, display_names: dict, dupes: set | None = None) -> list: + """The display names in `spec` that do not identify one object in the model.""" + dupes = ambiguous_titles(display_names) if dupes is None else dupes + names = _metric_names(spec, display_names) + _dim_names(spec, display_names) + return sorted({name for name in names if _normalize(name) in dupes}) + + +def granularity_phrase(uri: str, display_names: dict) -> str | None: + """What a date breakdown does, in words, or None if `uri` is not a date granularity. + + "Order Created At - Month" is a label name, not something a person says, and it does + not distinguish the sequential granularity from its cyclical twin. The phrase does + both: it pins the date dataset and states which reading is meant. + """ + enum = granularity_of(uri) + if enum is None: + return None + dataset = uri.split("/", 1)[-1].rpartition(".")[0] + return f"{display_name(f'dataset/{dataset}', display_names)}, {GRANULARITIES[enum][1]}" + + +def _dim_briefs(spec: dict, display_names: dict, aliases: list) -> list: + """Dimension names for the writer: date dimensions as phrases, labels verbatim.""" + fields = spec["query"]["fields"] + out = [] + for alias in aliases: + uri = resolve_alias_to_uri(alias, fields) + out.append(granularity_phrase(uri, display_names) or display_name(uri, display_names)) + return out + + +def _dim_names(spec: dict, display_names: dict) -> list: + fields = spec["query"]["fields"] + aliases = spec["view_by"] + spec["segment_by"] + spec["columns"] + spec["rows"] + return [display_name(resolve_alias_to_uri(a, fields), display_names) for a in aliases] + + +def _metric_names(spec: dict, display_names: dict) -> list: + fields = spec["query"]["fields"] + return [display_name(resolve_alias_to_uri(a, fields), display_names) for a in spec["metrics"]] + + +def _normalize(text: str) -> str: + return re.sub(r"[^a-z0-9 ]+", "", text.lower()).strip() + + +def _mentions(name: str, question: str) -> bool: + """Whether `question` names `name`, tolerating plurals and word order.""" + lowered = question.lower() + tokens = [t for t in _normalize(name).split() if len(t) >= 4] + return any(t in lowered for t in tokens) if tokens else _normalize(name) in lowered + + +def _without_field_names(question: str, spec: dict, display_names: dict) -> str: + """`question` with the spec's own field names blanked out. + + A field may be called "Most Recent Label Created At" or "Top Tier Customers". A + question naming it verbatim -- which the rules require -- is not thereby claiming a + ranking, so the claim checks have to read around the names. + """ + fields = spec["query"]["fields"] + names = [display_name(resolve_alias_to_uri(a, fields), display_names) for a in fields] + names += [display_name(f.get("using", ""), display_names) for f in spec["query"]["filter_by"].values()] + # A date label reads "Most Recent Label Created At - Month" but the question names + # the dataset and the granularity separately ("by month for Most Recent Label + # Created At"), so each side of the separator has to be maskable on its own. + names += [part for name in list(names) for part in name.split(" - ")] + for name in sorted(names, key=len, reverse=True): + if len(name.strip()) > 3: + question = re.sub(re.escape(name.strip()), " ", question, flags=re.I) + return question + + +def contradictions(question: str, spec: dict, display_names: dict | None = None) -> list: + """Ways `question` and `spec` disagree. Any hit is a hard error, never a warning.""" + display_names = display_names or {} + problems = [] + claims = _without_field_names(question, spec, display_names) + if not ranks(spec): + hit = RANK_WORDS.search(claims) + if hit: + problems.append(f"uses ranking word '{hit.group(0)}' but the chart has no sort or ranking filter") + if not filters(spec): + hit = FILTER_WORDS.search(claims) + if hit: + problems.append(f"uses filter word '{hit.group(0)}' but the chart has no date or attribute filter") + + hit = PLACEHOLDER.search(question) + if hit: + problems.append(f"leaks the un-substituted placeholder '{hit.group(0)}'") + + dims = _dim_names(spec, display_names) + clause = BY_CLAUSE.search(question) + if clause: + subject = _normalize(clause.group(1)) + echoes_metric = any(subject == _normalize(m) for m in _metric_names(spec, display_names)) + if echoes_metric and not ranks(spec): + # "Show me X by X" -- the metric echoed into its own breakdown slot. Harmless + # when the chart ranks, where "by <metric>" is how you say what it ranks on. + problems.append(f"breaks down '{clause.group(1).strip()}' by itself; it is a metric, not a dimension") + elif not dims and not NO_BREAKDOWN.search(question): + problems.append(f"asks for a breakdown by '{clause.group(1).strip()}' but view_by and segment_by are empty") + elif dims and not any(_mentions(d, question) for d in dims): + # The inverse error: the chart breaks down, the question never says so. + problems.append(f"names no dimension, but the chart breaks down by {', '.join(dims)}") + return problems + + +def _rules_for(spec: dict, display_names: dict) -> str: + dims = _dim_names(spec, display_names) + all_dims = spec["view_by"] + spec["segment_by"] + spec["columns"] + spec["rows"] + dated = [ + phrase + for alias in all_dims + if (phrase := granularity_phrase(resolve_alias_to_uri(alias, spec["query"]["fields"]), display_names)) + ] + segments = [ + display_name(resolve_alias_to_uri(a, spec["query"]["fields"]), display_names) for a in spec["segment_by"] + ] + lines = [ + "Write the question an analyst would ask to get exactly this chart. Rules:", + "- Name every metric listed above explicitly, using its name verbatim.", + ] + ranking = next((f for f in spec["query"]["filter_by"].values() if f.get("type") == "ranking_filter"), None) + # A ranking filter with no `attribute` ranks over the full dimension tuple, so the + # "top N <dimension>" shorthand is only true when there is exactly one dimension. + # With two, naming one tells the writer a scope the filter does not have -- "top 5 + # Customer State, split by City" reads as within-state, and the agent obliges. + ranked_dim = dims[0] if ranking and len(dims) == 1 and not ranking.get("attribute") else None + if ranking is not None and ranked_dim: + # The ranking phrasing already names the dimension. Asking for the breakdown as + # well produces "broken down by Carrier, showing the top 3 Carriers by Returns" -- + # the dimension twice, which no analyst writes. One instruction, not two. + n = ranking.get("top") or ranking.get("bottom") + end = "top" if "top" in ranking else "bottom" + lines.append( + f"- This chart keeps only the {end} {n} rows of {ranked_dim}. Ask for 'the {end} {n} " + f"{ranked_dim} by <metric>' and name {ranked_dim} exactly once -- do not also say " + f"'broken down by {ranked_dim}'." + ) + elif dated: + # A date breakdown is the one dimension not to quote verbatim: "broken down by + # Order Created At - Month" is a label id in prose, and it leaves the agent to + # guess between the sequential granularity and its cyclical twin. + plain = [name for name in dims if not any(name.startswith(p.split(",")[0]) for p in dated)] + lines.append( + "- Say the question is broken down by " + "; and ".join(dated) + ". Write that in " + "natural words ('by month', 'monthly', 'per month'), never as a label name like " + "'Order Created At - Month', but do keep the date dataset's name." + ) + if plain: + lines.append("- It is also broken down by " + ", ".join(plain) + ", naming each verbatim.") + elif dims: + lines.append("- Say the question is broken down by " + ", ".join(dims) + ", naming each verbatim.") + else: + lines += [ + "- This chart has NO breakdown. Ask for the metric on its own -- do not write " + "'by ...', 'broken down by ...' or 'grouped by ...' at all. Never break a metric " + "down by itself.", + # "What is the Upsell Ratio?" is answered as a definition, and "Show me Gross + # Revenue" is answered by looking the metric up -- the agent activates only its + # search skill and builds nothing. Naming the chart form is what makes a bare + # metric a charting request, so a no-breakdown question has to name it. + "- Ask for it AS A CHART, naming the form: 'as a single number', 'as a KPI' or " + "'as a headline'. Never a bare 'What is <metric>?' (answered as a definition) and " + "never a bare 'Show me <metric>' (answered by looking the metric up).", + ] + if segments: + lines.append("- Say it is split by " + ", ".join(segments) + ".") + lines += [ + "- State every filter and sort listed above in words (time period, included values, top/bottom N).", + "- Claim NOTHING that is not listed above. If no sort or ranking is listed, do not say " + "top/bottom/most/highest/lowest/ranked. If no filter is listed, do not restrict to a " + "time period or a subset of values.", + "- Write real names only. Never emit a literal word like 'breakdown dimension', 'metric' " + "or 'dimension' as a stand-in for a name.", + # A single-metric chart is the exception: without a named form the request is + # indistinguishable from a metric lookup, so there the form is the question. + *([] if not dims else ["- Do not name the chart type; the assistant should infer it."]), + "- Sound like a person asking a colleague, not like a chart title.", + "- One sentence.", + ] + return "\n".join(lines) + + +def phrase(specs: list, model: str, display_names: dict) -> list: + """Question per insight, or None where the writer kept contradicting the spec. + + One retry with the specific contradiction quoted back; a second failure drops the + item rather than shipping a question its own `expected_output` disagrees with. + """ + try: + from openai import OpenAI # noqa: PLC0415 + except ImportError as err: + raise ImportError( + "Question phrasing requires the llm-judge extra: uv add 'gooddata-eval[llm-judge]' (or pass --no-phrase)" + ) from err + if not os.environ.get("OPENAI_API_KEY"): + raise OSError("OPENAI_API_KEY environment variable is required for the phrasing step.") + + client = OpenAI() + questions = [] + for i, spec in enumerate(specs, 1): + messages: list = [ + {"role": "system", "content": PHRASE_SYSTEM}, + {"role": "user", "content": f"{describe(spec, display_names)}\n\n{_rules_for(spec, display_names)}"}, + ] + question, problems = None, [] + for _attempt in range(2): + reply = client.chat.completions.create(model=model, messages=messages) + # `content` is None when the model returns a refusal or no text at all; an + # empty candidate fails the contradiction check and takes the retry, which is + # what should happen anyway. + candidate = (reply.choices[0].message.content or "").strip().strip('"') + problems = contradictions(candidate, spec, display_names) + if not problems: + question = candidate + break + messages += [ + {"role": "assistant", "content": candidate}, + { + "role": "user", + "content": "That question " + + "; and ".join(problems) + + ". Rewrite it describing only what the definition above actually contains.", + }, + ] + if question is None: + print(f" DROP {spec['title']}: {'; '.join(problems)}", file=sys.stderr) + questions.append(question) + print(f" phrased {i}/{len(specs)}", file=sys.stderr) + return questions + + +def resolve_type(spec: dict, question: str) -> str: + """The insight's chart type, but only when the question actually names that form.""" + lowered = question.lower() + return spec["type"] if any(w in lowered for w in TYPE_WORDS.get(spec["type"], ())) else "" + + +def build(spec: dict, question: str, dataset_name: str, existing_ids: set) -> dict: + derived_from, derived_kind = spec.get("_derived_from"), spec.get("_derived_kind") + derived_basis = spec.get("_derived_basis") + spec = {k: v for k, v in spec.items() if not k.startswith("_")} + spec["type"] = resolve_type(spec, question) + question_id = mint_id(question, existing_ids) + existing_ids.add(question_id) + envelope = { + "id": question_id, + "dataset_name": dataset_name, + "test_kind": TEST_KIND, + "question": question, + "expected_output": {"visualization": spec}, + } + if derived_from: + # Provenance on the item itself: months later, "42 of these came from real charts + # and 10 were derived" has to be answerable from the dataset, not from memory -- + # and the pass rate has to be computable both ways. + envelope["derived_from"] = derived_from + envelope["derived_kind"] = derived_kind + envelope["derived_basis"] = derived_basis + return envelope + + +def langfuse_payload(envelopes: list, dataset: str, workspace_id: str, origin: str, id_prefix: str = "") -> dict: + """Langfuse-importable dataset JSON. + + `id_prefix` rewrites ids on export only: Langfuse item ids are unique per PROJECT, + so importing the same item into a second dataset under its original id is a 409. + """ + return { + "dataset": dataset, + "workspace": workspace_id, + "items": [ + { + "id": id_prefix + e["id"], + "input": {"question": e["question"]}, + "expected_output": e["expected_output"], + "metadata": { + "synthetic": True, + "test_kind": TEST_KIND, + "workspace": workspace_id, + "origin": origin, + **( + { + "derived_from": e["derived_from"], + "derived_kind": e["derived_kind"], + "derived_basis": e["derived_basis"], + } + if e.get("derived_from") + else {} + ), + }, + } + for e in envelopes + ], + } + + +def _validation_errors(envelope: dict) -> str | None: + """The envelope must load as both a DatasetItem and a scorable AAC visualization.""" + try: + DatasetItem.model_validate(envelope) + CreatedVisualization.model_validate(envelope["expected_output"]["visualization"]) + except Exception as exc: # pydantic ValidationError, or a missing key + return str(exc) + return None + + +def generate(args, sdk_factory=None) -> int: + """Run the whole generation pipeline. Returns a process exit code.""" + sdk = None + if args.snapshot_in: + snapshot = json.loads(Path(args.snapshot_in).read_text()) + else: + if sdk_factory is None: + raise ValueError("a live run needs an SDK; pass --snapshot-in to replay a saved model instead") + sdk = sdk_factory() + snapshot = fetch_snapshot(sdk, args.workspace) + + if args.snapshot_out: + Path(args.snapshot_out).write_text(json.dumps(snapshot, indent=2)) + + analytics = snapshot["analytics"] + date_instance_ids = set(snapshot.get("date_instance_ids") or []) + display_names = snapshot.get("display_names") or {} + visualizations = analytics.get("visualizationObjects") or [] + if args.dashboard: + keep = insight_ids_on(analytics, args.dashboard) + if not keep: + print(f"ERROR: no insights found on dashboard(s) {', '.join(args.dashboard)}", file=sys.stderr) + return 1 + visualizations = [v for v in visualizations if v.get("id") in keep] + + specs, skipped, promised = [], [], [] + for viz in visualizations: + if viz.get("isHidden"): + # Hidden objects are invisible to the AI assistant's catalog search, so a + # question about one is unwinnable rather than merely hard. + skipped.append((viz.get("id"), "hidden")) + continue + try: + specs.append(convert(viz, date_instance_ids, display_names)) + except PromisedRanking as exc: + # Unusable as a copy, but the title says what the definition forgot. Kept + # aside for `--enrich-ranked`; still a skip when enrichment is off. + promised.append(exc) + if not args.enrich_ranked: + skipped.append((viz.get("id"), str(exc))) + except Unsupported as exc: + skipped.append((viz.get("id"), str(exc))) + + n_base = len(specs) + derived = [] + if args.enrich_ranked: + counts = dict(snapshot.get("label_cardinality") or {}) + wanted = derived_candidates(specs, date_instance_ids, promised) + missing = wanted - set(counts) + if sdk is not None and missing: + counts.update(element_counts(sdk, snapshot["workspace_id"], missing)) + snapshot["label_cardinality"] = counts + if args.snapshot_out: + Path(args.snapshot_out).write_text(json.dumps(snapshot, indent=2)) + elif missing: + print( + f" no element counts for {len(missing)} candidate dimension(s) (replayed snapshot): " + f"deriving with the smallest N", + file=sys.stderr, + ) + derived = pick_derived(specs, date_instance_ids, args.enrich_ranked, counts, promised) + specs = specs + derived + rescued_ids = {spec["_derived_from"] for spec in derived if spec["_derived_basis"] == "title"} + # A promised ranking that did not make it was either ineligible or a duplicate of + # something already derived. Reporting the original "promises a ranking" message + # for a duplicate would send the reader looking for a problem in the wrong place. + taken = {spec_signature(spec) for spec in derived} + duplicates = { + spec["_derived_from"] + for spec in rescued(promised, date_instance_ids, counts) + if spec["_derived_from"] not in rescued_ids and spec_signature(spec) in taken + } + skipped.extend( + ( + e.spec["id"], + "definition duplicates an item already derived" if e.spec["id"] in duplicates else str(e), + ) + for e in promised + if e.spec["id"] not in rescued_ids + ) + + shapes: dict[str, list] = {} + for spec in specs: + shapes.setdefault(spec["_shape"], []).append(spec["title"]) + + dupes = ambiguous_titles(display_names) + ambiguous = [(spec, names) for spec in specs if (names := ambiguous_fields(spec, display_names, dupes))] + if args.skip_ambiguous: + drop = {id(spec) for spec, _ in ambiguous} + specs = [spec for spec in specs if id(spec) not in drop] + derived = [spec for spec in derived if id(spec) not in drop] + + n_filtered = sum(1 for s in specs if filters(s)) + n_ranked = sum(1 for s in specs if ranks(s)) + print( + f"workspace {snapshot['workspace_id']}: {len(visualizations)} insights read, " + f"{len(specs)} convertible, {len(skipped)} skipped" + ) + for shape in SHAPES: + print(f" {shape:<24} {len(shapes.get(shape, []))}") + print(f" with filters {n_filtered}") + print(f" with sort/ranking {n_ranked}") + if args.enrich_ranked: + print(f" derived from a base {len(derived)}, {n_base} from real insights") + n_rescued = sum(1 for spec in derived if spec["_derived_basis"] == "title") + print(f" of those, title-asked {n_rescued} of {len(promised)} insight(s) that promised a ranking") + if ambiguous: + verb = "dropped" if args.skip_ambiguous else "kept" + print( + f" ambiguous field names {len(ambiguous)} item(s) {verb}: a name below matches " + f"more than one object in the model, so the question cannot say which is meant" + ) + for spec, names in ambiguous[:5]: + print(f" {spec['title'][:40]:<40} {', '.join(names)}") + if len(ambiguous) > 5: + print(f" ... and {len(ambiguous) - 5} more") + if not args.skip_ambiguous: + print(" pass --skip-ambiguous to exclude them", file=sys.stderr) + for viz_id, reason in skipped: + print(f" SKIP {viz_id}: {reason}") + + failures = [] + if len(specs) < args.min_questions: + failures.append(f"only {len(specs)} questions, need >= {args.min_questions}") + if len(shapes) < args.min_shapes: + failures.append( + f"only {len(shapes)} distinct shapes ({', '.join(shapes) or 'none'}), need >= {args.min_shapes}" + ) + if n_filtered < args.min_filtered: + # With zero filtered items the eval can only punish a spurious filter, never + # confirm the agent builds a required one -- half the behaviour goes untested. + failures.append( + f"only {n_filtered} items carry a filter, need >= {args.min_filtered}; " + "point at dashboards whose insights actually filter" + ) + for failure in failures: + print(f"QUALITY GATE: {failure}", file=sys.stderr) + if failures: + print( + "Not enough real insights to build a usable dataset -- nothing is fabricated to " + "fill the gap. Point at more dashboards, or accept a smaller set with " + "--min-questions/--min-shapes.", + file=sys.stderr, + ) + + if args.dry_run: + for spec in specs: + print(f"\n[{spec['_shape']}] {spec['title']}\n{describe(spec, display_names)}") + return 1 if failures else 0 + + if args.no_phrase: + questions = [f"Show {s['title']}" for s in specs] + else: + questions = phrase(specs, args.phrase_model, display_names) + + dropped = [spec["title"] for spec, q in zip(specs, questions) if q is None] + specs, questions = zip(*[(s, q) for s, q in zip(specs, questions) if q]) if any(questions) else ([], []) + if dropped: + failures.append(f"{len(dropped)} question(s) dropped as self-contradictory: {', '.join(dropped[:5])}") + print(f"DROPPED {len(dropped)} self-contradictory question(s)", file=sys.stderr) + + out_dir = Path(args.out) + out_dir.mkdir(parents=True, exist_ok=True) + existing_ids = list_ids(out_dir) + envelopes = [build(spec, q, args.dataset_name, existing_ids) for spec, q in zip(specs, questions)] + if args.no_viz_type: + for envelope in envelopes: + envelope["expected_output"]["visualization"]["type"] = "" + + written = [] + for envelope in envelopes: + path = out_dir / f"{envelope['id']}.json" + path.write_text(json.dumps(envelope, indent=2) + "\n") + written.append(path) + print(f"wrote {len(written)} questions to {out_dir}") + + if args.langfuse_out: + origin = ( + f"AUTO-GENERATED by reverse-engineering real insights in workspace " + f"{snapshot['workspace_id']} -- expected_output copied from live " + f"visualization definitions, question text written by " + f"{'a mechanical template' if args.no_phrase else args.phrase_model}" + + ( + f"; {len(derived)} item(s) derived from a base insight by adding a ranking filter (see `derived_from`)" + if derived + else "" + ) + ) + payload = langfuse_payload(envelopes, args.dataset_name, snapshot["workspace_id"], origin, args.id_prefix) + Path(args.langfuse_out).parent.mkdir(parents=True, exist_ok=True) + Path(args.langfuse_out).write_text(json.dumps(payload, indent=2) + "\n") + print(f"wrote Langfuse dataset to {args.langfuse_out}") + + invalid = [(e["id"], err) for e in envelopes if (err := _validation_errors(e))] + if invalid: + print(f"VALIDATION FAILED for {len(invalid)} item(s):", file=sys.stderr) + for item_id, err in invalid[:5]: + print(f" {item_id}: {err}", file=sys.stderr) + return 1 + print(f"validated {len(written)}/{len(written)}") + return 1 if failures else 0 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 4cb9c6da7..f614c07af 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py @@ -13,10 +13,12 @@ ) from gooddata_eval.core.scoring import ( check_filters, + check_sorts, check_viz_type, get_dimension_uri_set, get_metric_uri_set, normalized_filters, + normalized_sorts, validate_cross_references, ) @@ -30,6 +32,7 @@ class EvaluationResult: metrics_correct: bool dimensions_correct: bool filters_correct: bool + sorts_correct: bool viz_type_hard: bool filter_date_score: bool filter_ranking_score: bool @@ -45,6 +48,8 @@ class EvaluationResult: # is otherwise undiagnosable from a finished run. expected_filters: dict[str, list[str]] actual_filters: dict[str, list[str]] + expected_sorts: list[str] + actual_sorts: list[str] @property def strict_pass(self) -> bool: @@ -54,6 +59,7 @@ def strict_pass(self) -> bool: and self.metrics_correct and self.dimensions_correct and self.filters_correct + and self.sorts_correct and self.viz_type_hard ) @@ -65,6 +71,7 @@ def strict_checks_passed_count(self) -> int: self.metrics_correct, self.dimensions_correct, self.filters_correct, + self.sorts_correct, self.viz_type_hard, ] ) @@ -95,6 +102,7 @@ def _evaluate_visualization( metrics_correct=False, dimensions_correct=False, filters_correct=False, + sorts_correct=False, viz_type_hard=False, filter_date_score=False, filter_ranking_score=False, @@ -107,6 +115,8 @@ def _evaluate_visualization( actual_dim_uris=set(), expected_filters=normalized_filters(expected), actual_filters={category: values.copy() for category, values in _NO_FILTERS.items()}, + expected_sorts=normalized_sorts(expected), + actual_sorts=[], ) cross_ref_valid, cross_ref_errors = validate_cross_references(actual) act_metric_uris = get_metric_uri_set(actual) @@ -118,6 +128,7 @@ def _evaluate_visualization( metrics_correct=act_metric_uris == exp_metric_uris, dimensions_correct=act_dim_uris == exp_dim_uris, filters_correct=filter_scores.all_ok, + sorts_correct=check_sorts(expected, actual), viz_type_hard=check_viz_type(expected, actual), filter_date_score=filter_scores.date_ok, filter_ranking_score=filter_scores.ranking_ok, @@ -128,6 +139,8 @@ def _evaluate_visualization( actual_metric_uris=act_metric_uris, expected_dim_uris=exp_dim_uris, actual_dim_uris=act_dim_uris, + expected_sorts=normalized_sorts(expected), + actual_sorts=normalized_sorts(actual), expected_filters=normalized_filters(expected), actual_filters=normalized_filters(actual), ) @@ -178,6 +191,7 @@ def evaluation_result_detail(ev: EvaluationResult) -> dict: "metrics_correct": ev.metrics_correct, "dimensions_correct": ev.dimensions_correct, "filters_correct": ev.filters_correct, + "sorts_correct": ev.sorts_correct, "filter_date_score": ev.filter_date_score, "filter_ranking_score": ev.filter_ranking_score, "filter_attribute_score": ev.filter_attribute_score, @@ -189,6 +203,8 @@ def evaluation_result_detail(ev: EvaluationResult) -> dict: "actual_dim_uris": sorted(ev.actual_dim_uris), "expected_filters": ev.expected_filters, "actual_filters": ev.actual_filters, + "expected_sorts": ev.expected_sorts, + "actual_sorts": ev.actual_sorts, } diff --git a/packages/gooddata-eval/src/gooddata_eval/core/granularity.py b/packages/gooddata-eval/src/gooddata_eval/core/granularity.py new file mode 100644 index 000000000..76a749a17 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/granularity.py @@ -0,0 +1,65 @@ +# (C) 2026 GoodData Corporation +"""Date granularities: a closed platform enum, shared by dataset generation and scoring.""" + +# Date granularities are a closed platform enum (`gooddata_api_client`), identical in +# every workspace, so unlike metric and label names they can be described once and for +# all. Each entry is (title, phrase): the title names the label, the phrase says what the +# breakdown actually does. The phrase matters because a granularity has a cyclical twin -- +# MONTH walks consecutive calendar months, MONTH_OF_YEAR stacks every January together -- +# and a question saying only "by month" does not choose between them. gpt-5.6-luna built +# `monthOfYear` where the insight used `month`, which is a defensible reading of the words. +GRANULARITIES = { + "MINUTE": ("Minute", "by minute, consecutive minutes over time (not minute-of-hour)"), + "HOUR": ("Hour", "by hour, consecutive hours over time (not hour-of-day)"), + "DAY": ("Day", "by day, one point per calendar day over time (not day-of-week)"), + "WEEK": ("Week", "by week, consecutive calendar weeks over time (not week-of-year)"), + "MONTH": ("Month", "by month, one point per calendar month over time (not month-of-year)"), + "QUARTER": ("Quarter", "by quarter, consecutive calendar quarters over time (not quarter-of-year)"), + "YEAR": ("Year", "by year, one point per calendar year"), + "MINUTE_OF_HOUR": ("Minute of Hour", "by minute of the hour (0-59), combining every hour"), + "MINUTE_OF_DAY": ("Minute of Day", "by minute of the day, combining every day"), + "HOUR_OF_DAY": ("Hour of Day", "by hour of the day (0-23), combining every day"), + "DAY_OF_WEEK": ("Day of Week", "by day of the week (Monday to Sunday), combining every week"), + "DAY_OF_MONTH": ("Day of Month", "by day of the month (1-31), combining every month"), + "DAY_OF_QUARTER": ("Day of Quarter", "by day of the quarter, combining every quarter"), + "DAY_OF_YEAR": ("Day of Year", "by day of the year (1-366), combining every year"), + "WEEK_OF_YEAR": ("Week of Year", "by week of the year (1-53), combining every year"), + "MONTH_OF_YEAR": ("Month of Year", "by month of the year (January to December), combining every year"), + "QUARTER_OF_YEAR": ("Quarter of Year", "by quarter of the year (Q1 to Q4), combining every year"), +} + + +def _camel(granularity: str) -> str: + """MONTH_OF_YEAR -> monthOfYear, the spelling the platform's label ids actually use.""" + head, *rest = granularity.lower().split("_") + return head + "".join(part.title() for part in rest) + + +# Both spellings resolve: label ids come back camelCase from the API, while the +# declarative LDM lists the granularity as the upper-snake enum member. +GRANULARITY_BY_ID = {spelling: enum for enum in GRANULARITIES for spelling in (_camel(enum), enum.lower(), enum)} + + +def granularity_of(uri: str) -> str | None: + """The granularity `uri` ends in, as an enum member, or None if it is not a date ref.""" + stem = uri.split("/", 1)[-1] + if "." not in stem: + return None + return GRANULARITY_BY_ID.get(stem.rpartition(".")[2]) + + +def canonical_date_uri(uri: str) -> str: + """One spelling for a date granularity, whichever the platform happened to return. + + A date dataset exposes each granularity as an attribute whose only label carries the + same id, so `attribute/order_created_at.month` and `label/order_created_at.month` + denote the same breakdown. Both come back from the API depending on how the agent + built the chart, and comparing the raw strings failed a chart that was correct. The + granularity itself is folded to one spelling too: the API returns `monthOfYear` where + the declarative LDM says `MONTH_OF_YEAR`, and the two are one breakdown, not two. + """ + enum = granularity_of(uri) + if enum is None: + return uri + dataset = uri.split("/", 1)[-1].rpartition(".")[0] + return f"label/{dataset}.{_camel(enum)}" diff --git a/packages/gooddata-eval/src/gooddata_eval/core/models.py b/packages/gooddata-eval/src/gooddata_eval/core/models.py index 8f696ce63..911ceb25d 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/models.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/models.py @@ -30,19 +30,31 @@ class AacBucketRef(BaseModel): class AacQuery(BaseModel): fields: dict[str, AacQueryField | str] filter_by: dict[str, dict] = Field(default_factory=dict) + # Entries are `{"type": "metric_sort", "direction", "metrics": [alias]}` or + # `{"type": "attribute_sort", "direction", "by": alias}`. Kept as raw dicts for the + # same reason as `filter_by`: the agent adds keys (`aggregation`) this does not read, + # and a typed model would reject a chart that is otherwise correct. + sort_by: list[dict] = Field(default_factory=list) @field_validator("filter_by", mode="before") @classmethod def _coerce_filter_by(cls, v: object) -> object: return v if v is not None else {} + @field_validator("sort_by", mode="before") + @classmethod + def _coerce_sort_by(cls, v: object) -> object: + return v if v is not None else [] + class CreatedVisualization(BaseModel): """Visualization in the AAC format (agent output and dataset expected output).""" model_config = ConfigDict(extra="ignore") - id: str + # Optional on purpose: nothing scores on it, and the agent sometimes omits it. A + # required field here turns a scorable chart into a parse error and an errored item. + id: str | None = None title: str | None = None type: str query: AacQuery diff --git a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py index 0d554cd85..a411b0f95 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/scoring.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/scoring.py @@ -2,8 +2,10 @@ """Visualization scoring — ported from gdc-nas tavern-e2e app/vis_assertions/metrics.py.""" import json +from collections.abc import Mapping from dataclasses import dataclass +from gooddata_eval.core.granularity import canonical_date_uri from gooddata_eval.core.models import AacBucketRef, AacQueryField, CreatedVisualization # Maps dataset chart-type names (and agent enum values) to a canonical token. @@ -28,13 +30,20 @@ def all_ok(self) -> bool: return self.date_ok and self.ranking_ok and self.attribute_ok -def _resolve_alias_to_uri(alias: str, fields: dict[str, AacQueryField | str]) -> str: - """Resolve a field alias to its `using` URI; return the alias unchanged if absent.""" +def resolve_alias_to_uri(alias: str, fields: Mapping[str, AacQueryField | str | dict]) -> str: + """Resolve a field alias to its `using` URI; return the alias unchanged if absent. + + A field may be an `AacQueryField`, a bare uri string, or the raw `{"using": uri}` + dict the AAC schema also allows -- generated specs carry the dict form before they + are ever validated into a model. + """ field = fields.get(alias) if field is None: return alias if isinstance(field, str): return field + if isinstance(field, dict): + return field["using"] # Duck-type: works even when field is from a different module's AacQueryField class return field.using @@ -43,7 +52,7 @@ def _resolve_bucket_to_uri_set(bucket: list[AacBucketRef | str], fields: dict[st uris: set[str] = set() for ref in bucket: alias = ref.field if isinstance(ref, AacBucketRef) else ref - uris.add(_resolve_alias_to_uri(alias, fields)) + uris.add(canonical_date_uri(resolve_alias_to_uri(alias, fields))) return uris @@ -81,7 +90,7 @@ def validate_cross_references(viz: CreatedVisualization) -> tuple[bool, list[str if not isinstance(using_val, str) or not using_val: errors.append(f"ranking filter '{filter_key}': using={using_val!r} — a metric/ or fact/ URI is required") else: - using_uri = _resolve_alias_to_uri(using_val, fields) + using_uri = resolve_alias_to_uri(using_val, fields) field_def = fields.get(using_val) is_adhoc_agg = isinstance(field_def, AacQueryField) and bool(field_def.aggregation) if not using_uri.startswith(("metric/", "fact/")) and not is_adhoc_agg: @@ -97,7 +106,7 @@ def validate_cross_references(viz: CreatedVisualization) -> tuple[bool, list[str f"ranking filter '{filter_key}': attribute={attr_val!r} — expected a label/ or attribute/ URI" ) continue - attr_uri = _resolve_alias_to_uri(attr_val, fields) + attr_uri = resolve_alias_to_uri(attr_val, fields) if not attr_uri.startswith(("label/", "attribute/")): errors.append( f"ranking filter '{filter_key}': attribute='{attr_val}' " @@ -106,6 +115,61 @@ def validate_cross_references(viz: CreatedVisualization) -> tuple[bool, list[str return len(errors) == 0, errors +def _normalize_sort(sort: dict, fields: Mapping[str, AacQueryField | str | dict]) -> str: + """One sort entry as a comparable string, aliases resolved to uris. + + The entry's own `type` decides which key names the fields: a `metric_sort` lists + them under `metrics`, an `attribute_sort` names one under `by`. One agent build + emits both keys on a metric sort, so reading `by` first would compare the wrong + thing on a chart that is otherwise right. + """ + if sort.get("type") == "attribute_sort": + refs = [sort.get("by")] + else: + refs = list(sort.get("metrics") or []) + if not refs and sort.get("by") is not None: + refs = [sort["by"]] + uris = [canonical_date_uri(resolve_alias_to_uri(ref, fields)) for ref in refs if isinstance(ref, str)] + return json.dumps( + { + "type": sort.get("type") or "", + "direction": (sort.get("direction") or "").upper(), + "fields": uris, + }, + sort_keys=True, + ) + + +def normalized_sorts(viz: CreatedVisualization) -> list[str]: + """`query.sort_by` in the canonical form equality is tested on. + + Order is preserved: a chart sorted by region then by revenue is not the chart + sorted by revenue then by region. + """ + return [_normalize_sort(sort, viz.query.fields) for sort in viz.query.sort_by] + + +def check_sorts(expected: CreatedVisualization, actual: CreatedVisualization) -> bool: + """Whether `actual` sorts the way `expected` does, when `expected` sorts at all. + + Deliberately not symmetric with the filter checks. An empty `sort_by` says the + fixture records no sort, not that the chart must be unsorted -- a generated item + inherits that emptiness from an insight whose author sorted in Analytical Designer + and saved without the sort sticking. A spurious filter changes which rows a reader + sees and is always wrong; a volunteered sort changes only their order, and on a time + axis ascending is the order any renderer would pick unprompted. + + So a required sort is enforced and a volunteered one is free. The same holds when + the fixture does sort: the recorded sorts must come first and in order, and a + tiebreak the agent appends after them ("state descending, then city") is free too. + The cost is that a genuinely wrong sort over an unsorted fixture goes ungraded, which + is the lesser error while `sort_by: []` cannot distinguish "unsorted" from + "unrecorded". + """ + expected_sorts = normalized_sorts(expected) + return normalized_sorts(actual)[: len(expected_sorts)] == expected_sorts + + def _normalize_date_filter(filter_dict: dict, _fields: dict) -> dict: return { "type": "date_filter", @@ -147,11 +211,11 @@ def _normalize_ranking_filter( if not isinstance(attr_val, str) or not attr_val: dim_uri = sole_dim_uri or "" else: - dim_uri = _resolve_alias_to_uri(attr_val, fields) + dim_uri = resolve_alias_to_uri(attr_val, fields) using_val = filter_dict.get("using") entry: dict = { "type": "ranking_filter", - "metric_uri": _resolve_alias_to_uri(using_val, fields) if isinstance(using_val, str) else "", + "metric_uri": resolve_alias_to_uri(using_val, fields) if isinstance(using_val, str) else "", "dim_uri": dim_uri, } if "top" in filter_dict: diff --git a/packages/gooddata-eval/tests/test_agentic_visualization.py b/packages/gooddata-eval/tests/test_agentic_visualization.py index 4787556c9..c43b4ab34 100644 --- a/packages/gooddata-eval/tests/test_agentic_visualization.py +++ b/packages/gooddata-eval/tests/test_agentic_visualization.py @@ -309,6 +309,7 @@ def test_evaluate_agentic_visualization_returns_reasoning_steps_on_pass(): "metrics_correct": True, "dimensions_correct": True, "filters_correct": True, + "sorts_correct": True, "filter_date_score": True, "filter_ranking_score": True, "filter_attribute_score": True, @@ -320,6 +321,8 @@ def test_evaluate_agentic_visualization_returns_reasoning_steps_on_pass(): "actual_dim_uris": ["label/date.quarter"], "expected_filters": {"date": [], "ranking": [], "attribute": []}, "actual_filters": {"date": [], "ranking": [], "attribute": []}, + "expected_sorts": [], + "actual_sorts": [], "latency_breakdown": [], "tool_calls": [], } @@ -361,6 +364,7 @@ def test_evaluate_agentic_visualization_attaches_reasoning_steps_to_exception_on "metrics_correct": False, "dimensions_correct": False, "filters_correct": False, + "sorts_correct": False, "filter_date_score": False, "filter_ranking_score": False, "filter_attribute_score": False, @@ -372,6 +376,8 @@ def test_evaluate_agentic_visualization_attaches_reasoning_steps_to_exception_on "actual_dim_uris": [], "expected_filters": {"date": [], "ranking": [], "attribute": []}, "actual_filters": {"date": [], "ranking": [], "attribute": []}, + "expected_sorts": [], + "actual_sorts": [], "latency_breakdown": [], "tool_calls": [], } diff --git a/packages/gooddata-eval/tests/test_from_insights.py b/packages/gooddata-eval/tests/test_from_insights.py new file mode 100644 index 000000000..6ed2fa7e1 --- /dev/null +++ b/packages/gooddata-eval/tests/test_from_insights.py @@ -0,0 +1,1664 @@ +# (C) 2026 GoodData Corporation +import json +from types import SimpleNamespace +from unittest.mock import patch + +import pytest +from gooddata_eval.core.dataset import from_insights as from_insights_mod +from gooddata_eval.core.dataset.from_insights import ( + PromisedRanking, + Unsupported, + _rules_for, + _validation_errors, + ambiguous_fields, + ambiguous_titles, + build, + build_display_names, + contradictions, + convert, + derive, + derived_candidates, + derived_n, + describe, + display_name, + element_counts, + generate, + granularity_phrase, + insight_ids_on, + langfuse_payload, + list_ids, + mint_id, + pick_derived, + rankable, + rescued, + resolve_type, + spec_signature, + title_direction, +) +from gooddata_eval.core.dataset.local import load_local_dataset +from gooddata_eval.core.models import CreatedVisualization +from gooddata_eval.core.scoring import check_filters, get_metric_uri_set, validate_cross_references + +DATE_IDS = {"process_date"} + + +def viz(url, buckets, filters=(), sorts=(), **kw): + content = {"visualizationUrl": url, "buckets": buckets, "filters": list(filters), "sorts": list(sorts)} + if "properties" in kw: + content["properties"] = kw.pop("properties") + return {"id": "v_x", "title": kw.pop("title", "X"), "content": content, **kw} + + +def measure(local_id, obj_id, obj_type="metric", **definition): + return { + "measure": { + "localIdentifier": local_id, + "definition": { + "measureDefinition": {"item": {"identifier": {"type": obj_type, "id": obj_id}}, **definition} + }, + } + } + + +def attribute(local_id, label_id): + return { + "attribute": {"localIdentifier": local_id, "displayForm": {"identifier": {"type": "label", "id": label_id}}} + } + + +def test_headline_converts_to_scorable_single_metric_spec(): + spec = convert( + viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "gross_revenue")]}]), DATE_IDS + ) + assert spec["type"] == "headline_chart" + assert spec["_shape"] == "single_metric_callout" + # The alias is arbitrary; what must survive is the URI gd-eval scores on. + parsed = CreatedVisualization(**{k: v for k, v in spec.items() if k != "_shape"}) + + assert get_metric_uri_set(parsed) == {"metric/gross_revenue"} + + +def test_breakdown_and_time_series_are_distinguished_by_date_instance(): + by_dim = convert( + viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "merchant.name")]}, + ], + ), + DATE_IDS, + ) + by_time = convert( + viz( + "local:column", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "process_date.month")]}, + ], + ), + DATE_IDS, + ) + assert by_dim["_shape"] == "breakdown_by_dimension" + assert by_time["_shape"] == "time_series" + + +def test_filters_round_trip_into_scorable_filter_by(): + spec = convert( + viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "merchant.name")]}, + ], + filters=[ + { + "absoluteDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "from": "2025-01-01", + "to": "2025-12-31", + } + }, + { + "positiveAttributeFilter": { + "displayForm": {"identifier": {"id": "region", "type": "label"}}, + "in": {"values": ["EMEA"]}, + } + }, + { + "rankingFilter": { + "measure": {"localIdentifier": "m"}, + "attributes": [{"localIdentifier": "a"}], + "operator": "TOP", + "value": 5, + } + }, + ], + ), + DATE_IDS, + ) + assert spec["_shape"] == "filtered_view" + parsed = CreatedVisualization(**{k: v for k, v in spec.items() if k != "_shape"}) + + # Ranking-filter aliases must resolve to metric/ and label/ URIs or gd-eval rejects them. + assert validate_cross_references(parsed) == (True, []) + assert check_filters(parsed, parsed).all_ok + + +def test_relative_date_granularity_is_normalized(): + spec = convert( + viz( + "local:headline", + [{"localIdentifier": "measures", "items": [measure("m", "spend")]}], + filters=[ + { + "relativeDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "granularity": "GDC.time.quarter", + "from": -1, + "to": -1, + } + } + ], + ), + DATE_IDS, + ) + assert spec["query"]["filter_by"]["f0"] == { + "type": "date_filter", + "using": "dataset/process_date", + "granularity": "QUARTER", + "from": -1, + "to": -1, + } + + +@pytest.mark.parametrize( + ("raw", "expected"), + [("GDC.time.month_in_year", "MONTH_OF_YEAR"), ("GDC.time.date", "DAY"), ("GDC.time.week_us", "WEEK")], +) +def test_cyclical_and_aliased_granularities_map_to_the_platform_enum(raw, expected): + """Stripping the prefix and upper-casing gives MONTH_IN_YEAR/DATE, which score zero.""" + spec = convert( + viz( + "local:headline", + [{"localIdentifier": "measures", "items": [measure("m", "spend")]}], + filters=[ + { + "relativeDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "granularity": raw, + "from": -1, + "to": -1, + } + } + ], + ), + DATE_IDS, + ) + assert spec["query"]["filter_by"]["f0"]["granularity"] == expected + + +def test_an_unknown_granularity_is_skipped_not_guessed(): + with pytest.raises(Unsupported, match="fortnight"): + convert( + viz( + "local:headline", + [{"localIdentifier": "measures", "items": [measure("m", "spend")]}], + filters=[ + { + "relativeDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "granularity": "GDC.time.fortnight", + "from": -1, + "to": -1, + } + } + ], + ), + DATE_IDS, + ) + + +def test_fact_measure_carries_its_aggregation(): + spec = convert( + viz( + "local:headline", + [{"localIdentifier": "measures", "items": [measure("m", "amount", "fact", aggregation="sum")]}], + ), + DATE_IDS, + ) + assert spec["query"]["fields"]["m_amount"] == {"using": "fact/amount", "aggregation": "SUM"} + + +@pytest.mark.parametrize( + "bad,reason", + [ + ( + { + "measure": { + "localIdentifier": "m", + "definition": {"arithmeticMeasure": {"operator": "SUM", "measureIdentifiers": ["x", "y"]}}, + } + }, + "derived", + ), + ( + { + "measure": { + "localIdentifier": "m", + "definition": { + "measureDefinition": { + "item": {"identifier": {"type": "metric", "id": "x"}}, + "filters": [ + { + "positiveAttributeFilter": { + "displayForm": {"identifier": {"id": "r.NAME", "type": "label"}}, + "in": {"values": ["EU"]}, + } + } + ], + } + }, + } + }, + "measure-level", + ), + ], +) +def test_underivable_measures_are_skipped_not_guessed(bad, reason): + with pytest.raises(Unsupported, match=reason): + convert(viz("local:headline", [{"localIdentifier": "measures", "items": [bad]}]), DATE_IDS) + + +def test_uri_form_attribute_filter_is_skipped_rather_than_guessed(): + with pytest.raises(Unsupported, match="not given by value"): + convert( + viz( + "local:bar", + [{"localIdentifier": "measures", "items": [measure("m", "spend")]}], + filters=[ + { + "positiveAttributeFilter": { + "displayForm": {"identifier": {"id": "region", "type": "label"}}, + "in": {"uris": ["/obj/1?id=2"]}, + } + } + ], + ), + DATE_IDS, + ) + + +def test_a_pushpin_maps_size_and_colour_to_metrics(): + """Real bucket set from a pushpin insight: no `measures` bucket, the metrics are here.""" + spec = convert( + viz( + "local:pushpin", + [ + {"localIdentifier": "size", "items": [measure("s", "orders")]}, + {"localIdentifier": "color", "items": [measure("c", "spend")]}, + {"localIdentifier": "segment", "items": [attribute("a", "country.NAME")]}, + ], + properties={"controls": {"latitude": "geo.lat", "longitude": "geo.lon"}}, + ), + DATE_IDS, + ) + assert sorted(spec["metrics"]) == ["m_orders", "m_spend"] + assert spec["segment_by"] == ["d_country_name"] + + +def test_a_bubbles_size_measure_is_kept_in_its_own_bucket(): + """AAC puts bubble size beside the dimensions (`segment_by`/`size_by`), not with x and y.""" + spec = convert( + viz( + "local:bubble", + [ + {"localIdentifier": "measures", "items": [measure("m", "price")]}, + {"localIdentifier": "secondary_measures", "items": [measure("s", "volume")]}, + {"localIdentifier": "tertiary_measures", "items": [measure("t", "profit")]}, + {"localIdentifier": "view", "items": [attribute("a", "brand.NAME")]}, + ], + ), + DATE_IDS, + ) + assert sorted(spec["metrics"]) == ["m_price", "m_volume"] + assert spec["segment_by"] == ["m_profit"] + assert spec["query"]["fields"]["m_profit"] == "metric/profit" + + +def test_a_sankeys_two_ends_are_dimensions(): + spec = convert( + viz( + "local:sankey", + [ + {"localIdentifier": "measures", "items": [measure("m", "orders")]}, + {"localIdentifier": "attribute_from", "items": [attribute("a", "customer.NAME")]}, + {"localIdentifier": "attribute_to", "items": [attribute("b", "product.NAME")]}, + ], + ), + DATE_IDS, + ) + assert spec["metrics"] == ["m_orders"] + assert sorted(spec["view_by"] + spec["segment_by"]) == ["d_customer_name", "d_product_name"] + + +def test_a_maps_location_bucket_is_skipped_not_broken_down_by(): + """`city_pushpin_latitude` is a rendering label, not a breakdown anyone asks for.""" + with pytest.raises(Unsupported, match="unknown bucket 'location'"): + convert( + viz( + "local:pushpin", + [ + {"localIdentifier": "measures", "items": [measure("m", "orders")]}, + {"localIdentifier": "location", "items": [attribute("a", "geo.city_pushpin_latitude")]}, + ], + ), + DATE_IDS, + ) + + +def test_a_count_over_an_attribute_is_a_metric_the_scorer_can_compare(): + """`COUNT(attribute/x)` is an ad-hoc metric, unlike a repeater's bare label column.""" + spec = convert( + viz( + "local:headline", + [ + { + "localIdentifier": "measures", + "items": [ + { + "measure": { + "localIdentifier": "m", + "definition": { + "measureDefinition": { + "item": {"identifier": {"type": "attribute", "id": "visit_id"}}, + "aggregation": "count", + } + }, + } + } + ], + } + ], + ), + DATE_IDS, + ) + assert spec["metrics"] == ["m_visit_id"] + assert spec["query"]["fields"]["m_visit_id"] == {"using": "attribute/visit_id", "aggregation": "COUNT"} + + +def test_treemap_is_mapped_not_dropped(): + spec = convert(viz("local:treemap", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS) + assert spec["type"] == "treemap_chart" + + +def test_unmapped_viz_url_fails_loudly(): + with pytest.raises(Unsupported, match="no spec for visualizationUrl"): + convert(viz("local:brandnew", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS) + + +def test_insight_ids_on_walks_nested_dashboard_layout(): + analytics = { + "analyticalDashboards": [ + { + "id": "d1", + "content": { + "layout": { + "sections": [ + { + "items": [ + {"widget": {"type": "insight", "insight": {"identifier": {"id": "v_a"}}}}, + { + "widget": { + "type": "IDashboardLayoutNested", + "sections": [ + { + "items": [ + { + "widget": { + "type": "insight", + "insight": {"identifier": {"id": "v_b"}}, + } + } + ] + } + ], + } + }, + ] + } + ] + } + }, + }, + { + "id": "d2", + "content": { + "layout": { + "sections": [ + {"items": [{"widget": {"type": "insight", "insight": {"identifier": {"id": "v_c"}}}}]} + ] + } + }, + }, + ] + } + assert insight_ids_on(analytics, ["d1"]) == {"v_a", "v_b"} + assert insight_ids_on(analytics, ["d1", "d2"]) == {"v_a", "v_b", "v_c"} + assert insight_ids_on(analytics, ["nope"]) == set() + + +def test_langfuse_payload_shape(): + envelope = {"id": "q1", "question": "How much?", "expected_output": {"visualization": {}}} + payload = langfuse_payload([envelope], "cust", "ws1", "origin note") + assert payload["dataset"] == "cust" and payload["workspace"] == "ws1" + item = payload["items"][0] + assert item["id"] == "q1" + assert item["input"] == {"question": "How much?"} + assert item["expected_output"] == {"visualization": {}} + assert item["metadata"]["origin"] == "origin note" + + +def test_built_envelope_is_loadable_as_a_dataset_item(tmp_path): + """The whole point: what this writes must be runnable by `gd-eval run` as-is.""" + + spec = convert( + viz( + "local:column", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "process_date.month")]}, + ], + ), + DATE_IDS, + ) + envelope = build(spec, "How did spend trend by month?", "micai_diagnose_master", set()) + assert "_shape" not in envelope["expected_output"]["visualization"] + assert _validation_errors(envelope) is None + + (tmp_path / f"{envelope['id']}.json").write_text(json.dumps(envelope, indent=2)) + items = load_local_dataset(tmp_path) + assert [i.id for i in items] == [envelope["id"]] + assert items[0].test_kind == "visualization" + assert items[0].dataset_name == "micai_diagnose_master" + + +def test_mint_id_is_stable_and_collision_safe(): + q = "How did spend trend by month?" + assert mint_id(q, set()) == "how-did-spend-trend-by-month" + second = mint_id(q, {"how-did-spend-trend-by-month"}) + assert second.startswith("how-did-spend-trend-by-month-") and second != q + + +def test_list_ids_reads_ids_already_in_the_output_folder(tmp_path): + (tmp_path / "a.json").write_text(json.dumps({"id": "already-there"})) + (tmp_path / "broken.json").write_text("{not json") + assert list_ids(tmp_path) == {"already-there"} + + +def test_langfuse_id_prefix_applies_to_the_export_only(): + envelope = {"id": "q1", "question": "How much?", "expected_output": {"visualization": {}}} + payload = langfuse_payload([envelope], "cust", "ws1", "origin", id_prefix="loop3-") + assert payload["items"][0]["id"] == "loop3-q1" + assert envelope["id"] == "q1" + + +# --- the fixes: no title leakage, no contradictions, real sorts/filters ------ + +DISPLAY = { + "metric/spend": "Spend Amount", + "label/merchant.NAME": "Merchant Name", + "label/process_date.month": "Process Date - Month", + "dataset/process_date": "Process Date", +} + + +def spend_by_merchant(**kw): + return viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "merchant.NAME")]}, + ], + **kw, + ) + + +def test_brief_omits_title_and_chart_type_and_uses_display_names(): + spec = convert( + spend_by_merchant( + title="Top Merchants", + sorts=[ + { + "measureSortItem": { + "direction": "desc", + "locators": [{"measureLocatorItem": {"measureIdentifier": "m"}}], + } + }, + ], + ), + DATE_IDS, + ) + brief = describe(spec, DISPLAY) + assert "Top Merchants" not in brief + assert "bar" not in brief.lower() + assert "metric/spend" not in brief and "merchant.NAME" not in brief + assert "metric: Spend Amount" in brief + assert "broken down by: Merchant Name" in brief + assert "sorted by: Spend Amount, descending" in brief + + +def test_filters_are_briefed_in_words_not_json(): + spec = convert( + spend_by_merchant( + filters=[ + { + "absoluteDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "from": "2025-01-01", + "to": "2025-12-31", + } + }, + { + "rankingFilter": { + "measure": {"localIdentifier": "m"}, + "attributes": [{"localIdentifier": "a"}], + "operator": "TOP", + "value": 5, + } + }, + ] + ), + DATE_IDS, + ) + brief = describe(spec, DISPLAY) + assert "date range 2025-01-01 to 2025-12-31 on Process Date" in brief + assert "top 5 by Spend Amount, ranked within Merchant Name" in brief + assert "{" not in brief + + +@pytest.mark.parametrize( + "title", + [ + "Products by Most Items Sold", + # "Least"/"Worst"/"Largest" name a ranking as plainly as "Most" does. Missing any + # of them lets a mis-specified insight through as a base, and `--enrich-ranked` + # then derives a *top* N from a chart whose own title says the opposite. + "Products by Least Items Sold", + "Worst Performing Merchants", + "Largest Accounts", + ], +) +def test_degenerate_ranking_titles_are_skipped(title): + with pytest.raises(Unsupported, match="promises a ranking"): + convert(spend_by_merchant(title=title), DATE_IDS) + + +def test_degenerate_titles_are_skipped_rather_than_contradicted(): + with pytest.raises(Unsupported, match="promises a ranking"): + convert(spend_by_merchant(title="Products by Most Items Sold"), DATE_IDS) + with pytest.raises(Unsupported, match="promises a filter"): + convert(spend_by_merchant(title="Spend for repeat purchases only"), DATE_IDS) + + +def test_a_real_sort_or_ranking_legitimises_a_ranking_title(): + spec = convert( + spend_by_merchant( + title="Top 5 Merchants", + filters=[{"rankingFilter": {"measure": {"localIdentifier": "m"}, "operator": "TOP", "value": 5}}], + ), + DATE_IDS, + ) + assert spec["_shape"] == "filtered_view" + sorted_spec = convert( + spend_by_merchant( + title="Merchants, Most Spend First", + sorts=[{"attributeSortItem": {"attributeIdentifier": "a", "direction": "asc"}}], + ), + DATE_IDS, + ) + assert sorted_spec["query"]["sort_by"] == [{"type": "attribute_sort", "by": "d_merchant_name", "direction": "ASC"}] + + +def test_contradictions_flag_ranking_and_filter_language_the_spec_lacks(): + plain = convert(spend_by_merchant(), DATE_IDS) + assert contradictions("Which merchants drove the most spend?", plain) + assert contradictions("Show spend by merchant for last quarter", plain) + assert contradictions("How does spend break down across merchants?", plain) == [] + + ranked = convert( + spend_by_merchant( + filters=[{"rankingFilter": {"measure": {"localIdentifier": "m"}, "operator": "TOP", "value": 5}}] + ), + DATE_IDS, + ) + assert contradictions("What are the top 5 merchants by spend?", ranked) == [] + + +def test_type_is_kept_only_when_the_question_names_the_chart_form(): + spec = convert(spend_by_merchant(), DATE_IDS) + assert resolve_type(spec, "Show me spend by merchant as a bar chart") == "bar_chart" + assert resolve_type(spec, "Which merchants did we spend the most with?") == "" + + +def test_build_blanks_type_for_a_question_that_names_no_chart_form(): + + spec = convert(spend_by_merchant(), DATE_IDS) + envelope = build(spec, "How does spend break down across merchants?", "p", set()) + assert envelope["expected_output"]["visualization"]["type"] == "" + + +def test_display_names_cover_metrics_facts_labels_and_date_granularities(): + names = build_display_names( + {"metrics": [{"id": "m_spend", "title": "Spend Amount"}]}, + { + "datasets": [ + { + "id": "merchant", + "title": "Merchant", + "facts": [{"id": "amt", "title": "Amount"}], + "attributes": [ + {"id": "merchant.NAME", "title": "Merchant Name", "labels": []}, + { + "id": "merchant.CTRY", + "title": "Country", + "labels": [{"id": "merchant.CTRY_ISO", "title": "Country ISO"}], + }, + ], + } + ], + "dateInstances": [{"id": "process_date", "title": "Process Date", "granularities": ["MONTH", "YEAR"]}], + }, + ) + assert names["metric/m_spend"] == "Spend Amount" + assert names["fact/amt"] == "Amount" + assert names["label/merchant.NAME"] == "Merchant Name" + assert names["label/merchant.CTRY_ISO"] == "Country ISO" + assert names["label/process_date.month"] == "Process Date - Month" + assert names["dataset/process_date"] == "Process Date" + # No raw id ever reaches question text, even for something the LDM didn't name. + assert display_name("metric/m_units_sold", names) == "M Units Sold" + + +# --- breakdown clause must match the spec's actual dimensions --------------- + + +def test_metric_echoed_as_its_own_dimension_is_a_hard_error(): + headline = convert( + viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS + ) + assert contradictions("Can you show me Spend Amount by Spend Amount?", headline, DISPLAY) + assert contradictions("Can you show me Spend Amount broken down by Spend Amount?", headline, DISPLAY) + assert contradictions("Can you show me Spend Amount?", headline, DISPLAY) == [] + + +def test_unsubstituted_placeholder_is_a_hard_error(): + headline = convert( + viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS + ) + for bad in ( + "Can you show me Spend Amount by breakdown dimension?", + "Can you show me Spend Amount by {dimension}?", + "Can you show me Spend Amount by <split dimension>?", + ): + assert contradictions(bad, headline, DISPLAY), bad + + +def test_breakdown_promised_but_not_expected(): + headline = convert( + viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS + ) + assert contradictions("Can you show me Spend Amount by Merchant Name?", headline, DISPLAY) + # Explicit negation is legitimate phrasing, not a contradiction. + for ok in ( + "Can you show me Spend Amount with no breakdown?", + "Can you show me Spend Amount without breaking it down by any dimension?", + ): + assert contradictions(ok, headline, DISPLAY) == [], ok + + +def test_breakdown_expected_but_not_asked_is_the_same_severity(): + spec = convert(spend_by_merchant(), DATE_IDS) + assert contradictions("Can you show me Spend Amount?", spec, DISPLAY) + assert contradictions("Can you show me Spend Amount by Merchant Name?", spec, DISPLAY) == [] + # Plurals and reordering still count as naming the dimension. + assert contradictions("How does Spend Amount break down across merchants?", spec, DISPLAY) == [] + + +def test_ranking_phrasing_without_a_dimension_is_not_a_false_breakdown(): + ranked = convert( + viz( + "local:headline", + [{"localIdentifier": "measures", "items": [measure("m", "spend")]}], + filters=[{"rankingFilter": {"measure": {"localIdentifier": "m"}, "operator": "TOP", "value": 5}}], + ), + DATE_IDS, + ) + assert contradictions("What is the top 5 by Spend Amount?", ranked, DISPLAY) == [] + + +def test_every_reported_malformed_question_is_caught(): + """The 7 real failures from the gpt-5.4 run over the Loop workspace.""" + headline = convert( + viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS + ) + names = {"metric/spend": "Variant Exchange Ratio"} + for bad in ( + "Can you show me Variant Exchange Ratio broken down by Variant Exchange Ratio?", + "Can you show me the Variant Exchange Ratio by Variant Exchange Ratio?", + "Can you show me Variant Exchange Ratio by breakdown dimension?", + ): + assert contradictions(bad, headline, names), bad + assert contradictions("Can you show me Variant Exchange Ratio?", headline, names) == [] + + +# --- AD's "All" filters and the singular ranking form ----------------------- + + +@pytest.mark.parametrize( + "noop", + [ + { + "negativeAttributeFilter": { + "displayForm": {"identifier": {"id": "product_name", "type": "label"}}, + "notIn": {"values": []}, + } + }, + { + "positiveAttributeFilter": { + "displayForm": {"identifier": {"id": "product_name", "type": "label"}}, + "in": {"values": []}, + } + }, + { + "relativeDateFilter": { + "dataSet": {"identifier": {"id": "process_date", "type": "dataset"}}, + "granularity": "GDC.time.month", + } + }, + ], +) +def test_all_selection_filters_are_dropped_not_fatal(noop): + """AD writes an unset filter as an empty exclusion or an all-time window. + + It restricts nothing, so it must not appear in the spec -- and must not cost the + insight, which is otherwise perfectly expressible. + """ + spec = convert(spend_by_merchant(filters=[noop]), DATE_IDS) + assert spec["query"]["filter_by"] == {} + assert spec["_shape"] == "breakdown_by_dimension" + + +def test_dropped_noop_filter_does_not_leave_a_gap_in_filter_keys(): + spec = convert( + spend_by_merchant( + filters=[ + { + "negativeAttributeFilter": { + "displayForm": {"identifier": {"id": "x", "type": "label"}}, + "notIn": {"values": []}, + } + }, + { + "positiveAttributeFilter": { + "displayForm": {"identifier": {"id": "region", "type": "label"}}, + "in": {"values": ["EMEA"]}, + } + }, + ] + ), + DATE_IDS, + ) + assert list(spec["query"]["filter_by"]) == ["f0"] + + +def test_uri_form_attribute_filter_is_still_skipped(): + """The guard the empty-values case was wrongly sharing: uris can't become literals.""" + with pytest.raises(Unsupported, match="not given by value"): + convert( + spend_by_merchant( + filters=[ + { + "negativeAttributeFilter": { + "displayForm": {"identifier": {"id": "x", "type": "label"}}, + "notIn": {"uris": ["/obj/1"]}, + } + } + ] + ), + DATE_IDS, + ) + + +# --- derived ranking variants ------------------------------------------------- + + +def _bar(metric_id, label_id, **kw): + return viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", metric_id)]}, + {"localIdentifier": "view", "items": [attribute("a", label_id)]}, + ], + **kw, + ) + + +def test_a_plain_single_metric_breakdown_is_rankable(): + spec = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + assert rankable(spec, DATE_IDS) == "d_merchant_name" + + +@pytest.mark.parametrize( + "content,reason", + [ + ( + viz( + "local:bar", + [ + { + "localIdentifier": "measures", + "items": [measure("m", "spend"), measure("m2", "gross_revenue")], + }, + {"localIdentifier": "view", "items": [attribute("a", "merchant.NAME")]}, + ], + ), + "two metrics leave 'top 3 by what?' unanswered", + ), + ( + viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "view", "items": [attribute("a", "merchant.NAME")]}, + {"localIdentifier": "stack", "items": [attribute("b", "region.NAME")]}, + ], + ), + "a segment makes the N ambiguous between the pair and within a group", + ), + ( + viz( + "local:line", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "trend", "items": [attribute("a", "process_date.month")]}, + ], + ), + "'top 3 months' is not a question anyone asks", + ), + ( + _bar( + "spend", + "merchant.NAME", + sorts=[{"attributeSortItem": {"attributeIdentifier": "a", "direction": "asc"}}], + ), + "already sorts, so the shape is covered by the real insight", + ), + ], +) +def test_ineligible_bases_are_not_ranked(content, reason): + assert rankable(convert(content, DATE_IDS), DATE_IDS) is None, reason + + +def test_headline_without_a_dimension_is_not_rankable(): + spec = convert(viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS) + assert rankable(spec, DATE_IDS) is None + + +@pytest.mark.parametrize("count,expected", [(None, 3), (2, None), (4, None), (5, 3), (6, 3), (7, 5), (50, 5)]) +def test_n_needs_headroom_over_the_element_count(count, expected): + assert derived_n(count) == expected + + +def test_ranking_variant_limits_the_rows_and_keeps_the_base_intact(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + out = derive(base, 3, DATE_IDS) + + assert list(out["query"]["filter_by"].values()) == [{"type": "ranking_filter", "using": "m_spend", "top": 3}] + assert out["query"]["sort_by"] == [] + assert out["_derived_from"] == "v_x" + assert out["_derived_kind"] == "ranking_filter" + assert out["id"] != base["id"] + assert base["query"]["filter_by"] == {}, "the base spec must not be mutated" + + +def test_derived_variants_are_scorable_and_valid(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + envelope = build(derive(base, 3, DATE_IDS), "Show the top 3 Merchants by Spend", "d", set()) + assert _validation_errors(envelope) is None + CreatedVisualization.model_validate(envelope["expected_output"]["visualization"]) + + +def test_a_derived_item_records_where_it_came_from(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + envelope = build(derive(base, 5, DATE_IDS), "Show the top 5 Merchants by Spend", "d", set()) + + assert envelope["derived_from"] == "v_x" + assert envelope["derived_kind"] == "ranking_filter" + assert "_derived_from" not in envelope["expected_output"]["visualization"], "provenance is not part of the spec" + + payload = langfuse_payload([envelope], "d", "ws", "origin") + assert payload["items"][0]["metadata"]["derived_from"] == "v_x" + + +def test_a_base_item_carries_no_provenance_keys(): + envelope = build(convert(_bar("spend", "merchant.NAME"), DATE_IDS), "Show Spend by Merchant", "d", set()) + assert "derived_from" not in envelope + assert "derived_kind" not in langfuse_payload([envelope], "d", "ws", "o")["items"][0]["metadata"] + + +def test_derived_ranking_reads_as_a_ranking_not_a_breakdown(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + rules = _rules_for(derive(base, 3, DATE_IDS), DISPLAY) + assert "the top 3 Merchant Name" in rules + assert "exactly once" in rules + assert "- Say the question is broken down by" not in rules, "the ranking line replaces the breakdown line" + + +def test_a_ranking_over_two_dimensions_names_neither_as_the_ranked_one(): + """No `attribute` means the filter ranks (state, city) pairs, not states within cities.""" + spec = convert( + viz( + "local:bar", + [ + {"localIdentifier": "measures", "items": [measure("m", "orders")]}, + {"localIdentifier": "view", "items": [attribute("a", "state.NAME")]}, + {"localIdentifier": "stack", "items": [attribute("b", "city.NAME")]}, + ], + filters=[{"rankingFilter": {"measure": {"localIdentifier": "m"}, "operator": "TOP", "value": 5}}], + ), + DATE_IDS, + ) + rules = _rules_for(spec, {}) + assert "top 5 rows of" not in rules, "that shorthand asserts a single-dimension scope the filter lacks" + assert "broken down by" in rules + + +def test_a_ranking_within_an_attribute_still_asks_for_the_breakdown(): + # `ranked within <attribute>` ranks inside each group, so the breakdown is real and + # the question has to name it. + spec = convert(spend_by_merchant(), DATE_IDS) + spec["query"]["filter_by"]["f0"] = { + "type": "ranking_filter", + "using": "m_spend", + "attribute": "d_merchant_name", + "top": 3, + } + assert "- Say the question is broken down by" in _rules_for(spec, DISPLAY) + + +def test_a_derived_question_naming_the_ranking_is_not_a_contradiction(): + spec = derive(convert(_bar("spend", "merchant.NAME"), DATE_IDS), 3, DATE_IDS) + assert contradictions("Show me the top 3 Merchant Name values by Spend", spec, DISPLAY) == [] + + +def test_picks_spread_across_metrics_before_repeating_one(): + specs = [ + convert(_bar("spend", "merchant.NAME"), DATE_IDS), + convert(_bar("spend", "region.NAME"), DATE_IDS), + convert(_bar("gross_revenue", "merchant.NAME"), DATE_IDS), + ] + picked = pick_derived(specs, DATE_IDS, 2, counts={}) + + metrics = {p["metrics"][0] for p in picked} + assert len(metrics) == 2, "one popular metric must not take the whole budget" + + +def test_only_ranking_filters_are_derived(): + """A sort-only variant validates into a copy of its base, so none is produced.""" + specs = [convert(_bar("spend", "merchant.NAME"), DATE_IDS), convert(_bar("gross_revenue", "region.NAME"), DATE_IDS)] + + assert [p["_derived_kind"] for p in pick_derived(specs, DATE_IDS, 4, counts={})] == [ + "ranking_filter", + "ranking_filter", + ] + + +def test_the_budget_is_a_hard_cap(): + specs = [convert(_bar("spend", f"d{i}.NAME"), DATE_IDS) for i in range(10)] + assert len(pick_derived(specs, DATE_IDS, 3, counts={})) == 3 + + +def test_a_low_cardinality_dimension_is_not_ranked(): + specs = [convert(_bar("spend", "merchant.NAME"), DATE_IDS)] + assert pick_derived(specs, DATE_IDS, 5, counts={"label/merchant.NAME": 3}) == [] + + +def test_n_follows_the_element_count(): + specs = [convert(_bar("spend", "merchant.NAME"), DATE_IDS)] + picked = pick_derived(specs, DATE_IDS, 1, counts={"label/merchant.NAME": 40}) + assert next(iter(picked[0]["query"]["filter_by"].values()))["top"] == 5 + + +def test_candidates_are_only_the_dimensions_a_derivation_would_need(): + specs = [ + convert(_bar("spend", "merchant.NAME"), DATE_IDS), + convert( + viz( + "local:line", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "trend", "items": [attribute("a", "process_date.month")]}, + ], + ), + DATE_IDS, + ), + ] + assert derived_candidates(specs, DATE_IDS) == {"label/merchant.NAME"} + + +def test_element_counts_stop_at_the_ceiling_and_survive_an_unservable_label(): + class _Content: + def get_label_elements(self, workspace_id, label_id, limit=None): + if label_id == "label/bad": + raise RuntimeError("no such label") + assert limit == 7, "counting further than the largest N plus headroom is wasted work" + return ["v"] * limit + + class _Sdk: + catalog_workspace_content = _Content() + + assert element_counts(_Sdk(), "ws", {"label/merchant.NAME", "label/bad"}) == {"label/merchant.NAME": 7} + + +# --- rescuing insights whose titles promised a ranking ------------------------ + + +@pytest.mark.parametrize( + "title,direction", + [ + ("Top Returned Reasons", "top"), + ("Products With the Highest Return Rate", "top"), + ("Products by Most Items Sold", "top"), + ("Largest Accounts", "top"), + ("Products With the Lowest Return Rate", "bottom"), + ("Products by Least Items Sold", "bottom"), + ("Worst Performing Merchants", "bottom"), + # Names both ends, so it names neither: implementing one would be a coin flip. + ("Top and Bottom Products", None), + ("Spend by Merchant", None), + ], +) +def test_title_direction(title, direction): + assert title_direction(title) == direction + + +def test_a_promised_ranking_carries_the_spec_and_the_intent(): + with pytest.raises(PromisedRanking) as caught: + convert(spend_by_merchant(title="Top 10 Merchants by Spend"), DATE_IDS) + + assert caught.value.direction == "top" + assert caught.value.n == 10 + assert caught.value.spec["metrics"] == ["m_spend"] + assert isinstance(caught.value, Unsupported), "still unusable as a copied fixture" + + +def test_a_promised_ranking_without_a_number_leaves_n_open(): + with pytest.raises(PromisedRanking) as caught: + convert(spend_by_merchant(title="Top Merchants"), DATE_IDS) + assert caught.value.n is None + + +def test_an_ambiguous_ranking_title_is_a_plain_skip(): + with pytest.raises(Unsupported) as caught: + convert(spend_by_merchant(title="Top and Bottom Merchants"), DATE_IDS) + assert not isinstance(caught.value, PromisedRanking) + + +def _promised(title): + with pytest.raises(PromisedRanking) as caught: + convert(spend_by_merchant(title=title), DATE_IDS) + return caught.value + + +def test_a_lowest_title_is_implemented_as_a_bottom_n(): + items = rescued([_promised("Merchants With the Lowest Spend")], DATE_IDS, {"label/merchant.NAME": 40}) + + ranking = next(iter(items[0]["query"]["filter_by"].values())) + assert ranking == {"type": "ranking_filter", "using": "m_spend", "bottom": 5} + assert items[0]["_derived_basis"] == "title", "the human's title asked for this, not the generator" + + +def test_an_explicit_n_in_the_title_wins_over_the_default(): + items = rescued([_promised("Top 10 Merchants by Spend")], DATE_IDS, {"label/merchant.NAME": 40}) + assert next(iter(items[0]["query"]["filter_by"].values()))["top"] == 10 + + +def test_a_title_asking_for_more_rows_than_exist_is_not_rescued(): + assert rescued([_promised("Top 10 Merchants by Spend")], DATE_IDS, {"label/merchant.NAME": 7}) == [] + + +def test_a_promised_ranking_on_an_unrankable_shape_is_not_rescued(): + error = PromisedRanking( + "promises a ranking", + convert( + viz( + "local:line", + [ + {"localIdentifier": "measures", "items": [measure("m", "spend")]}, + {"localIdentifier": "trend", "items": [attribute("a", "process_date.month")]}, + ], + ), + DATE_IDS, + ), + "top", + 5, + ) + assert rescued([error], DATE_IDS, {}) == [] + + +def test_rescued_items_are_spent_before_anything_the_generator_invents(): + # A base unrelated to the rescued one, so ordering is what is under test here and + # not the dedup that would otherwise collapse two identical rankings. + specs = [convert(_bar("revenue", "region.NAME"), DATE_IDS)] + promised = [_promised("Top Merchants by Spend")] + + picked = pick_derived(specs, DATE_IDS, 1, counts={}, promised=promised) + assert [p["_derived_basis"] for p in picked] == ["title"] + + # One base yields one variant, so a budget of 3 over one base is not filled. + picked = pick_derived(specs, DATE_IDS, 3, counts={}, promised=promised) + assert [p["_derived_basis"] for p in picked] == ["title", "shape"] + + +def test_element_counts_cover_the_rescue_candidates_too(): + promised = [_promised("Top Merchants by Spend")] + assert derived_candidates([], DATE_IDS, promised) == {"label/merchant.NAME"} + + +def test_a_bottom_ranking_keeps_the_lowest_rows(): + out = derive(convert(_bar("spend", "merchant.NAME"), DATE_IDS), 3, DATE_IDS, direction="bottom") + assert list(out["query"]["filter_by"].values()) == [{"type": "ranking_filter", "using": "m_spend", "bottom": 3}] + + +def test_an_unknown_direction_is_a_programming_error(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + with pytest.raises(ValueError, match="direction"): + derive(base, 3, DATE_IDS, direction="middle") + + +def test_a_rescued_item_is_scorable_and_says_the_title_asked_for_it(): + items = rescued([_promised("Top 5 Merchants by Spend")], DATE_IDS, {"label/merchant.NAME": 40}) + envelope = build(items[0], "What are the top 5 Merchants by Spend?", "d", set()) + + assert _validation_errors(envelope) is None + assert envelope["derived_basis"] == "title" + assert langfuse_payload([envelope], "d", "ws", "o")["items"][0]["metadata"]["derived_basis"] == "title" + + +def test_a_bottom_ranking_is_briefed_as_bottom(): + items = rescued([_promised("Merchants With the Lowest Spend")], DATE_IDS, {"label/merchant.NAME": 40}) + assert "bottom 5 by Spend Amount" in describe(items[0], DISPLAY) + assert "the bottom 5 Merchant Name" in _rules_for(items[0], DISPLAY) + + +def test_two_insights_with_one_definition_do_not_become_two_items(): + # loop has "Products by Most Items Sold" and "Products Driving the Highest Number of + # Repeat Purchases" over the same metric and dimension. Both promise a ranking, and + # deriving from each produced the identical question twice. + promised = [_promised("Products by Most Items Sold"), _promised("Products With the Highest Spend")] + picked = pick_derived([], DATE_IDS, 5, counts={"label/merchant.NAME": 40}, promised=promised) + + assert len(picked) == 1 + assert len({spec_signature(spec) for spec in picked}) == 1 + + +def test_dedup_compares_what_is_asked_not_how_it_is_titled(): + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + same = derive(base, 5, DATE_IDS) + renamed = derive({**base, "title": "Something Else", "id": "v_other"}, 5, DATE_IDS) + assert spec_signature(same) == spec_signature(renamed) + + other_n = derive(base, 3, DATE_IDS) + other_end = derive(base, 5, DATE_IDS, direction="bottom") + assert len({spec_signature(s) for s in (same, other_n, other_end)}) == 3 + + +def test_a_rescue_and_an_invented_ranking_that_agree_yield_one_item(): + # `pick_derived` spends rescues first, so the surviving item is the grounded one. + base = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + picked = pick_derived( + [base], + DATE_IDS, + 5, + counts={"label/merchant.NAME": 40}, + promised=[_promised("Top 5 Merchants by Spend")], + ) + ranked = [spec for spec in picked if spec["_derived_kind"] == "ranking_filter"] + assert [spec["_derived_basis"] for spec in ranked] == ["title"] + + +# --- items that cannot name what they mean ------------------------------------ + + +def test_a_question_asking_for_one_number_must_ask_to_see_it(): + # A bare "What is the Upsell Ratio?" reads as a request for a definition, and the + # agent answers in prose: seven of loop's headline items failed with no chart built. + spec = convert(viz("local:headline", [{"localIdentifier": "measures", "items": [measure("m", "spend")]}]), DATE_IDS) + rules = _rules_for(spec, DISPLAY) + assert "AS A CHART" in rules + assert "as a single number" in rules + assert "Never a bare 'What is <metric>?'" in rules + assert "Do not name the chart type" not in rules, "a single number needs its form named" + + +def test_a_broken_down_question_is_not_told_to_name_a_chart_form(): + assert "AS A CHART" not in _rules_for(convert(spend_by_merchant(), DATE_IDS), DISPLAY) + + +def test_titles_carried_by_more_than_one_object_are_ambiguous(): + names = { + "label/product_details.LINE_ITEM_TITLE": "Product Title", + "label/EXT__RETURNED_ITEMS.PRODUCT_TITLE": "Product Title", + "label/merchant.NAME": "Merchant Name", + "metric/spend": "Spend Amount", + } + assert ambiguous_titles(names) == {"product title"} + + +def test_the_same_object_listed_twice_is_not_ambiguous(): + assert ambiguous_titles({"label/a": "Product Title"}) == set() + + +def test_an_item_naming_an_ambiguous_dimension_is_reported(): + spec = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + names = {**DISPLAY, "label/other_dataset.NAME": "Merchant Name"} + assert ambiguous_fields(spec, names) == ["Merchant Name"] + assert ambiguous_fields(spec, DISPLAY) == [] + + +def test_an_ambiguous_metric_name_counts_too(): + spec = convert(_bar("spend", "merchant.NAME"), DATE_IDS) + names = {**DISPLAY, "metric/spend_v2": "Spend Amount"} + assert ambiguous_fields(spec, names) == ["Spend Amount"] + + +# --- date granularities ------------------------------------------------------- + + +@pytest.mark.parametrize("spelling", ["monthOfYear", "month_of_year", "MONTH_OF_YEAR"]) +def test_every_spelling_of_a_granularity_resolves(spelling): + # The API returns label ids camelCase; the declarative LDM lists the enum member. + # A lookup under one spelling must not miss the other and fall back to a de-slugged + # id ("Order Created At - Monthofyear"). + phrase = granularity_phrase(f"label/ORDER_CREATED_AT.{spelling}", {"dataset/ORDER_CREATED_AT": "Order Created At"}) + assert phrase == "Order Created At, by month of the year (January to December), combining every year" + + +def test_a_sequential_granularity_rules_out_its_cyclical_twin(): + phrase = granularity_phrase("label/ORDER_CREATED_AT.month", {"dataset/ORDER_CREATED_AT": "Order Created At"}) + assert "one point per calendar month over time" in phrase + assert "not month-of-year" in phrase + + +def test_a_plain_label_has_no_granularity_phrase(): + assert granularity_phrase("label/product_details.LINE_ITEM_TITLE", DISPLAY) is None + assert granularity_phrase("metric/spend", DISPLAY) is None + + +def test_display_names_cover_both_spellings_of_every_granularity(): + names = build_display_names( + {}, + { + "dateInstances": [ + {"id": "ORDER_CREATED_AT", "title": "Order Created At", "granularities": ["MONTH_OF_YEAR"]} + ] + }, + ) + assert names["label/ORDER_CREATED_AT.monthOfYear"] == "Order Created At - Month of Year" + assert names["label/ORDER_CREATED_AT.month_of_year"] == "Order Created At - Month of Year" + + +def _monthly(metric="spend", granularity="month"): + return viz( + "local:line", + [ + {"localIdentifier": "measures", "items": [measure("m", metric)]}, + {"localIdentifier": "trend", "items": [attribute("a", f"process_date.{granularity}")]}, + ], + ) + + +def test_a_date_breakdown_is_briefed_by_what_it_does(): + spec = convert(_monthly(), DATE_IDS) + brief = describe(spec, DISPLAY) + assert "broken down by: Process Date, by month, one point per calendar month over time" in brief + assert "Process Date - Month" not in brief, "a label id in prose is not what an analyst says" + + +def test_a_date_breakdown_is_not_asked_for_verbatim(): + rules = _rules_for(convert(_monthly(), DATE_IDS), DISPLAY) + assert "natural words" in rules + assert "never as a label name" in rules + assert "naming each verbatim" not in rules + + +def test_a_plain_dimension_is_still_asked_for_verbatim(): + rules = _rules_for(convert(spend_by_merchant(), DATE_IDS), DISPLAY) + assert "broken down by Merchant Name, naming each verbatim" in rules + assert "natural words" not in rules + + +def test_a_question_saying_by_month_still_names_the_dimension(): + spec = convert(_monthly(), DATE_IDS) + assert contradictions("Can you show me Spend Amount by month for Process Date?", spec, DISPLAY) == [] + + +def test_a_ranking_word_inside_a_field_name_is_not_a_claim(): + # loop's date dataset is called "Most Recent Label Created At". A question naming it + # verbatim -- which the rules require -- was dropped for "using ranking word 'Most'". + spec = convert( + viz( + "local:line", + [ + {"localIdentifier": "measures", "items": [measure("m", "total_labels")]}, + {"localIdentifier": "trend", "items": [attribute("a", "most_recent_label_created_at.month")]}, + ], + ), + {"most_recent_label_created_at"}, + ) + names = { + "metric/total_labels": "Total Labels", + "dataset/most_recent_label_created_at": "Most Recent Label Created At", + "label/most_recent_label_created_at.month": "Most Recent Label Created At - Month", + } + question = "Can you show me Total Labels by month for Most Recent Label Created At?" + assert contradictions(question, spec, names) == [] + + +def test_a_real_ranking_claim_is_still_caught_around_the_names(): + spec = convert(spend_by_merchant(), DATE_IDS) + problems = contradictions("Show me the top 5 Merchant Name values by Spend Amount", spec, DISPLAY) + assert any("ranking word" in p for p in problems) + + +def test_a_filter_word_inside_a_field_name_is_not_a_claim(): + spec = convert(spend_by_merchant(), DATE_IDS) + names = {**DISPLAY, "label/merchant.NAME": "Merchant Name Excluding Test Accounts"} + assert contradictions("Show me Spend Amount by Merchant Name Excluding Test Accounts", spec, names) == [] + + +def test_granularity_aliases_are_one_object_not_a_collision(): + names = build_display_names( + {}, + {"dateInstances": [{"id": "RETURN_AT", "title": "Return At", "granularities": ["MONTH", "MONTH_OF_YEAR"]}]}, + ) + # Each granularity is registered under several spellings; the aliases must fold. + assert ambiguous_titles(names) == set() + + +# --- the generate() pipeline -------------------------------------------------- + + +def _snapshot(*vizs, granularities=("MONTH",)): + return { + "workspace_id": "ws", + "analytics": { + "visualizationObjects": list(vizs), + "metrics": [{"id": "spend", "title": "Spend Amount"}], + }, + "date_instance_ids": ["process_date"], + "display_names": { + "metric/spend": "Spend Amount", + "metric/revenue": "Revenue Amount", + "label/merchant.NAME": "Merchant Name", + "label/region.NAME": "Region Name", + "dataset/process_date": "Process Date", + }, + "label_cardinality": {"label/merchant.NAME": 40, "label/region.NAME": 40}, + } + + +def _args(tmp_path, **kw): + base = { + "workspace": "ws", + "dataset_name": "d", + "out": str(tmp_path / "out"), + "dashboard": [], + "snapshot_in": None, + "snapshot_out": None, + "langfuse_out": None, + "id_prefix": "", + "no_phrase": True, + "phrase_model": "gpt-4o", + "no_viz_type": False, + "min_questions": 1, + "min_shapes": 1, + "min_filtered": 0, + "enrich_ranked": 0, + "skip_ambiguous": False, + "dry_run": False, + } + return SimpleNamespace(**{**base, **kw}) + + +def _snapshot_file(tmp_path, snapshot): + path = tmp_path / "snap.json" + path.write_text(json.dumps(snapshot)) + return str(path) + + +def test_generate_writes_a_validated_item_per_insight(tmp_path): + snapshot = _snapshot(_bar("spend", "merchant.NAME"), _bar("revenue", "region.NAME")) + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, snapshot)) + + assert generate(args) == 0 + + written = sorted(p.name for p in (tmp_path / "out").glob("*.json")) + assert len(written) == 2 + for path in (tmp_path / "out").glob("*.json"): + assert _validation_errors(json.loads(path.read_text())) is None + + +def test_generate_fails_the_run_when_too_few_insights_survive(tmp_path): + # The gate exists so a thin workspace fails loudly instead of quietly shipping a + # dataset too small to mean anything. + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME")))) + args.min_questions = 15 + + assert generate(args) == 1 + assert list((tmp_path / "out").glob("*.json")), "the items are still written; only the exit code fails" + + +def test_generate_skips_hidden_insights(tmp_path): + # Hidden objects are invisible to the assistant's catalog search, so a question about + # one is unwinnable rather than merely hard. + hidden = _bar("spend", "merchant.NAME") + hidden["isHidden"] = True + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(hidden, _bar("revenue", "region.NAME")))) + + assert generate(args) == 0 + assert len(list((tmp_path / "out").glob("*.json"))) == 1 + + +def test_generate_dry_run_writes_nothing(tmp_path): + args = _args( + tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME"))), dry_run=True + ) + + assert generate(args) == 0 + assert not (tmp_path / "out").exists() + + +def test_generate_exports_a_langfuse_dataset_with_prefixed_ids(tmp_path): + args = _args( + tmp_path, + snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME"))), + langfuse_out=str(tmp_path / "lf.json"), + id_prefix="lr-", + ) + assert generate(args) == 0 + + payload = json.loads((tmp_path / "lf.json").read_text()) + assert payload["workspace"] == "ws" + assert all(item["id"].startswith("lr-") for item in payload["items"]) + + +def test_generate_derives_ranked_items_and_records_their_provenance(tmp_path): + args = _args( + tmp_path, + snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME"), _bar("revenue", "region.NAME"))), + enrich_ranked=2, + ) + assert generate(args) == 0 + + items = [json.loads(p.read_text()) for p in (tmp_path / "out").glob("*.json")] + derived = [i for i in items if i.get("derived_from")] + assert len(derived) == 2 + assert {i["derived_kind"] for i in derived} == {"ranking_filter"} + + +def test_generate_can_drop_the_items_that_name_something_ambiguous(tmp_path): + snapshot = _snapshot(_bar("spend", "merchant.NAME"), _bar("revenue", "region.NAME")) + snapshot["display_names"]["label/other.NAME"] = "Merchant Name" # a second "Merchant Name" + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, snapshot), skip_ambiguous=True) + + assert generate(args) == 0 + kept = [json.loads(p.read_text())["question"] for p in (tmp_path / "out").glob("*.json")] + assert len(kept) == 1, "the item naming the duplicated label is dropped" + + +def test_generate_restricts_to_the_requested_dashboard(tmp_path): + keep, drop = _bar("spend", "merchant.NAME"), _bar("revenue", "region.NAME") + keep["id"], drop["id"] = "v_keep", "v_drop" + snapshot = _snapshot(keep, drop) + snapshot["analytics"]["analyticalDashboards"] = [ + {"id": "dash", "content": {"layout": [{"type": "insight", "insight": {"identifier": {"id": "v_keep"}}}]}} + ] + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, snapshot), dashboard=["dash"]) + + assert generate(args) == 0 + assert len(list((tmp_path / "out").glob("*.json"))) == 1 + + +def test_generate_reports_an_unknown_dashboard_instead_of_generating_everything(tmp_path): + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME")))) + args.dashboard = ["nope"] + + assert generate(args) == 1 + assert not (tmp_path / "out").exists() + + +def test_generate_saves_the_fetched_snapshot_for_replay(tmp_path): + class _Sdk: + pass + + calls = {} + + def fake_fetch(sdk, workspace_id): + calls["workspace"] = workspace_id + return _snapshot(_bar("spend", "merchant.NAME")) + + args = _args(tmp_path, snapshot_out=str(tmp_path / "snap-out.json")) + with patch.object(from_insights_mod, "fetch_snapshot", fake_fetch): + assert generate(args, sdk_factory=_Sdk) == 0 + + assert calls["workspace"] == "ws" + assert json.loads((tmp_path / "snap-out.json").read_text())["workspace_id"] == "ws" + + +def test_generate_without_a_snapshot_or_an_sdk_says_which_is_missing(tmp_path): + with pytest.raises(ValueError, match="snapshot-in"): + generate(_args(tmp_path)) + + +def test_generate_blanks_every_expected_chart_type_on_request(tmp_path): + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME")))) + args.no_viz_type = True + assert generate(args) == 0 + + items = [json.loads(p.read_text()) for p in (tmp_path / "out").glob("*.json")] + assert all(i["expected_output"]["visualization"]["type"] == "" for i in items) + + +# --- the phrasing step -------------------------------------------------------- + + +def _reply(text): + return SimpleNamespace(choices=[SimpleNamespace(message=SimpleNamespace(content=text))]) + + +def _fake_openai(*replies): + """An OpenAI stub returning `replies` in order, recording the prompts it received.""" + sent = [] + + class _Completions: + def create(self, model, messages): + sent.append(messages) + return replies[min(len(sent) - 1, len(replies) - 1)] + + class _Client: + chat = SimpleNamespace(completions=_Completions()) + + return _Client, sent + + +def _phrase(specs, *replies): + client_cls, sent = _fake_openai(*replies) + with ( + patch("openai.OpenAI", client_cls), + patch.dict("os.environ", {"OPENAI_API_KEY": "sk-test"}), + ): + return from_insights_mod.phrase(specs, "gpt-4o", DISPLAY), sent + + +def test_phrase_returns_a_question_the_spec_agrees_with(): + spec = convert(spend_by_merchant(), DATE_IDS) + (questions, sent) = _phrase([spec], _reply('"Show me Spend Amount by Merchant Name"')) + + assert questions == ["Show me Spend Amount by Merchant Name"], "surrounding quotes are stripped" + assert len(sent) == 1, "a clean question is not re-asked" + + +def test_phrase_feeds_a_contradiction_back_once_and_keeps_the_rewrite(): + spec = convert(spend_by_merchant(), DATE_IDS) + (questions, sent) = _phrase( + [spec], + _reply("Show me the top 5 Merchant Name by Spend Amount"), # ranking the spec lacks + _reply("Show me Spend Amount by Merchant Name"), + ) + + assert questions == ["Show me Spend Amount by Merchant Name"] + assert len(sent) == 2 + assert "ranking word" in sent[1][-1]["content"], "the specific contradiction is quoted back" + + +def test_phrase_drops_an_item_the_writer_keeps_contradicting(): + # Shipping a question its own expected_output disagrees with is worse than shipping + # fewer questions, so the second failure drops the item. + spec = convert(spend_by_merchant(), DATE_IDS) + (questions, sent) = _phrase([spec], _reply("Show me the top 5 Merchant Name by Spend Amount")) + + assert questions == [None] + assert len(sent) == 2, "one retry, then give up" + + +def test_phrase_treats_a_refusal_with_no_text_as_a_failed_attempt(): + # `message.content` is None for a refusal; .strip() on it used to crash the run. + spec = convert(spend_by_merchant(), DATE_IDS) + (questions, _) = _phrase([spec], _reply(None)) + assert questions == [None] + + +def test_phrase_requires_the_api_key_rather_than_failing_per_item(): + spec = convert(spend_by_merchant(), DATE_IDS) + client_cls, _ = _fake_openai(_reply("x")) + with ( + patch("openai.OpenAI", client_cls), + patch.dict("os.environ", {}, clear=True), + pytest.raises(OSError, match="OPENAI_API_KEY"), + ): + from_insights_mod.phrase([spec], "gpt-4o", DISPLAY) + + +def test_generate_uses_the_phrasing_step_when_it_is_not_disabled(tmp_path): + args = _args(tmp_path, snapshot_in=_snapshot_file(tmp_path, _snapshot(_bar("spend", "merchant.NAME")))) + args.no_phrase = False + client_cls, sent = _fake_openai(_reply("Show me Spend Amount by Merchant Name")) + + with patch("openai.OpenAI", client_cls), patch.dict("os.environ", {"OPENAI_API_KEY": "sk-test"}): + assert generate(args) == 0 + + assert sent, "the LLM was asked" + written = [json.loads(p.read_text()) for p in (tmp_path / "out").glob("*.json")] + assert written[0]["question"] == "Show me Spend Amount by Merchant Name" diff --git a/packages/gooddata-eval/tests/test_models.py b/packages/gooddata-eval/tests/test_models.py index adf3262d5..0c09a23ef 100644 --- a/packages/gooddata-eval/tests/test_models.py +++ b/packages/gooddata-eval/tests/test_models.py @@ -165,3 +165,9 @@ def test_dataset_item_user_context_defaults_to_none(): } ) assert item.user_context is None + + +def test_a_visualization_without_an_id_still_parses(): + """The agent sometimes omits `id`; nothing scores on it, so it must not error the item.""" + viz = CreatedVisualization.model_validate({"type": "bar_chart", "query": {"fields": {"m": "metric/x"}}}) + assert viz.id is None diff --git a/packages/gooddata-eval/tests/test_scoring.py b/packages/gooddata-eval/tests/test_scoring.py index 873e30628..a5b97dc4d 100644 --- a/packages/gooddata-eval/tests/test_scoring.py +++ b/packages/gooddata-eval/tests/test_scoring.py @@ -2,10 +2,12 @@ from gooddata_eval.core.models import CreatedVisualization from gooddata_eval.core.scoring import ( check_filters, + check_sorts, check_viz_type, get_dimension_uri_set, get_metric_uri_set, normalized_filters, + normalized_sorts, uri_to_display_name, validate_cross_references, ) @@ -205,3 +207,89 @@ def test_normalized_filters_is_empty_per_category_when_unfiltered(): } ) assert normalized_filters(viz) == {"date": [], "ranking": [], "attribute": []} + + +def test_a_date_granularity_compares_equal_whichever_prefix_it_carries(): + # A date dataset exposes each granularity as an attribute whose only label carries + # the same id, so both spellings denote one breakdown. gpt-5.6-luna returned + # `attribute/ORDER_CREATED_AT.month` for a chart the insight recorded as + # `label/ORDER_CREATED_AT.month`, and the raw string compare failed a correct chart. + as_label = _viz(query={"fields": {"d": {"using": "label/ORDER_CREATED_AT.month"}}, "filter_by": {}}, view_by=["d"]) + as_attribute = _viz( + query={"fields": {"d": {"using": "attribute/ORDER_CREATED_AT.month"}}, "filter_by": {}}, view_by=["d"] + ) + assert get_dimension_uri_set(as_label) == get_dimension_uri_set(as_attribute) + + +def test_the_granularity_itself_still_has_to_match(): + sequential = _viz(query={"fields": {"d": {"using": "label/d.month"}}, "filter_by": {}}, view_by=["d"]) + cyclical = _viz(query={"fields": {"d": {"using": "label/d.monthOfYear"}}, "filter_by": {}}, view_by=["d"]) + assert get_dimension_uri_set(sequential) != get_dimension_uri_set(cyclical) + + +def test_a_plain_attribute_is_not_rewritten_as_a_label(): + viz = _viz(query={"fields": {"d": {"using": "attribute/product.title"}}, "filter_by": {}}, view_by=["d"]) + assert get_dimension_uri_set(viz) == {"attribute/product.title"} + + +_SORT_FIELDS = {"m_rev": {"using": "metric/revenue"}, "d_q": {"using": "label/date.quarter"}} + + +def _sorted_viz(sort_by, fields=None): + return _viz(query={"fields": fields or _SORT_FIELDS, "filter_by": {}, "sort_by": sort_by}) + + +def test_sorts_survive_validation_and_resolve_to_uris(): + viz = _sorted_viz([{"type": "metric_sort", "direction": "DESC", "metrics": ["m_rev"]}]) + assert normalized_sorts(viz) == ['{"direction": "DESC", "fields": ["metric/revenue"], "type": "metric_sort"}'] + + +def test_the_same_sort_written_by_either_side_compares_equal(): + expected = _sorted_viz([{"type": "attribute_sort", "by": "d_q", "direction": "ASC"}]) + # The agent adds `aggregation`, and names the field with an alias of its own. + actual = _viz( + query={ + "fields": {"dim0": "attribute/date.quarter"}, + "filter_by": {}, + "sort_by": [{"type": "attribute_sort", "by": "dim0", "direction": "ASC", "aggregation": None}], + } + ) + assert check_sorts(expected, actual) + + +def test_a_metric_sort_reads_metrics_even_when_the_agent_also_sends_by(): + expected = _sorted_viz([{"type": "metric_sort", "direction": "ASC", "metrics": ["m_rev"]}]) + actual = _sorted_viz([{"type": "metric_sort", "direction": "ASC", "metrics": ["m_rev"], "by": "d_q"}]) + assert check_sorts(expected, actual) + + +def test_a_missing_sort_fails_but_a_volunteered_one_does_not(): + """`sort_by: []` records no sort; it does not assert the chart must be unsorted.""" + unsorted = _sorted_viz([]) + sorted_ = _sorted_viz([{"type": "metric_sort", "direction": "DESC", "metrics": ["m_rev"]}]) + assert not check_sorts(sorted_, unsorted), "a sort the question asked for is required" + assert check_sorts(unsorted, sorted_), "chronological order on a time series is not an error" + + +def test_a_wrong_sort_still_fails_when_the_fixture_records_one(): + expected = _sorted_viz([{"type": "metric_sort", "direction": "DESC", "metrics": ["m_rev"]}]) + assert not check_sorts(expected, _sorted_viz([{"type": "attribute_sort", "by": "d_q", "direction": "ASC"}])) + + +def test_a_tiebreak_appended_after_the_required_sorts_is_free(): + """ "State descending" is satisfied by "state descending, then city" -- not by the reverse.""" + required = [{"type": "attribute_sort", "by": "d_q", "direction": "DESC"}] + tiebreak = {"type": "metric_sort", "direction": "ASC", "metrics": ["m_rev"]} + assert check_sorts(_sorted_viz(required), _sorted_viz(required + [tiebreak])) + assert not check_sorts(_sorted_viz(required), _sorted_viz([tiebreak] + required)) + + +def test_direction_type_and_order_all_matter(): + base = [ + {"type": "attribute_sort", "by": "d_q", "direction": "ASC"}, + {"type": "metric_sort", "direction": "DESC", "metrics": ["m_rev"]}, + ] + assert check_sorts(_sorted_viz(base), _sorted_viz(list(base))) + assert not check_sorts(_sorted_viz(base), _sorted_viz(list(reversed(base)))) + flipped = [{**base[0], "direction": "DESC"}, base[1]] + assert not check_sorts(_sorted_viz(base), _sorted_viz(flipped)) diff --git a/uv.lock b/uv.lock index d039a3b3f..452ebdc58 100644 --- a/uv.lock +++ b/uv.lock @@ -1070,6 +1070,7 @@ llm-judge = [ [package.dev-dependencies] dev = [ + { name = "gooddata-eval", extra = ["llm-judge"] }, { name = "pytest" }, ] test = [ @@ -1091,7 +1092,10 @@ requires-dist = [ provides-extras = ["llm-judge"] [package.metadata.requires-dev] -dev = [{ name = "pytest", specifier = ">=8.3.5" }] +dev = [ + { name = "gooddata-eval", extras = ["llm-judge"], editable = "packages/gooddata-eval" }, + { name = "pytest", specifier = ">=8.3.5" }, +] test = [ { name = "pytest", specifier = "~=9.1.1" }, { name = "pytest-cov", specifier = "~=7.1.0" },