From 39306769a637a43b2121e36c4f21a113730bf865 Mon Sep 17 00:00:00 2001 From: huyenthanh09 Date: Thu, 12 Nov 2026 17:20:34 +0700 Subject: [PATCH] feat(gooddata-eval): record tool arguments and results per failed run Only some agentic kinds put tool_calls into their detail, so a failed dashboard, document or KDA run showed which tools ran but not what they were handed or returned. Triage from the CI bundle could not see a wrong id, a tool refusal or the outline the agent read. build_failed_runs now adds tool_calls for every kind whose run result keeps tool_call_events. The redacted HTML report drops it at run level, as it already does inside detail. jira: QA-30037 risk: low Co-Authored-By: Claude Opus 5.5 --- .../core/agentic/_failed_runs.py | 20 +++++-- .../core/reporting/html_report.py | 5 +- .../tests/test_agentic_guardrail.py | 53 ++++++++++++++++++- .../gooddata-eval/tests/test_html_report.py | 3 ++ 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_failed_runs.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_failed_runs.py index a324978d5..694928b9b 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_failed_runs.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_failed_runs.py @@ -9,10 +9,15 @@ evaluator. This turns that list into the same records the single-shot path writes. Keys mirror ``core.runner._failed_run_record`` so one consumer can read ``failed_runs`` from -either path without branching on test kind. ``tool_call_count``/``tool_names`` are the -addition: the agentic kinds capture tool calls per run, and an agent that produced a final -answer having made no tool call at all answered from the model's own knowledge rather than -from the workspace -- a distinction no other recorded field exposes. +either path without branching on test kind. ``tool_call_count``/``tool_names``/``tool_calls`` +are the addition: the agentic kinds capture tool calls per run, and an agent that produced a +final answer having made no tool call at all answered from the model's own knowledge rather +than from the workspace -- a distinction no other recorded field exposes. + +``tool_calls`` holds each call's arguments and result, in the shape of ``build_tool_calls``. +It is recorded for every kind whose run result keeps ``tool_call_events``, including the +kinds whose ``detail`` has no ``tool_calls``. A run result without that list, such as a +document takeaway run, records an empty list. """ from __future__ import annotations @@ -20,6 +25,8 @@ from collections.abc import Callable, Sequence from typing import Any +from gooddata_eval.core.models import build_tool_calls + def build_failed_runs( run_results: Sequence[Any], @@ -33,7 +40,9 @@ def build_failed_runs( kinds: guardrail reads ``run.passed`` while visualization reads ``run.eval_result.strict_pass``, and each kind's diagnostic dict is its own. Everything read here directly goes through ``getattr`` with a default, so a kind whose run result - lacks a field (no response id, no tool capture) records a null instead of raising. + lacks a field (no response id, no tool capture) records a null instead of raising. The + events inside ``tool_call_events`` are the exception: ``build_tool_calls`` reads them as + ``ToolCallEvent``s, so a run that has the list must hold real events in it. A run the judge could not grade is recorded like any other non-passing run, with its ``error`` set. That matches ``core/runner.py``, which also records ungraded runs; the @@ -57,6 +66,7 @@ def build_failed_runs( "reasoning_steps": reasoning_steps, "tool_call_count": len(tool_calls), "tool_names": [getattr(e, "function_name", None) for e in tool_calls], + "tool_calls": build_tool_calls(tool_calls), } ) return records 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 index 10db5cec8..98a290eae 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/reporting/html_report.py @@ -39,11 +39,14 @@ # item's top level left the identical content exposed one level deeper -- the top-level # `reasoning` dropped while the same text survived verbatim in `failed_runs`. # +# The run's own `tool_calls` goes for the reason `detail.tool_calls` goes: its arguments +# and results carry semantic-layer internals and real query rows. +# # Kept on purpose, matching what the top level keeps: `reasoning_step_count` (a count, like # `detail.turns`), `tool_call_count`/`tool_names` (which tool ran, never its arguments or # its result -- the same line `latency_breakdown` draws), and the run's verdict, error and # timings, which are facts about the run rather than about our infrastructure. -_REDACTED_RUN_FIELDS = frozenset({"conversation_id", "response_id", "reasoning_steps"}) +_REDACTED_RUN_FIELDS = frozenset({"conversation_id", "response_id", "reasoning_steps", "tool_calls"}) def _redact_detail(detail: dict) -> dict: diff --git a/packages/gooddata-eval/tests/test_agentic_guardrail.py b/packages/gooddata-eval/tests/test_agentic_guardrail.py index 6686353e1..a75175a00 100644 --- a/packages/gooddata-eval/tests/test_agentic_guardrail.py +++ b/packages/gooddata-eval/tests/test_agentic_guardrail.py @@ -2,9 +2,11 @@ # SPDX-License-Identifier: LicenseRef-GoodData-Enterprise import itertools from contextlib import contextmanager +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest +from gooddata_eval.core.agentic._failed_runs import build_failed_runs from gooddata_eval.core.agentic.guardrail import ( GuardrailAssertionError, GuardrailResult, @@ -12,7 +14,7 @@ run_agentic_guardrail, ) from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError -from gooddata_eval.core.models import ChatResult +from gooddata_eval.core.models import ChatResult, ToolCallEvent @contextmanager @@ -337,6 +339,55 @@ def test_failed_runs_carries_each_run_s_tool_calls(): ) assert [(r["tool_call_count"], r["tool_names"]) for r in outcome.failed_runs] == [(0, [])] + assert [r["tool_calls"] for r in outcome.failed_runs] == [[]] + + +def test_failed_runs_carries_each_call_s_arguments_and_result(): + client = MagicMock() + client.create_conversation.side_effect = (f"conv-{i}" for i in itertools.count(1)) + client.send_message.return_value = ChatResult.model_validate( + { + "textResponse": "answer", + "toolCallEvents": [ + { + "functionName": "search_objects", + "functionArguments": '{"keywords": ["Net Sales"]}', + "result": '{"objects": []}', + "index": 0, + }, + # No index: nothing can join to it, so it is left out as build_tool_calls does. + {"functionName": "draft_dashboard", "functionArguments": "{}"}, + ], + "reasoningSteps": [], + "responseId": "r", + } + ) + judge = MagicMock() + judge.model = "gpt-4o" + judge.score.return_value = (False, "answered it") + + with _patched(client, judge), pytest.raises(GuardrailAssertionError) as exc_info: + evaluate_agentic_guardrail(host="h", token="t", workspace_id="ws", question="q", expected_output="e", k=1) + + (run,) = exc_info.value.failed_runs + assert run["tool_names"] == ["search_objects", "draft_dashboard"] + assert run["tool_calls"] == [ + {"index": 0, "name": "search_objects", "arguments": {"keywords": ["Net Sales"]}, "result": '{"objects": []}'} + ] + + +def test_failed_runs_carries_tool_calls_when_the_kind_s_detail_has_none(): + # Guardrail's detail already holds tool_calls; dashboard, document and KDA build theirs + # from build_latency_breakdown alone, which is the case the run-level field exists for. + event = ToolCallEvent(functionName="create_kda", functionArguments='{"metric": "m"}', result="refused", index=0) + run = SimpleNamespace(passed=False, tool_call_events=[event]) + + (record,) = build_failed_runs([run], passed=lambda r: r.passed, detail=lambda r: {"latency_breakdown": []}) + + assert "tool_calls" not in record["detail"] + assert record["tool_calls"] == [ + {"index": 0, "name": "create_kda", "arguments": {"metric": "m"}, "result": "refused"} + ] # --- an item whose every run went ungraded is still diagnosable --- diff --git a/packages/gooddata-eval/tests/test_html_report.py b/packages/gooddata-eval/tests/test_html_report.py index f78b99303..8435fd23a 100644 --- a/packages/gooddata-eval/tests/test_html_report.py +++ b/packages/gooddata-eval/tests/test_html_report.py @@ -49,6 +49,7 @@ def _doc(model: str, passed: bool) -> dict: "reasoning_steps": ["**Thinking**\n\nper-run internal thoughts"], "tool_call_count": 1, "tool_names": ["search_objects"], + "tool_calls": [{"index": 0, "name": "search_objects", "result": "run-level row data"}], "latency_s": 3.5, "detail": { "metrics_correct": False, @@ -186,11 +187,13 @@ def test_redact_reaches_inside_failed_runs(): assert "run-conv-secret" not in html assert "run-resp-secret" not in html assert "per-run internal thoughts" not in html + assert "run-level row data" not in html run = _embedded(html)["runs"]["Model A"]["items"]["item-1"]["failed_runs"][0] assert "conversation_id" not in run assert "response_id" not in run assert "reasoning_steps" not in run + assert "tool_calls" not in run # What the run actually did survives -- the same line the top level draws. assert run["run_index"] == 2 assert run["passed"] is False