fix(gooddata-eval): recheck metric deletes, report chart execution

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,

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(

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,13 +311,16 @@ 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.
 
     If initial_conversation_id is provided, Run 0 reuses that conversation
     (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,15 +335,19 @@ 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)
 
         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,13 +446,19 @@ 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")
                     ctx.score(tid, name="assertion-vis-dimensions", value=ev.dimensions_correct, data_type="BOOLEAN")
                     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

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),