From 1c328c4e8d1a0b0da5a8fac9ed1351fb28388642 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 5 Aug 2026 14:02:51 +0200 Subject: [PATCH] feat(gooddata-eval): capture conversation_id/response_id through the agentic-CLI path 8010bd44 wired reasoning_steps through cli/agentic_runner.py -> evaluate_agentic_*, but conversation_id/response_id stayed unset on ItemReport for every agentic kind (agentic_alert_skill/agentic_metric_skill/agentic_conversation) -- each ChatResult already carries both, and conversation_id was already threaded up to the Alert/Metric/ConversationRunResult layer, but neither ever reached the top-level evaluate_agentic_* return value or its failure exception, so run_agentic_items had nothing to read. Mirrors the reasoning_steps idiom exactly: widens each evaluate_agentic_*'s return from list[str] to (reasoning_steps, conversation_id, response_id), attaches all three to the raised exception on failure, and has run_agentic_items unpack either form (tuple or the untouched kinds' bare list/None) onto ItemReport.conversation_id /response_id. response_id is new at the RunResult layer for all three kinds -- captured as the last non-null value across a run's turns, same pattern already used for reasoning_steps accumulation. general_question/guardrail/search_tool/visualization untouched (already populated via the single-turn runner.py path, not this one). Co-Authored-By: Claude Opus 5 --- .../src/gooddata_eval/cli/agentic_runner.py | 16 +++++++++++---- .../gooddata_eval/core/agentic/alert_skill.py | 20 ++++++++++++++----- .../core/agentic/conversation.py | 19 +++++++++++++----- .../core/agentic/metric_skill.py | 20 ++++++++++++++----- .../tests/test_agentic_alert_skill.py | 6 +++++- .../tests/test_agentic_conversation.py | 8 +++++++- .../tests/test_agentic_metric_skill.py | 6 +++++- .../tests/test_agentic_runner.py | 12 ++++++++++- 8 files changed, 84 insertions(+), 23 deletions(-) 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 6edf78fd1..faceb2e43 100644 --- a/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py +++ b/packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py @@ -83,11 +83,12 @@ def _dispatch_agentic( run_ts: str, model_version_override: str | None, reasoning_effort: ReasoningEffort | None = None, -) -> list[str] | None: +) -> tuple[list[str], str, str | None] | list[str] | None: """Call the appropriate evaluate_agentic_* function for the item's test_kind. - Returns whatever that function returns -- only alert_skill/metric_skill/conversation - currently return their reasoning_steps; the rest still return None (unchanged). + Returns whatever that function returns -- alert_skill/metric_skill/conversation return + their (reasoning_steps, conversation_id, response_id); the rest still return None + (unchanged). """ kind = item.test_kind eo = item.expected_output @@ -211,16 +212,23 @@ def run_agentic_items( ) t0 = time.perf_counter() try: - reasoning_steps = _dispatch_agentic( + outcome = _dispatch_agentic( item, host, token, workspace_id, k, langfuse, run_ts, model_version, reasoning_effort ) + reasoning_steps, conversation_id, response_id = ( + outcome if isinstance(outcome, tuple) else (outcome, None, None) + ) item_report.pass_at_k = True item_report.runs = k item_report.reasoning_steps = reasoning_steps or [] + item_report.conversation_id = conversation_id + item_report.response_id = response_id except AssertionError as exc: item_report.pass_at_k = False item_report.runs = k item_report.reasoning_steps = getattr(exc, "reasoning_steps", None) or [] + item_report.conversation_id = getattr(exc, "conversation_id", None) + item_report.response_id = getattr(exc, "response_id", None) print(f"[agentic] {item.id} FAIL: {exc}", flush=True) except Exception as exc: item_report.error = f"{type(exc).__name__}: {exc}" 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 d82ad1600..0d0098280 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 @@ -234,6 +234,7 @@ class AlertRunResult: eval: AlertEvaluation actual_alert_arguments: dict reasoning_steps: list[str] = field(default_factory=list) + response_id: str | None = None @dataclass @@ -359,6 +360,7 @@ def _run_once(conv_id: str) -> AlertRunResult: actual_args: dict = {} tool_called = False reasoning_steps: list[str] = [] + response_id: str | None = None # conversation_history stores prior turns for GPT-4o context. # Roles follow GPT-4o's perspective: "assistant"=agent text, "user"=sim-user reply. conversation_history: list = [] @@ -367,6 +369,7 @@ def _run_once(conv_id: str) -> AlertRunResult: for _iteration in range(max_iterations): chat_result = client.send_message(conv_id, current_question) reasoning_steps.extend(chat_result.reasoning_steps or []) + response_id = chat_result.response_id or response_id alert_id, actual_args, tool_called = _extract_alert_call(chat_result.tool_call_events or []) if tool_called: alert_id_to_delete = alert_id @@ -401,6 +404,7 @@ def _run_once(conv_id: str) -> AlertRunResult: eval=ev, actual_alert_arguments=actual_args, reasoning_steps=reasoning_steps, + response_id=response_id, ) finally: if alert_id_to_delete: @@ -452,6 +456,8 @@ class AlertSkillAssertionError(AssertionError): __tracebackhide__ = True reasoning_steps: list[str] + conversation_id: str + response_id: str | None def evaluate_agentic_alert_skill( @@ -470,12 +476,14 @@ def evaluate_agentic_alert_skill( model_version_override: str | None = None, run_metadata_extra: dict | None = None, reasoning_effort: ReasoningEffort | None = None, -) -> list[str]: +) -> tuple[list[str], str, str | None]: """Run alert-skill evaluation, log to Langfuse, and raise AlertSkillAssertionError on failure. - Returns the best run's reasoning_steps on success; on failure the same list is attached - to the raised exception as ``.reasoning_steps`` (mirrors the `conversation_id`-on-exception - idiom in `ChatClient.ask()`) so callers can retrieve it either way. + Returns the best run's (reasoning_steps, conversation_id, response_id) on success; on + failure the same three values are attached to the raised exception as + ``.reasoning_steps``/``.conversation_id``/``.response_id`` (mirrors the + `conversation_id`-on-exception idiom in `ChatClient.ask()`) so callers can retrieve them + either way. """ from datetime import datetime as _dt # noqa: PLC0415 from datetime import timezone as _tz # noqa: PLC0415 @@ -558,5 +566,7 @@ def evaluate_agentic_alert_skill( f"Actual args: {best.actual_alert_arguments}" ) exc.reasoning_steps = best.reasoning_steps + exc.conversation_id = best.conversation_id + exc.response_id = best.response_id raise exc - return summary.best.reasoning_steps + return summary.best.reasoning_steps, summary.best.conversation_id, summary.best.response_id 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 0715f3611..0711d4700 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -271,6 +271,7 @@ class ConversationResult: conversation_success: bool total_clarification_turns: int reasoning_steps: list[str] = field(default_factory=list) + response_id: str | None = None def run_agentic_conversation( @@ -300,6 +301,7 @@ def run_agentic_conversation( # the end — a later turn may $ref a metric an earlier turn created. created_metric_ids: list[str] = [] reasoning_steps: list[str] = [] + response_id: str | None = None try: if initial_conversation_id is not None: @@ -323,6 +325,7 @@ def run_agentic_conversation( final_result = chat_result all_tool_calls.extend(chat_result.tool_call_events or []) reasoning_steps.extend(chat_result.reasoning_steps or []) + response_id = chat_result.response_id or response_id if _check_output_present(resolved_turn, chat_result): break @@ -387,6 +390,7 @@ def run_agentic_conversation( conversation_success=conversation_success, total_clarification_turns=total_clarification_turns, reasoning_steps=reasoning_steps, + response_id=response_id, ) @@ -395,6 +399,8 @@ class ConversationAssertionError(AssertionError): __tracebackhide__ = True reasoning_steps: list[str] + conversation_id: str + response_id: str | None def evaluate_agentic_conversation( @@ -411,12 +417,13 @@ def evaluate_agentic_conversation( model_version_override: str | None = None, run_metadata_extra: dict | None = None, reasoning_effort: ReasoningEffort | None = None, -) -> list[str]: +) -> tuple[list[str], str, str | None]: """Run conversation evaluation, log to Langfuse, and raise on failure. - Returns the conversation's reasoning_steps on success; on failure the same list is - attached to the raised exception as ``.reasoning_steps`` (mirrors the - `conversation_id`-on-exception idiom in `ChatClient.ask()`) so callers can retrieve it + Returns the conversation's (reasoning_steps, conversation_id, response_id) on success; + on failure the same three values are attached to the raised exception as + ``.reasoning_steps``/``.conversation_id``/``.response_id`` (mirrors the + `conversation_id`-on-exception idiom in `ChatClient.ask()`) so callers can retrieve them either way. """ from datetime import datetime as _dt # noqa: PLC0415 @@ -500,5 +507,7 @@ def evaluate_agentic_conversation( f"Failed turns: {[t.turn_id for t in failed_turns]}" ) exc.reasoning_steps = result.reasoning_steps + exc.conversation_id = result.conversation_id + exc.response_id = result.response_id raise exc - return result.reasoning_steps + return result.reasoning_steps, result.conversation_id, result.response_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 553dced6d..930b33b82 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 @@ -110,6 +110,7 @@ class MetricRunResult: maql_correct: bool total_turns: float reasoning_steps: list[str] = field(default_factory=list) + response_id: str | None = None @dataclass @@ -194,12 +195,14 @@ def _execute_single_metric_run( turns = 0 current_question = question reasoning_steps: list[str] = [] + response_id: str | None = None try: for _iteration in range(max_iterations): turns += 1 chat_result = client.send_message(conversation_id, current_question) reasoning_steps.extend(chat_result.reasoning_steps or []) + response_id = chat_result.response_id or response_id candidate = _extract_metric_result(chat_result.tool_call_events or []) if candidate is not None: metric_result = candidate @@ -222,6 +225,7 @@ def _execute_single_metric_run( maql_correct=maql_correct, total_turns=float(turns), reasoning_steps=reasoning_steps, + response_id=response_id, ) finally: if metric_id_to_delete: @@ -290,6 +294,8 @@ class MetricSkillAssertionError(AssertionError): __tracebackhide__ = True reasoning_steps: list[str] + conversation_id: str + response_id: str | None def evaluate_agentic_metric_skill( @@ -308,12 +314,14 @@ def evaluate_agentic_metric_skill( model_version_override: str | None = None, run_metadata_extra: dict | None = None, reasoning_effort: ReasoningEffort | None = None, -) -> list[str]: +) -> tuple[list[str], str, str | None]: """Run metric-skill evaluation, log to Langfuse, and raise MetricSkillAssertionError on failure. - Returns the best run's reasoning_steps on success; on failure the same list is attached - to the raised exception as ``.reasoning_steps`` (mirrors the `conversation_id`-on-exception - idiom in `ChatClient.ask()`) so callers can retrieve it either way. + Returns the best run's (reasoning_steps, conversation_id, response_id) on success; on + failure the same three values are attached to the raised exception as + ``.reasoning_steps``/``.conversation_id``/``.response_id`` (mirrors the + `conversation_id`-on-exception idiom in `ChatClient.ask()`) so callers can retrieve them + either way. """ from datetime import datetime as _dt # noqa: PLC0415 from datetime import timezone as _tz # noqa: PLC0415 @@ -385,5 +393,7 @@ def evaluate_agentic_metric_skill( f"Actual MAQL: {best.actual_maql}." ) exc.reasoning_steps = best.reasoning_steps + exc.conversation_id = best.conversation_id + exc.response_id = best.response_id raise exc - return summary.best.reasoning_steps + return summary.best.reasoning_steps, summary.best.conversation_id, summary.best.response_id diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index bb12bc2e5..0d2b3c0ef 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -324,7 +324,7 @@ def test_evaluate_agentic_alert_skill_returns_reasoning_steps_on_pass(): patch("gooddata_eval.core.agentic.alert_skill.ChatClient", return_value=mock_client), patch("gooddata_eval.core.agentic.alert_skill._delete_alert"), ): - reasoning = evaluate_agentic_alert_skill( + reasoning, conversation_id, response_id = evaluate_agentic_alert_skill( host="http://host", token="tok", workspace_id="ws1", @@ -335,6 +335,8 @@ def test_evaluate_agentic_alert_skill_returns_reasoning_steps_on_pass(): ) assert reasoning == ["thinking about it"] + assert conversation_id == "conv-1" + assert response_id is None def test_evaluate_agentic_alert_skill_attaches_reasoning_steps_to_exception_on_fail(): @@ -363,3 +365,5 @@ def test_evaluate_agentic_alert_skill_attaches_reasoning_steps_to_exception_on_f max_iterations=1, ) assert exc_info.value.reasoning_steps == ["confused thinking"] + 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_conversation.py b/packages/gooddata-eval/tests/test_agentic_conversation.py index 5ea4eee8b..60e091405 100644 --- a/packages/gooddata-eval/tests/test_agentic_conversation.py +++ b/packages/gooddata-eval/tests/test_agentic_conversation.py @@ -417,6 +417,7 @@ def test_evaluate_agentic_conversation_returns_reasoning_steps_on_pass(): chat_result.created_visualizations = [MagicMock()] chat_result.tool_call_events = [tc] chat_result.reasoning_steps = ["thinking about it"] + chat_result.response_id = "resp-1" mock_client.send_message.return_value = chat_result fixture = ConversationFixture( @@ -432,13 +433,15 @@ def test_evaluate_agentic_conversation_returns_reasoning_steps_on_pass(): ], ) with patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client): - reasoning = evaluate_agentic_conversation( + reasoning, conversation_id, response_id = evaluate_agentic_conversation( host="http://host", token="tok", workspace_id="ws1", fixture=fixture, ) assert reasoning == ["thinking about it"] + assert conversation_id == "conv-1" + assert response_id == "resp-1" def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_fail(): @@ -453,6 +456,7 @@ def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_ chat_result.tool_call_events = [tc] chat_result.alert_proposals = [] chat_result.reasoning_steps = ["confused thinking"] + chat_result.response_id = "resp-2" mock_client.send_message.return_value = chat_result fixture = ConversationFixture( @@ -478,3 +482,5 @@ def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_ fixture=fixture, ) assert exc_info.value.reasoning_steps == ["confused thinking"] + assert exc_info.value.conversation_id == "conv-1" + assert exc_info.value.response_id == "resp-2" diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index 19076dacf..f1c6316de 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -287,7 +287,7 @@ def test_evaluate_agentic_metric_skill_returns_reasoning_steps_on_pass(): } ) with patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client): - reasoning = evaluate_agentic_metric_skill( + reasoning, conversation_id, response_id = evaluate_agentic_metric_skill( host="http://host/api/v1/actions/workspaces/ws1/ai", token="tok", workspace_id="ws1", @@ -297,6 +297,8 @@ def test_evaluate_agentic_metric_skill_returns_reasoning_steps_on_pass(): max_iterations=1, ) assert reasoning == ["thinking about it"] + assert conversation_id == "conv-1" + assert response_id is None def test_evaluate_agentic_metric_skill_attaches_reasoning_steps_to_exception_on_fail(): @@ -323,3 +325,5 @@ def test_evaluate_agentic_metric_skill_attaches_reasoning_steps_to_exception_on_ max_iterations=1, ) assert exc_info.value.reasoning_steps == ["confused thinking"] + 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_runner.py b/packages/gooddata-eval/tests/test_agentic_runner.py index 9d6c5c70c..d433541cb 100644 --- a/packages/gooddata-eval/tests/test_agentic_runner.py +++ b/packages/gooddata-eval/tests/test_agentic_runner.py @@ -20,7 +20,7 @@ def _item(test_kind: str = "agentic_alert_skill") -> DatasetItem: def test_run_agentic_items_surfaces_reasoning_steps_on_pass(): with patch( "gooddata_eval.cli.agentic_runner.evaluate_agentic_alert_skill", - return_value=["it created the alert"], + return_value=(["it created the alert"], "conv-1", "resp-1"), ): report = run_agentic_items( [_item()], @@ -31,11 +31,15 @@ def test_run_agentic_items_surfaces_reasoning_steps_on_pass(): ) assert report.items[0].pass_at_k is True assert report.items[0].reasoning_steps == ["it created the alert"] + assert report.items[0].conversation_id == "conv-1" + assert report.items[0].response_id == "resp-1" def test_run_agentic_items_surfaces_reasoning_steps_from_exception_on_fail(): exc = AlertSkillAssertionError("nope") exc.reasoning_steps = ["it got confused"] + exc.conversation_id = "conv-2" + exc.response_id = "resp-2" with patch("gooddata_eval.cli.agentic_runner.evaluate_agentic_alert_skill", side_effect=exc): report = run_agentic_items( [_item()], @@ -46,6 +50,8 @@ def test_run_agentic_items_surfaces_reasoning_steps_from_exception_on_fail(): ) assert report.items[0].pass_at_k is False assert report.items[0].reasoning_steps == ["it got confused"] + assert report.items[0].conversation_id == "conv-2" + assert report.items[0].response_id == "resp-2" def test_run_agentic_items_defaults_reasoning_steps_to_empty_when_exception_has_none(): @@ -61,6 +67,8 @@ def test_run_agentic_items_defaults_reasoning_steps_to_empty_when_exception_has_ run_ts="2026-01-01", ) assert report.items[0].reasoning_steps == [] + assert report.items[0].conversation_id is None + assert report.items[0].response_id is None def test_run_agentic_items_defaults_reasoning_steps_to_empty_for_untouched_kinds(): @@ -75,3 +83,5 @@ def test_run_agentic_items_defaults_reasoning_steps_to_empty_for_untouched_kinds ) assert report.items[0].pass_at_k is True assert report.items[0].reasoning_steps == [] + assert report.items[0].conversation_id is None + assert report.items[0].response_id is None