Skip to content

Commit 316a0dc

Browse files
Tomkessclaude
andcommitted
Merge branch 'master' into integration/eval-rebuild
Master gained #1847 (user_context on every kind), #1849 (adhoc-visualization fallback logging, metric-delete recheck, chart-execution reporting) and #1852 (W3C baggage trace labels). Resolutions: - cli/agentic_runner.py: additive imports on both sides, then one duplicate `user_context=item.user_context` removed -- both sides had added it to the report-skill branch independently. - core/agentic/visualization.py: #1849 added two lines to the assertion message (Executed, Stated Value Matches) exactly where this branch moved the raise behind the shared outcome tail. Both land. - tests/test_agentic_general_question.py: #1847 relocated the local user_context test into its own cross-kind table, so its deletion stands; this branch's per-run failure assertions beside it stay. - tests/test_agentic_runner.py rebuilt from master's copy plus the fourteen blocks that exist only here. A union is wrong for this file: one hunk begins mid-test, with its head on the other side. The two branch-only kinds were re-added to master's coverage tables: dashboard_summary and forecasting are absent from any table written against master. dashboard_summary stays OUT of the user-context relay, with the reason recorded -- it builds its own context from the dashboard it summarises and takes its scope from summary_input. 1754 passed, 2 skipped. ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 parents bee813a + 0f69236 commit 316a0dc

12 files changed

Lines changed: 676 additions & 29 deletions

File tree

‎packages/gooddata-eval/AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ Run this before guessing a `--model` string.
156156
| `GD_EVAL_AGENT_ID` | which agent to drive, same as `--agent-id` |
157157
| `LANGFUSE_HOST`, `LANGFUSE_PUBLIC_KEY`, `LANGFUSE_SECRET_KEY` | `--langfuse`, `--langfuse-dataset` |
158158
| `GOODDATA_EVAL_CHAT_*` | SSE retry, backoff and timeout knobs |
159+
| `GOODDATA_EVAL_TRACE_LABELS` | `key=value,...` labels stamped on every observation of a chat conversation via W3C baggage; `model_version` also sets the observation `version`, so set it only for a one-model run: every `--model` in one process gets the same labels. Keys must match `[A-Za-z0-9_-]+`; others are skipped with a warning. Dashboard-summary runs are not labelled |
159160
| `GD_EVAL_TIMERS` | same as `--timers` |
160161

161162
A gitignored `.env` at the repo root is the normal place for these; load it with

‎packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from gooddata_eval.core.agentic.what_if import evaluate_agentic_what_if
2828
from gooddata_eval.core.config import ReasoningEffort
2929
from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError
30+
from gooddata_eval.core.evaluators.visualization import requires_execution_of
3031
from gooddata_eval.core.models import AgenticEvalOutcome, CreatedVisualization, DatasetItem
3132
from gooddata_eval.core.runner import EvalReport, ItemReport
3233

