diff --git a/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py b/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py index 1f17dd5f8..d5fdabef0 100644 --- a/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py +++ b/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py @@ -24,6 +24,7 @@ from gooddata_eval.core.agentic.visualization import evaluate_agentic_visualization from gooddata_eval.core.agentic.what_if import evaluate_agentic_what_if from gooddata_eval.core.config import ReasoningEffort +from gooddata_eval.core.evaluators.visualization import requires_execution_of from gooddata_eval.core.models import AgenticEvalOutcome, CreatedVisualization, DatasetItem from gooddata_eval.core.runner import EvalReport, ItemReport @@ -179,6 +180,7 @@ def _dispatch_agentic( workspace_id=workspace_id, question=item.question, expected_outputs=_parse_visualization_expected(eo), + requires_execution=requires_execution_of(eo), k=k, gate=gate, agent_id=agent_id, 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 e2e1dced5..04ccdc032 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 @@ -9,6 +9,7 @@ from dataclasses import dataclass, field from typing import Any +from gooddata_api_client.exceptions import NotFoundException from gooddata_sdk import GoodDataSdk from gooddata_eval.core.agentic._gate import ( @@ -50,6 +51,9 @@ _DEFAULT_K = 1 _DEFAULT_MAX_ITERATIONS = 7 +# Seconds to wait before each check that a deleted metric stayed deleted, so the checks fall +# 3, 6 and 12 seconds after the delete; see _delete_metric. +_DELETE_RECHECK_DELAYS_S: tuple[float, ...] = (3.0, 3.0, 6.0) def _best_maql_match(actual_maql: str, expected_outputs: list[dict]) -> tuple[bool, str]: @@ -238,11 +242,53 @@ def _delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> None: MAQL) and the assertion fails. Deleting the created metric on the way out keeps the workspace clean for the next run. Best-effort: failures are logged, not raised. Mirrors ``alert_skill._delete_alert``. + + A delete does not stay done on its own. ``create_metric`` reads the whole analytics + model and writes it back, so a concurrent test whose read preceded this delete + restores the metric when its write lands. The metric is therefore checked again after + each of ``_DELETE_RECHECK_DELAYS_S`` and deleted again while it is still there. """ + if not _try_delete_metric(sdk, workspace_id, metric_id): + return + for delay in _DELETE_RECHECK_DELAYS_S: + time.sleep(delay) + if not _metric_exists(sdk, workspace_id, metric_id): + return + print(f"[CLEANUP] Metric {metric_id} is back or could not be checked; deleting again") + if not _try_delete_metric(sdk, workspace_id, metric_id): + return + if _DELETE_RECHECK_DELAYS_S: + # Every check found it back, so the last delete is unverified: say so rather than + # report a clean workspace. The post-run reset is what removes it for good. + print( + f"[CLEANUP] Metric {metric_id} was back or unchecked at all {len(_DELETE_RECHECK_DELAYS_S)} checks; " + "the last delete is not verified and the metric may remain" + ) + + +def _try_delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> bool: + """Delete the metric; True when it is gone afterwards, a 404 included.""" try: sdk._client.entities_api.delete_entity_metrics(workspace_id, metric_id) + except NotFoundException: + return True except Exception as exc: print(f"[CLEANUP] Failed to delete metric {metric_id}: {exc}") + return False + return True + + +def _metric_exists(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> bool: + """Whether the metric may still be in the workspace. Only a 404 reads as absent; any other + lookup failure reads as present, so the caller deletes again rather than leave a restored + metric behind. The recheck delays bound how often that happens.""" + try: + sdk._client.entities_api.get_entity_metrics(workspace_id, metric_id) + except NotFoundException: + return False + except Exception as exc: + print(f"[CLEANUP] Could not check metric {metric_id}: {exc}") + return True def _execute_single_metric_run( 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 b76b2f1ae..dfd5c5c65 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py @@ -35,6 +35,7 @@ _check_visualization_skill_activated, _evaluate_against_candidates, evaluation_result_detail, + with_execution, ) from gooddata_eval.core.models import ( AgenticAssertionError, @@ -189,6 +190,7 @@ def _execute_single_run( question: str, expected_outputs: list[CreatedVisualization], max_iterations: int = _DEFAULT_MAX_ITERATIONS, + requires_execution: bool = False, ) -> RunResult: """Drive one full multi-turn conversation and evaluate the result.""" total_turns = 0 @@ -214,6 +216,9 @@ def _execute_single_run( # K-run already completed along with any exit_reason. print(f"[CHAT] send_message failed for conversation {conversation_id}: {exc}") current_result = getattr(exc, "partial_result", None) or ChatResult() + # The loop that collects events does not run after CHAT_ERROR, so a partial result's + # chart executions are added here. + all_tool_call_events.extend(current_result.tool_call_events) exit_reason = LoopExit.CHAT_ERROR # Counted here, not at the top of the loop: this request is sent unconditionally, so # with max_iterations=0 the loop body never runs and total_turns would report 0 turns @@ -265,6 +270,9 @@ def _execute_single_run( partial = getattr(exc, "partial_result", None) if partial is not None: current_result = partial + # The loop adds a turn's events at its next pass, which a break skips. The + # partial result is scored, so its chart executions count too. + all_tool_call_events.extend(partial.tool_call_events) exit_reason = LoopExit.CHAT_ERROR break @@ -274,6 +282,7 @@ def _execute_single_run( actual_output = current_result.created_visualizations.objects[0] eval_result, best_expected = _evaluate_against_candidates(expected_outputs, actual_output, skill_activated) + eval_result = with_execution(eval_result, all_tool_call_events, current_result.text_response, requires_execution) return RunResult( conversation_id=conversation_id, @@ -302,6 +311,7 @@ def run_agentic_visualization( reasoning_effort: ReasoningEffort | None = None, agent_id: str | None = None, user_context: dict | None = None, + requires_execution: bool = False, ) -> AgenticRunSummary: """Run K independent conversations and return evaluation results. @@ -309,6 +319,8 @@ def run_agentic_visualization( (e.g. one created by a Tavern YAML POST). Subsequent runs always create fresh conversations. Caller-supplied conversations are not deleted; all conversations created by this function are deleted on completion. + + ``requires_execution`` fails a run whose agent built the chart but never ran it. """ client = ChatClient( host=host, @@ -323,7 +335,9 @@ def run_agentic_visualization( try: conv_id_0 = initial_conversation_id if initial_conversation_id is not None else client.create_conversation() try: - run_results.append(_execute_single_run(client, conv_id_0, question, expected_outputs, max_iterations)) + run_results.append( + _execute_single_run(client, conv_id_0, question, expected_outputs, max_iterations, requires_execution) + ) finally: if initial_conversation_id is None: client.delete_conversation(conv_id_0) @@ -331,7 +345,9 @@ def run_agentic_visualization( for _ in range(1, k): conv_id = client.create_conversation() try: - run_results.append(_execute_single_run(client, conv_id, question, expected_outputs, max_iterations)) + run_results.append( + _execute_single_run(client, conv_id, question, expected_outputs, max_iterations, requires_execution) + ) finally: client.delete_conversation(conv_id) finally: @@ -385,6 +401,7 @@ def evaluate_agentic_visualization( reasoning_effort: ReasoningEffort | None = None, submit_trace_link: SubmitTraceLink = run_trace_link_inline, user_context: dict | None = None, + requires_execution: bool = False, gate: EvalGate = DEFAULT_GATE, ) -> AgenticEvalOutcome: """Run visualization evaluation, log to Langfuse, and raise VisualizationAssertionError on failure. @@ -408,6 +425,7 @@ def evaluate_agentic_visualization( reasoning_effort=reasoning_effort, agent_id=agent_id, user_context=user_context, + requires_execution=requires_execution, ) if langfuse is not None and dataset_item_id: @@ -428,6 +446,8 @@ def _write_scores(ctx: RunTraceContext) -> None: "assertion-vis-filters": ev.filters_correct, "assertion-vis-type": ev.viz_type_hard, } + if ev.requires_execution: + strict_checks["assertion-vis-executed"] = ev.executed with ctx.observe(pt, run_idx, conversation_id=run.conversation_id, output=strict_checks) as tid: ctx.score(tid, name="assertion-cross-ref-valid", value=ev.cross_ref_valid, data_type="BOOLEAN") ctx.score(tid, name="assertion-vis-metric", value=ev.metrics_correct, data_type="BOOLEAN") @@ -435,6 +455,10 @@ def _write_scores(ctx: RunTraceContext) -> None: ctx.score(tid, name="assertion-vis-filters", value=ev.filters_correct, data_type="BOOLEAN") ctx.score(tid, name="assertion-vis-type", value=ev.viz_type_hard, data_type="BOOLEAN") ctx.score(tid, name="skill_selection", value=ev.skill_activated, data_type="BOOLEAN") + # Scored on every item, gating or not, so the run rate is visible per model. + ctx.score(tid, name="assertion-vis-executed", value=ev.executed, data_type="BOOLEAN") + if ev.stated_value_matches is not None: + ctx.score(tid, name="stated-value-matches", value=ev.stated_value_matches, data_type="BOOLEAN") # Superseded by log_gate_scores' K-stable names, kept until the readers # migrate: gdc-nas combo_report.py matches on the literal "pass_at_2". ctx.score(tid, name=f"pass_at_{K}", value=summary.pass_at_k, data_type="BOOLEAN") @@ -532,6 +556,8 @@ def _write_scores(ctx: RunTraceContext) -> None: f" attribute : {ev.filter_attribute_score}\n" f"{_filter_diff('attribute', ev)}" f" Viz Type Hard : {ev.viz_type_hard}\n" + f" Executed : {ev.executed}{' (required)' if ev.requires_execution else ''}\n" + f" Stated Value Matches : {ev.stated_value_matches} (reported only)\n" "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n" ) exc.reasoning_steps = best.reasoning_steps 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 d8381c907..321f6fd19 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 @@ -308,11 +308,15 @@ def _build_chat_result(acc: _SseAccumulator) -> ChatResult: # create_adhoc_visualization but the call failed (e.g. data source not # accessible). The last attempt is the agent's best answer. # - # These are raw tool-call arguments, so they carry no `id` -- nothing was - # ever persisted. CreatedVisualization requires one, so synthesize a - # sentinel rather than letting the whole ChatResult fail to validate: - # dropping the turn entirely would score a stalled data source as a - # content failure, which is exactly what this fallback exists to prevent. + # These are raw tool-call arguments, so they carry no `id` -- the server + # mints it. The sentinel marks the chart as never persisted. Logged because + # a turn that called the tool and answered without a chart is otherwise + # invisible: the user saw no chart either. + _log.warning( + "create_adhoc_visualization was called %d time(s) but the answer has no visualization part; " + "scoring the last call's arguments", + len(acc.adhoc_viz_args), + ) payload["createdVisualizations"] = { "objects": [{"id": _ADHOC_VIZ_ID, **acc.adhoc_viz_args[-1]}], "reasoning": "\n".join(acc.viz_reasoning_parts), 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 55b417d86..19b7bb0b8 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py @@ -1,8 +1,10 @@ # (C) 2026 GoodData Corporation """Agentic visualization evaluator — ported from gdc-nas tavern-e2e app/vis_agentic.py.""" -from dataclasses import dataclass +import re +from dataclasses import dataclass, replace from datetime import date +from typing import Any from gooddata_eval.core.evaluators.base import ItemEvaluation from gooddata_eval.core.models import ( @@ -51,6 +53,14 @@ class EvaluationResult: actual_filters: dict[str, list[str]] expected_sorts: list[str] actual_sorts: list[str] + # Whether the agent ran the chart it built (see `execution_signals`). It gates + # `strict_pass` only on an item whose expected output sets `requires_execution`: a + # chart the user only looks at needs no run, a question that asks for a number does. + executed: bool = False + requires_execution: bool = False + # Whether a number in the reply matches a value the chart returned. None when the reply + # states no number. Reported only: reading numbers out of prose has false hits. + stated_value_matches: bool | None = None @property def strict_pass(self) -> bool: @@ -62,6 +72,7 @@ def strict_pass(self) -> bool: and self.filters_correct and self.sorts_correct and self.viz_type_hard + and (self.executed or not self.requires_execution) ) @property @@ -78,6 +89,128 @@ def strict_checks_passed_count(self) -> int: ) +# A number as prose writes it: a sign before or after an optional currency symbol, thousands +# separators, decimals, then an optional K/M/B scale or a percent sign. Not preceded or +# followed by a word character, so "Q3", "viz_1" and "2x" are not read as numbers. +_NUMBER_RE = re.compile( + r"(?[-+]?)[$€£]?(?P[-+]?)(?P\d{1,3}(?:,\d{3})+|\d+)(?:\.(?P\d+))?" + r"\s?(?P[kKmMbB%])?(?!\w)" +) +_SCALES = {"k": 1e3, "m": 1e6, "b": 1e9} + + +def _numbers_in(text: str) -> list[tuple[float, int, str]]: + """(value, decimals, suffix) for every number written in ``text``; suffix is lowercased.""" + out: list[tuple[float, int, str]] = [] + for m in _NUMBER_RE.finditer(text): + integer, fraction = m.group("int").replace(",", ""), m.group("frac") or "" + suffix = (m.group("suffix") or "").lower() + value = float(f"{integer}.{fraction}" if fraction else integer) + if "-" in (m.group("sign"), m.group("sign2")): + value = -value + out.append((value, len(fraction), suffix)) + return out + + +def _stated_matches(stated: tuple[float, int, str], raw: list[float], formatted: list[tuple[float, str]]) -> bool: + """Whether one number from the reply is a rounding of a returned value.""" + value, decimals, suffix = stated + half_step = 0.5 * 10**-decimals + 1e-9 + # Same number and same scale: "1.2" does not quote a formatted "1.2M". + if any(abs(f - value) < 1e-9 and f_suffix == suffix for f, f_suffix in formatted): + return True + for v in raw: + # A percent states the fraction times 100: "10%" quotes 0.1, and "0.1%" does not. + if suffix == "%": + if abs(v * 100 - value) <= half_step: + return True + continue + scale = _SCALES.get(suffix, 1.0) + if abs(v - value * scale) <= half_step * scale: + return True + return False + + +def _row_values(data: Any) -> tuple[list[float], list[tuple[float, str]]]: + """Numeric cells of an execution result: the raw `rows`, and the (number, scale suffix) + pairs read out of `formatted_rows` -- the display strings the agent is told to quote. + A malformed result yields nothing rather than raising.""" + raw: list[float] = [] + formatted: list[tuple[float, str]] = [] + if not isinstance(data, dict): + return raw, formatted + rows, formatted_rows = data.get("rows"), data.get("formatted_rows") + for row in rows if isinstance(rows, list) else []: + if isinstance(row, dict): + raw += [float(v) for v in row.values() if isinstance(v, (int, float)) and not isinstance(v, bool)] + for row in formatted_rows if isinstance(formatted_rows, list) else []: + if isinstance(row, dict): + for v in row.values(): + if isinstance(v, str): + formatted += [(n[0], n[2]) for n in _numbers_in(v)] + return raw, formatted + + +def execution_signals(tool_call_events: list[ToolCallEvent], reply_text: str | None) -> tuple[bool, bool | None]: + """``(executed, stated_value_matches)`` for one conversation. + + ``executed`` is True when ``execute_visualization`` succeeded for a ref that a successful + ``create_adhoc_visualization`` in the same conversation returned. The chart part of the + answer carries no ref, so this cannot tell which of several built charts was shown. + + ``stated_value_matches`` is None when the reply states no number. Otherwise it is True + when one of those numbers is a rounding of a value an execution returned, and False when + none is -- including when nothing was executed, so the agent could not know the value. + """ + created: set[str] = set() + for tc in tool_call_events: + if tc.function_name == "create_adhoc_visualization": + result = tc.parsed_result() + if not isinstance(result, dict): + continue + ref = result.get("ref") if result.get("status", "success") == "success" else None + if isinstance(ref, str): + created.add(ref) + raw: list[float] = [] + formatted: list[tuple[float, str]] = [] + executed = False + for tc in tool_call_events: + if tc.function_name != "execute_visualization": + continue + result = tc.parsed_result() + args = tc.parsed_arguments() + if not isinstance(result, dict) or not isinstance(args, dict): + continue + ref = args.get("visualization_ref") + if result.get("success") is True and isinstance(ref, str) and ref in created: + executed = True + r, f = _row_values(result.get("data")) + raw += r + formatted += f + stated = _numbers_in(reply_text or "") + if not stated: + return executed, None + return executed, any(_stated_matches(n, raw, formatted) for n in stated) + + +def with_execution( + ev: EvaluationResult, + tool_call_events: list[ToolCallEvent], + reply_text: str | None, + requires_execution: bool, +) -> EvaluationResult: + """``ev`` with the execution signals of the conversation that produced it.""" + executed, stated_value_matches = execution_signals(tool_call_events, reply_text) + return replace( + ev, executed=executed, requires_execution=requires_execution, stated_value_matches=stated_value_matches + ) + + +def requires_execution_of(expected_output: object) -> bool: + """The item-level `requires_execution` flag; absent or not a boolean reads as False.""" + return isinstance(expected_output, dict) and expected_output.get("requires_execution") is True + + def _check_visualization_skill_activated(tool_call_events: list[ToolCallEvent]) -> bool: """Return True if set_skills was called with 'visualization' in skill_names.""" for tc in tool_call_events: @@ -216,6 +349,14 @@ def evaluation_result_detail(ev: EvaluationResult) -> dict: "actual_filters": ev.actual_filters, "expected_sorts": ev.expected_sorts, "actual_sorts": ev.actual_sorts, + # Nested: `quality_score` counts every top-level boolean, and a check an item does not + # gate on must not lower it. `executed` joins the top level only when it gates. + "execution": { + "executed": ev.executed, + "required": ev.requires_execution, + "stated_value_matches": ev.stated_value_matches, + }, + **({"executed": ev.executed} if ev.requires_execution else {}), } @@ -227,6 +368,9 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation actual = _extract_actual(chat_result) skill_activated = _check_visualization_skill_activated(chat_result.tool_call_events) ev, _best_expected = _evaluate_against_candidates(candidates, actual, skill_activated) + ev = with_execution( + ev, chat_result.tool_call_events, chat_result.text_response, requires_execution_of(item.expected_output) + ) return ItemEvaluation( passed=ev.strict_pass, rank_key=(ev.strict_pass, ev.strict_checks_passed_count), diff --git a/packages/gooddata-eval/tests/conftest.py b/packages/gooddata-eval/tests/conftest.py index 61ddf5805..a0e4e835e 100644 --- a/packages/gooddata-eval/tests/conftest.py +++ b/packages/gooddata-eval/tests/conftest.py @@ -43,3 +43,10 @@ def fake_langfuse(monkeypatch: pytest.MonkeyPatch): monkeypatch.setenv("LANGFUSE_SECRET_KEY", "sk-fake") monkeypatch.delenv("LANGFUSE_HOST", raising=False) yield server + + +@pytest.fixture(autouse=True) +def _no_metric_delete_recheck(monkeypatch: pytest.MonkeyPatch) -> None: + """A MagicMock SDK reports every metric as present, so the recheck would sleep and + delete again. Tests of the recheck itself set the delays they need.""" + monkeypatch.setattr("gooddata_eval.core.agentic.metric_skill._DELETE_RECHECK_DELAYS_S", ()) diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index 1a747c437..37e23f3e2 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -7,6 +7,8 @@ from unittest.mock import MagicMock, patch import pytest +from gooddata_api_client.exceptions import NotFoundException +from gooddata_eval.core.agentic import metric_skill as metric_skill_mod from gooddata_eval.core.agentic.metric_skill import ( AgenticMetricSummary, MetricRunResult, @@ -440,6 +442,61 @@ def test_delete_metric_uses_sdk_entities_api(): sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "foo_metric") +def test_delete_metric_deletes_again_when_a_concurrent_write_restores_it(monkeypatch): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0, 0.0, 0.0)) + sdk = MagicMock() + # Present after the first delete, gone after the second. + sdk._client.entities_api.get_entity_metrics.side_effect = [object(), NotFoundException(status=404)] + _delete_metric(sdk, "ws1", "foo_metric") + assert sdk._client.entities_api.delete_entity_metrics.call_count == 2 + assert sdk._client.entities_api.get_entity_metrics.call_count == 2 + + +def test_delete_metric_checks_once_when_the_delete_holds(monkeypatch): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0, 0.0, 0.0)) + sdk = MagicMock() + sdk._client.entities_api.get_entity_metrics.side_effect = NotFoundException(status=404) + _delete_metric(sdk, "ws1", "foo_metric") + sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "foo_metric") + sdk._client.entities_api.get_entity_metrics.assert_called_once_with("ws1", "foo_metric") + + +def test_delete_metric_gives_up_after_the_last_recheck(monkeypatch, capsys): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0, 0.0)) + sdk = MagicMock() # get_entity_metrics always succeeds: the metric keeps coming back + _delete_metric(sdk, "ws1", "foo_metric") + assert sdk._client.entities_api.delete_entity_metrics.call_count == 3 + assert "back or unchecked at all 2 checks" in capsys.readouterr().out + # No check after the last delete: a check right after it would always pass. + assert sdk._client.entities_api.get_entity_metrics.call_count == 2 + + +def test_a_failed_lookup_is_not_read_as_a_deleted_metric(monkeypatch): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0, 0.0)) + sdk = MagicMock() + # A transient lookup failure, then a confirmed 404. + sdk._client.entities_api.get_entity_metrics.side_effect = [RuntimeError("503"), NotFoundException(status=404)] + _delete_metric(sdk, "ws1", "foo_metric") + assert sdk._client.entities_api.delete_entity_metrics.call_count == 2 + + +def test_a_404_on_delete_counts_as_deleted(monkeypatch): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0,)) + sdk = MagicMock() + sdk._client.entities_api.delete_entity_metrics.side_effect = NotFoundException(status=404) + sdk._client.entities_api.get_entity_metrics.side_effect = NotFoundException(status=404) + _delete_metric(sdk, "ws1", "foo_metric") + sdk._client.entities_api.get_entity_metrics.assert_called_once_with("ws1", "foo_metric") + + +def test_delete_metric_does_not_recheck_a_failed_delete(monkeypatch): + monkeypatch.setattr(metric_skill_mod, "_DELETE_RECHECK_DELAYS_S", (0.0,)) + sdk = MagicMock() + sdk._client.entities_api.delete_entity_metrics.side_effect = RuntimeError("500") + _delete_metric(sdk, "ws1", "foo_metric") + sdk._client.entities_api.get_entity_metrics.assert_not_called() + + def test_delete_metric_swallows_failures(): sdk = MagicMock() sdk._client.entities_api.delete_entity_metrics.side_effect = RuntimeError("500") diff --git a/packages/gooddata-eval/tests/test_agentic_visualization.py b/packages/gooddata-eval/tests/test_agentic_visualization.py index 49228e2db..fa48b8a91 100644 --- a/packages/gooddata-eval/tests/test_agentic_visualization.py +++ b/packages/gooddata-eval/tests/test_agentic_visualization.py @@ -78,6 +78,17 @@ def test_execute_single_run_viz_on_first_turn(): client.send_message.assert_called_once_with("conv-1", "Show revenue") +def test_execute_single_run_fails_an_unrun_chart_when_execution_is_required(): + client = MagicMock() + client.send_message.return_value = _chat_with_viz() + + result = _execute_single_run(client, "conv-1", "What is revenue?", [_expected()], requires_execution=True) + + assert result.eval_result.visualization_created is True + assert result.eval_result.executed is False + assert result.eval_result.strict_pass is False + + def test_execute_single_run_clarification_then_viz(monkeypatch): """Agent asks a clarification question, simulated user replies, then viz arrives.""" client = MagicMock() @@ -327,6 +338,7 @@ def test_evaluate_agentic_visualization_returns_reasoning_steps_on_pass(): "max_iterations": 4, "expected_sorts": [], "actual_sorts": [], + "execution": {"executed": False, "required": False, "stated_value_matches": None}, "latency_breakdown": [], "tool_calls": [], } @@ -387,6 +399,7 @@ def test_evaluate_agentic_visualization_attaches_reasoning_steps_to_exception_on "max_iterations": 1, "expected_sorts": [], "actual_sorts": [], + "execution": {"executed": False, "required": False, "stated_value_matches": None}, "latency_breakdown": [], "tool_calls": [], } @@ -456,6 +469,68 @@ def test_execute_single_run_records_chat_error_on_a_follow_up_request(monkeypatc assert client.send_message.call_count == 2 +def test_a_partial_result_that_carries_the_chart_counts_its_execution(monkeypatch): + monkeypatch.setattr( + "gooddata_eval.core.agentic.visualization.generate_simulated_response", lambda *a, **k: "the revenue one" + ) + partial = ChatResult.model_validate( + { + "createdVisualizations": {"objects": [_viz()], "reasoning": ""}, + "toolCallEvents": [ + { + "functionName": "create_adhoc_visualization", + "functionArguments": "{}", + "result": '{"status":"success","ref":"viz_1"}', + }, + { + "functionName": "execute_visualization", + "functionArguments": '{"visualization_ref": "viz_1"}', + "result": '{"success":true,"data":{"rows":[]}}', + }, + ], + } + ) + error = ChatError("stream died") + error.partial_result = partial + client = MagicMock() + client.send_message.side_effect = [_chat_clarification(), error] + + result = _execute_single_run(client, "conv-1", "What is revenue?", [_expected()], requires_execution=True) + + assert result.exit_reason is LoopExit.CHAT_ERROR + assert result.eval_result.executed is True + assert result.eval_result.strict_pass is True + + +def test_an_opening_partial_result_that_carries_the_chart_counts_its_execution(): + partial = ChatResult.model_validate( + { + "createdVisualizations": {"objects": [_viz()], "reasoning": ""}, + "toolCallEvents": [ + { + "functionName": "create_adhoc_visualization", + "functionArguments": "{}", + "result": '{"status":"success","ref":"viz_1"}', + }, + { + "functionName": "execute_visualization", + "functionArguments": '{"visualization_ref": "viz_1"}', + "result": '{"success":true,"data":{"rows":[]}}', + }, + ], + } + ) + error = ChatError("stream died") + error.partial_result = partial + client = MagicMock() + client.send_message.side_effect = [error] + + result = _execute_single_run(client, "conv-1", "What is revenue?", [_expected()], requires_execution=True) + + assert result.exit_reason is LoopExit.CHAT_ERROR + assert result.eval_result.executed is True + + def test_execute_single_run_records_simulated_user_failure_separately(monkeypatch): """A harness-side fault must not read as the agent failing to produce a chart.""" diff --git a/packages/gooddata-eval/tests/test_sse_client.py b/packages/gooddata-eval/tests/test_sse_client.py index e40dae503..cee741410 100644 --- a/packages/gooddata-eval/tests/test_sse_client.py +++ b/packages/gooddata-eval/tests/test_sse_client.py @@ -349,7 +349,6 @@ def test_parse_sse_lines_stream_ended_true_when_response_ended_has_no_data_line( def test_parse_sse_lines_falls_back_to_adhoc_viz_when_multipart_viz_is_null(): """Visualization from create_adhoc_visualization args used when multipart viz is null.""" viz_def = { - "id": "total_sales_by_month", "type": "line_chart", "query": {"fields": {"m": {"using": "metric/total_sales"}}, "filter_by": {}}, "metrics": ["m"], @@ -365,16 +364,15 @@ def test_parse_sse_lines_falls_back_to_adhoc_viz_when_multipart_viz_is_null(): ] result = parse_sse_lines(lines) assert result.created_visualizations is not None - assert result.created_visualizations.objects[0].id == "total_sales_by_month" assert result.created_visualizations.objects[0].type == "line_chart" + assert result.created_visualizations.objects[0].metrics == ["m"] -def test_parse_sse_lines_adhoc_fallback_synthesizes_id_when_args_have_none(): - """A create_adhoc_visualization definition carries no `id` -- nothing was persisted. +def test_parse_sse_lines_adhoc_fallback_synthesizes_id_when_args_have_none(caplog): + """A create_adhoc_visualization definition carries no `id` -- the server mints it. - CreatedVisualization requires one, so without a synthesized stand-in the whole - ChatResult fails to validate and the turn is lost. Regression test: real agent - tool arguments have no `id`, unlike the hand-written fixtures above. + The fallback marks the chart with a stand-in id so a report can tell it was never + persisted, and the taking of the fallback is logged. """ viz_def = { "type": "line_chart", @@ -385,9 +383,11 @@ def test_parse_sse_lines_adhoc_fallback_synthesizes_id_when_args_have_none(): f'data: {{"item": {{"role": "assistant", "content": {{"type": "toolCall", "callId": "c1", "name": "create_adhoc_visualization", "arguments": {{"visualization": {json.dumps(viz_def)}}}}}}}}}', 'data: {"item": {"role": "assistant", "content": {"type": "multipart", "parts": [{"type": "visualization", "visualization": null}]}}}', ] - result = parse_sse_lines(lines) + with caplog.at_level("WARNING", logger="gooddata_eval.core.chat.sse_client"): + result = parse_sse_lines(lines) assert result.created_visualizations is not None assert result.created_visualizations.objects[0].id == "adhoc-visualization-not-persisted" + assert "no visualization part" in caplog.text assert result.created_visualizations.objects[0].type == "line_chart" @@ -425,7 +425,7 @@ def test_parse_sse_lines_stamps_reasoning_step_receipt_time(monkeypatch): def test_parse_sse_lines_prefers_multipart_viz_over_adhoc_fallback(): """Real multipart visualization takes priority over adhoc tool call stash.""" - adhoc_viz = {"id": "adhoc", "type": "table", "query": {"fields": {}, "filter_by": {}}} + adhoc_viz = {"type": "table", "query": {"fields": {}, "filter_by": {}}} real_viz = { "id": "real", "type": "column_chart", diff --git a/packages/gooddata-eval/tests/test_visualization_evaluator.py b/packages/gooddata-eval/tests/test_visualization_evaluator.py index bd6ae3d89..3d5cac1e6 100644 --- a/packages/gooddata-eval/tests/test_visualization_evaluator.py +++ b/packages/gooddata-eval/tests/test_visualization_evaluator.py @@ -2,9 +2,10 @@ import re from datetime import date +import pytest from gooddata_eval.core.evaluators import get_evaluator -from gooddata_eval.core.evaluators.visualization import _evaluate_visualization -from gooddata_eval.core.models import ChatResult, CreatedVisualization, DatasetItem +from gooddata_eval.core.evaluators.visualization import _evaluate_visualization, execution_signals +from gooddata_eval.core.models import ChatResult, CreatedVisualization, DatasetItem, ToolCallEvent def _item(expected_viz) -> DatasetItem: @@ -209,3 +210,160 @@ def test_reported_filters_use_the_same_date_anchor_as_the_score(): assert '"from": "2026-02-01"' in result.expected_filters["date"][0] assert '"to": "2026-02-28"' in result.expected_filters["date"][0] assert result.expected_filters["date"] == result.actual_filters["date"] + + +# ── execution signals ─────────────────────────────────────────────────────── + +_CREATE = ToolCallEvent( + functionName="create_adhoc_visualization", + functionArguments='{"visualization": {"type": "headline_chart"}}', + result='{"status":"success","ref":"viz_1"}', +) +# Shapes as recorded on a live sonnet55 run: the raw value and the display string the agent quotes. +_EXECUTE = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1", "max_rows": 10}', + result=( + '{"success":true,"data":{"output_format":"rows","columns":[],' + '"rows":[{"Units Per Transaction":18.303635747929686}],' + '"formatted_rows":[{"Units Per Transaction":"18.30"}],"row_count":1,"truncated":false}}' + ), +) + + +def test_a_built_and_run_chart_whose_value_the_reply_quotes(): + assert execution_signals([_CREATE, _EXECUTE], "The average customer buys **18.30** items per order.") == ( + True, + True, + ) + + +def test_a_built_chart_that_was_never_run(): + assert execution_signals([_CREATE], "Here is the visualization of Star Wars sets.") == (False, None) + + +def test_a_figure_stated_without_running_the_chart_does_not_match(): + assert execution_signals([_CREATE], "There are 15,587 Star Wars sets.") == (False, False) + + +def test_a_figure_that_differs_from_the_executed_value(): + assert execution_signals([_CREATE, _EXECUTE], "Customers buy 21.5 items per order.") == (True, False) + + +def test_a_failed_execution_or_one_for_another_ref_does_not_count(): + failed = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result='{"success":false,"error":"boom"}', + ) + foreign = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_9"}', + result='{"success":true,"data":{"rows":[]}}', + ) + assert execution_signals([_CREATE, failed, foreign], None) == (False, None) + + +@pytest.mark.parametrize( + ("value", "text"), + [(1234567.0, "about 1.2M"), (0.4567, "45.7%"), (15587.0, "$15,587"), (950.0, "950 sets")], +) +def test_a_stated_number_matches_a_rounding_of_the_raw_value(value, text): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result=f'{{"success":true,"data":{{"rows":[{{"v":{value}}}]}}}}', + ) + assert execution_signals([_CREATE, execute], text) == (True, True) + + +def _item_requiring_execution(expected_viz) -> DatasetItem: + return DatasetItem( + id="i1", + dataset_name="d", + test_kind="visualization", + question="What is revenue?", + expected_output={"visualization": expected_viz, "requires_execution": True}, + ) + + +def test_a_correct_chart_fails_when_execution_is_required_and_missing(): + ev = get_evaluator("visualization") + result = ev.evaluate(_item_requiring_execution(_expected()), _chat_result_with(dict(_expected()))) + assert result.passed is False + assert result.detail["executed"] is False + assert result.detail["execution"]["required"] is True + + +def test_a_correct_chart_passes_when_execution_is_required_and_done(): + ev = get_evaluator("visualization") + chat = ChatResult.model_validate( + { + "createdVisualizations": {"objects": [dict(_expected())], "reasoning": ""}, + "toolCallEvents": [_CREATE.model_dump(by_alias=True), _EXECUTE.model_dump(by_alias=True)], + } + ) + result = ev.evaluate(_item_requiring_execution(_expected()), chat) + assert result.passed is True + assert result.detail["executed"] is True + + +def test_an_unrun_chart_still_passes_when_the_item_does_not_require_execution(): + result = get_evaluator("visualization").evaluate(_item(_expected()), _chat_result_with(dict(_expected()))) + assert result.passed is True + assert result.detail["execution"]["executed"] is False + # Not gating, so not a top-level check that quality_score would count. + assert "executed" not in result.detail + + +def test_a_tool_result_that_is_not_a_json_object_is_skipped_not_raised(): + odd_create = ToolCallEvent(functionName="create_adhoc_visualization", functionArguments="{}", result='["viz_1"]') + odd_execute = ToolCallEvent(functionName="execute_visualization", functionArguments='"viz_1"', result='"ok"') + assert execution_signals([odd_create, _CREATE, odd_execute, _EXECUTE], None) == (True, None) + + +def test_a_sign_after_the_currency_symbol_is_read(): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result='{"success":true,"data":{"rows":[{"v":-1234}]}}', + ) + assert execution_signals([_CREATE, execute], "a loss of $-1,234") == (True, True) + + +def test_a_number_without_the_scale_does_not_quote_a_scaled_display_string(): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result='{"success":true,"data":{"rows":[{"v":1200000}],"formatted_rows":[{"v":"1.2M"}]}}', + ) + assert execution_signals([_CREATE, execute], "Revenue was 1.2.") == (True, False) + assert execution_signals([_CREATE, execute], "Revenue was 1.2M.") == (True, True) + + +def test_malformed_rows_in_a_successful_result_are_skipped_not_raised(): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result='{"success":true,"data":{"rows":42,"formatted_rows":"x"}}', + ) + assert execution_signals([_CREATE, execute], "It is 42.") == (True, False) + + +def test_a_percent_is_compared_with_the_fraction_times_100(): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": "viz_1"}', + result='{"success":true,"data":{"rows":[{"v":0.1}]}}', + ) + assert execution_signals([_CREATE, execute], "a 10% share") == (True, True) + assert execution_signals([_CREATE, execute], "a 0.1% share") == (True, False) + + +def test_a_ref_that_is_not_a_string_is_skipped_not_raised(): + execute = ToolCallEvent( + functionName="execute_visualization", + functionArguments='{"visualization_ref": ["viz_1"]}', + result='{"success":true,"data":{"rows":[]}}', + ) + assert execution_signals([_CREATE, execute], None) == (False, None)