@@ -188,6 +189,7 @@ def _dispatch_agentic(
188189
workspace_id=workspace_id,
189190
question=item.question,
190191
expected_outputs=_parse_visualization_expected(eo),
192+
requires_execution=requires_execution_of(eo),
191193
k=k,
192194
gate=gate,
193195
agent_id=agent_id,
@@ -230,8 +232,8 @@ def _dispatch_agentic(
230232
k=k,
231233
gate=gate,
232234
agent_id=agent_id,
233-
**lf_kw,
234235
user_context=item.user_context,
236+
**lf_kw,
235237
)
236238
elif kind == "agentic_alert_skill":
237239
return evaluate_agentic_alert_skill(

‎packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from dataclasses import dataclass, field
1010
from typing import Any
1111

12+
from gooddata_api_client.exceptions import NotFoundException
1213
from gooddata_sdk import GoodDataSdk
1314

1415
from gooddata_eval.core.agentic._failed_runs import build_failed_runs
@@ -51,6 +52,9 @@
5152

5253
_DEFAULT_K = 1
5354
_DEFAULT_MAX_ITERATIONS = 7
55+
# Seconds to wait before each check that a deleted metric stayed deleted, so the checks fall
56+
# 3, 6 and 12 seconds after the delete; see _delete_metric.
57+
_DELETE_RECHECK_DELAYS_S: tuple[float, ...] = (3.0, 3.0, 6.0)
5458

5559

5660
def _best_maql_match(actual_maql: str, expected_outputs: list[dict]) -> tuple[bool, str]:
@@ -239,11 +243,53 @@ def _delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> None:
239243
MAQL) and the assertion fails. Deleting the created metric on the way out keeps
240244
the workspace clean for the next run. Best-effort: failures are logged, not raised.
241245
Mirrors ``alert_skill._delete_alert``.
246+
247+
A delete does not stay done on its own. ``create_metric`` reads the whole analytics
248+
model and writes it back, so a concurrent test whose read preceded this delete
249+
restores the metric when its write lands. The metric is therefore checked again after
250+
each of ``_DELETE_RECHECK_DELAYS_S`` and deleted again while it is still there.
242251
"""
252+
if not _try_delete_metric(sdk, workspace_id, metric_id):
253+
return
254+
for delay in _DELETE_RECHECK_DELAYS_S:
255+
time.sleep(delay)
256+
if not _metric_exists(sdk, workspace_id, metric_id):
257+
return
258+
print(f"[CLEANUP] Metric {metric_id} is back or could not be checked; deleting again")
259+
if not _try_delete_metric(sdk, workspace_id, metric_id):
260+
return
261+
if _DELETE_RECHECK_DELAYS_S:
262+
# Every check found it back, so the last delete is unverified: say so rather than
263+
# report a clean workspace. The post-run reset is what removes it for good.
264+
print(
265+
f"[CLEANUP] Metric {metric_id} was back or unchecked at all {len(_DELETE_RECHECK_DELAYS_S)} checks; "
266+
"the last delete is not verified and the metric may remain"
267+
)
268+
269+
270+
def _try_delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> bool:
271+
"""Delete the metric; True when it is gone afterwards, a 404 included."""
243272
try:
244273
sdk._client.entities_api.delete_entity_metrics(workspace_id, metric_id)
274+
except NotFoundException:
275+
return True
245276
except Exception as exc:
246277
print(f"[CLEANUP] Failed to delete metric {metric_id}: {exc}")
278+
return False
279+
return True
280+
281+
282+
def _metric_exists(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> bool:
283+
"""Whether the metric may still be in the workspace. Only a 404 reads as absent; any other
284+
lookup failure reads as present, so the caller deletes again rather than leave a restored
285+
metric behind. The recheck delays bound how often that happens."""
286+
try:
287+
sdk._client.entities_api.get_entity_metrics(workspace_id, metric_id)
288+
except NotFoundException:
289+
return False
290+
except Exception as exc:
291+
print(f"[CLEANUP] Could not check metric {metric_id}: {exc}")
292+
return True
247293

248294

249295
def _execute_single_metric_run(

‎packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
_check_visualization_skill_activated,
3939
_evaluate_against_candidates,
4040
evaluation_result_detail,
41+
with_execution,
4142
)
4243
from gooddata_eval.core.models import (
4344
AgenticAssertionError,
@@ -195,6 +196,7 @@ def _execute_single_run(
195196
question: str,
196197
expected_outputs: list[CreatedVisualization],
197198
max_iterations: int = _DEFAULT_MAX_ITERATIONS,
199+
requires_execution: bool = False,
198200
) -> RunResult:
199201
"""Drive one full multi-turn conversation and evaluate the result."""
200202
run_started = time.monotonic()
@@ -221,6 +223,9 @@ def _execute_single_run(
221223
# K-run already completed along with any exit_reason.
222224
print(f"[CHAT] send_message failed for conversation {conversation_id}: {exc}")
223225
current_result = getattr(exc, "partial_result", None) or ChatResult()
226+
# The loop that collects events does not run after CHAT_ERROR, so a partial result's
227+
# chart executions are added here.
228+
all_tool_call_events.extend(current_result.tool_call_events)
224229
exit_reason = LoopExit.CHAT_ERROR
225230
# Counted here, not at the top of the loop: this request is sent unconditionally, so
226231
# with max_iterations=0 the loop body never runs and total_turns would report 0 turns
@@ -272,6 +277,9 @@ def _execute_single_run(
272277
partial = getattr(exc, "partial_result", None)
273278
if partial is not None:
274279
current_result = partial
280+
# The loop adds a turn's events at its next pass, which a break skips. The
281+
# partial result is scored, so its chart executions count too.
282+
all_tool_call_events.extend(partial.tool_call_events)
275283
exit_reason = LoopExit.CHAT_ERROR
276284
break
277285

@@ -281,6 +289,7 @@ def _execute_single_run(
281289
actual_output = current_result.created_visualizations.objects[0]
282290

283291
eval_result, best_expected = _evaluate_against_candidates(expected_outputs, actual_output, skill_activated)
292+
eval_result = with_execution(eval_result, all_tool_call_events, current_result.text_response, requires_execution)
284293

285294
return RunResult(
286295
conversation_id=conversation_id,
@@ -310,13 +319,16 @@ def run_agentic_visualization(
310319
reasoning_effort: ReasoningEffort | None = None,
311320
agent_id: str | None = None,
312321
user_context: dict | None = None,
322+
requires_execution: bool = False,
313323
) -> AgenticRunSummary:
314324
"""Run K independent conversations and return evaluation results.
315325
316326
If initial_conversation_id is provided, Run 0 reuses that conversation
317327
(e.g. one created by a Tavern YAML POST). Subsequent runs always create
318328
fresh conversations. Caller-supplied conversations are not deleted; all
319329
conversations created by this function are deleted on completion.
330+
331+
``requires_execution`` fails a run whose agent built the chart but never ran it.
320332
"""
321333
client = ChatClient(
322334
host=host,
@@ -331,15 +343,19 @@ def run_agentic_visualization(
331343
try:
332344
conv_id_0 = initial_conversation_id if initial_conversation_id is not None else client.create_conversation()
333345
try:
334-
run_results.append(_execute_single_run(client, conv_id_0, question, expected_outputs, max_iterations))
346+
run_results.append(
347+
_execute_single_run(client, conv_id_0, question, expected_outputs, max_iterations, requires_execution)
348+
)
335349
finally:
336350
if initial_conversation_id is None:
337351
client.delete_conversation(conv_id_0)
338352

339353
for _ in range(1, k):
340354
conv_id = client.create_conversation()
341355
try:
342-
run_results.append(_execute_single_run(client, conv_id, question, expected_outputs, max_iterations))
356+
run_results.append(
357+
_execute_single_run(client, conv_id, question, expected_outputs, max_iterations, requires_execution)
358+
)
343359
finally:
344360
client.delete_conversation(conv_id)
345361
finally:
@@ -410,6 +426,7 @@ def evaluate_agentic_visualization(
410426
reasoning_effort: ReasoningEffort | None = None,
411427
submit_trace_link: SubmitTraceLink = run_trace_link_inline,
412428
user_context: dict | None = None,
429+
requires_execution: bool = False,
413430
gate: EvalGate = DEFAULT_GATE,
414431
) -> AgenticEvalOutcome:
415432
"""Run visualization evaluation, log to Langfuse, and raise VisualizationAssertionError on failure.
@@ -433,6 +450,7 @@ def evaluate_agentic_visualization(
433450
reasoning_effort=reasoning_effort,
434451
agent_id=agent_id,
435452
user_context=user_context,
453+
requires_execution=requires_execution,
436454
)
437455

438456
if langfuse is not None and dataset_item_id:
@@ -453,13 +471,19 @@ def _write_scores(ctx: RunTraceContext) -> None:
453471
"assertion-vis-filters": ev.filters_correct,
454472
"assertion-vis-type": ev.viz_type_hard,
455473
}
474+
if ev.requires_execution:
475+
strict_checks["assertion-vis-executed"] = ev.executed
456476
with ctx.observe(pt, run_idx, conversation_id=run.conversation_id, output=strict_checks) as tid:
457477
ctx.score(tid, name="assertion-cross-ref-valid", value=ev.cross_ref_valid, data_type="BOOLEAN")
458478
ctx.score(tid, name="assertion-vis-metric", value=ev.metrics_correct, data_type="BOOLEAN")
459479
ctx.score(tid, name="assertion-vis-dimensions", value=ev.dimensions_correct, data_type="BOOLEAN")
460480
ctx.score(tid, name="assertion-vis-filters", value=ev.filters_correct, data_type="BOOLEAN")
461481
ctx.score(tid, name="assertion-vis-type", value=ev.viz_type_hard, data_type="BOOLEAN")
462482
ctx.score(tid, name="skill_selection", value=ev.skill_activated, data_type="BOOLEAN")
483+
# Scored on every item, gating or not, so the run rate is visible per model.
484+
ctx.score(tid, name="assertion-vis-executed", value=ev.executed, data_type="BOOLEAN")
485+
if ev.stated_value_matches is not None:
486+
ctx.score(tid, name="stated-value-matches", value=ev.stated_value_matches, data_type="BOOLEAN")
463487
# Superseded by log_gate_scores' K-stable names, kept until the readers
464488
# migrate: gdc-nas combo_report.py matches on the literal "pass_at_2".
465489
ctx.score(tid, name=f"pass_at_{K}", value=summary.pass_at_k, data_type="BOOLEAN")
@@ -562,6 +586,8 @@ def _write_scores(ctx: RunTraceContext) -> None:
562586
f" attribute : {ev.filter_attribute_score}\n"
563587
f"{_filter_diff('attribute', ev)}"
564588
f" Viz Type Hard : {ev.viz_type_hard}\n"
589+
f" Executed : {ev.executed}{' (required)' if ev.requires_execution else ''}\n"
590+
f" Stated Value Matches : {ev.stated_value_matches} (reported only)\n"
565591
"━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n",
566592
reasoning_steps=best.reasoning_steps,
567593
conversation_id=best.conversation_id,

‎packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313
protocol, not on this class.
1414
"""
1515

16+
import functools
1617
import json
1718
import logging
1819
import os
20+
import re
1921
import time
2022
from dataclasses import dataclass, field
2123
from typing import Any, Callable, Iterable, TypeVar
24+
from urllib.parse import quote
2225

2326
import httpx
2427

@@ -135,6 +138,45 @@ def _float_env(name: str, default: float) -> float:
135138
# 4-turn conversation run to 4x the budget, which is not what a user would sit through.
136139
_ITEM_TIMEOUT_S = _float_env("GOODDATA_EVAL_CHAT_ITEM_TIMEOUT_S", 0.0)
137140

141+
# Comma-separated `key=value` labels, e.g. `model_version=gpt-5.2,github_run_id=42`, sent as
142+
# W3C baggage on every chat request. gen-ai's Langfuse span processor copies `langfuse_*`
143+
# baggage onto each span it starts, so every observation of the conversation -- the cost-bearing
144+
# generations, retries and the title trace included -- carries them from creation. Langfuse v4
145+
# cannot add them to an observation afterwards. A value cannot contain a comma, and a key must be
146+
# a plain token (letters, digits, `_`, `-`); any other key is skipped with a warning, because httpx
147+
# refuses a non-ASCII header and gen-ai's OTel propagator drops or rewrites the rest.
148+
_TRACE_LABELS_ENV = "GOODDATA_EVAL_TRACE_LABELS"
149+
_TRACE_LABEL_KEY = re.compile(r"[A-Za-z0-9_-]+")
150+
151+
152+
@functools.cache
153+
def _warn_skipped_trace_label(key: str) -> None:
154+
"""Warn once per key per process: every ChatClient re-reads the labels."""
155+
_log.warning("%s: skipping label with key %r, keys must match [A-Za-z0-9_-]+", _TRACE_LABELS_ENV, key)
156+
157+
158+
def _trace_baggage() -> dict[str, str]:
159+
"""The `baggage` header for the labels in ``GOODDATA_EVAL_TRACE_LABELS``, or none.
160+
161+
Each label becomes trace metadata, and ``model_version`` -- the combo the report groups
162+
on -- also becomes the observation ``version``, the dimension Langfuse groups cost by.
163+
Values are percent-encoded: a space or comma would otherwise end the baggage entry.
164+
"""
165+
labels: dict[str, str] = {}
166+
for pair in os.getenv(_TRACE_LABELS_ENV, "").split(","):
167+
key, sep, value = pair.partition("=")
168+
key = key.strip()
169+
if not sep or not key:
170+
continue
171+
if _TRACE_LABEL_KEY.fullmatch(key):
172+
labels[key] = value.strip()
173+
else:
174+
_warn_skipped_trace_label(key)
175+
entries = [f"langfuse_metadata_{k}={quote(v, safe='')}" for k, v in labels.items()]
176+
if labels.get("model_version"):
177+
entries.insert(0, f"langfuse_version={quote(labels['model_version'], safe='')}")
178+
return {"baggage": ",".join(entries)} if entries else {}
179+
138180

139181
def set_default_turn_timeout(seconds: float | None) -> None:
140182
"""Set the per-turn budget every ChatClient built afterwards inherits.
@@ -308,11 +350,15 @@ def _build_chat_result(acc: _SseAccumulator) -> ChatResult:
308350
# create_adhoc_visualization but the call failed (e.g. data source not
309351
# accessible). The last attempt is the agent's best answer.
310352
#
311-
# These are raw tool-call arguments, so they carry no `id` -- nothing was
312-
# ever persisted. CreatedVisualization requires one, so synthesize a
313-
# sentinel rather than letting the whole ChatResult fail to validate:
314-
# dropping the turn entirely would score a stalled data source as a
315-
# content failure, which is exactly what this fallback exists to prevent.
353+
# These are raw tool-call arguments, so they carry no `id` -- the server
354+
# mints it. The sentinel marks the chart as never persisted. Logged because
355+
# a turn that called the tool and answered without a chart is otherwise
356+
# invisible: the user saw no chart either.
357+
_log.warning(
358+
"create_adhoc_visualization was called %d time(s) but the answer has no visualization part; "
359+
"scoring the last call's arguments",
360+
len(acc.adhoc_viz_args),
361+
)
316362
payload["createdVisualizations"] = {
317363
"objects": [{"id": _ADHOC_VIZ_ID, **acc.adhoc_viz_args[-1]}],
318364
"reasoning": "\n".join(acc.viz_reasoning_parts),
@@ -466,7 +512,7 @@ def __init__(
466512
the ``enableAiContextSetup`` feature flag is on for the organization.
467513
"""
468514
self._base = f"{host.rstrip('/')}/api/v1/ai/workspaces/{workspace_id}/chat/conversations"
469-
self._auth = {"Authorization": f"Bearer {token}"}
515+
self._auth = {"Authorization": f"Bearer {token}", **_trace_baggage()}
470516
# 0/None disables the cap. Also lowered onto the read timeout: the wall-clock check
471517
# fires between events, so a turn that goes silent needs the transport to give up too.
472518
budget = _TURN_TIMEOUT_S if turn_timeout_s is None else turn_timeout_s

0 commit comments

Comments
 (0)