fix(eval): process ChatError.partial_result so created objects cannot leak

Addresses the three open CodeRabbit findings on this PR.

metric_skill / alert_skill -- an object leak, not a reporting gap. The SSE stream
can break AFTER create_metric / create_metric_alert has already succeeded
server-side. Those ids reached created_metric_ids / alert_id_to_delete only from
the normal path, so the ChatError branch broke out of the loop with the id never
registered and the `finally` cleanup deleted nothing. The object stayed in the
workspace, where the next run sharing it can see and reuse it -- exactly what
_delete_metric's own comment says must not happen. Both branches now read
exc.partial_result with the same extraction the normal path uses.

kda_skill -- narrowed `except Exception` to `(ChatError, httpx.HTTPError)`. The
bare form also swallowed bugs in this package: a TypeError in the accumulator came
back as a tidy failed run with exit_reason=CHAT_ERROR, indistinguishable from a
real GoodData fault. ChatError covers what ChatClient raises deliberately;
httpx.HTTPError covers the transport faults it re-raises untouched mid-stream
(RemoteProtocolError, ReadError), which the KDA tests exercise directly. Catching
ChatError alone would not have been enough.

One existing test drove send_message with a bare RuntimeError("stream died") and
relied on it being swallowed. It now raises httpx.ReadError -- a fault
send_message actually produces -- since a bare RuntimeError propagating is the
point of the change.

Both leak fixes are pinned by a test verified to fail without them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: Tomkess

packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py

@@ -700,6 +700,18 @@ def _run_once(conv_id: str) -> AlertRunResult:
                     # Without this the exception escapes run_agentic_alert_skill entirely,
                     # discarding every K-run already completed along with any exit_reason.
                     print(f"[CHAT] send_message failed for conversation {conv_id}: {exc}")
+                    # The stream can break AFTER create_metric_alert already succeeded
+                    # server-side. That id only ever reached alert_id_to_delete from the
+                    # normal path below, so breaking here left the alert in the workspace for
+                    # the `finally` cleanup to miss -- a real object leaking out of a failed
+                    # run, not a reporting gap.
+                    partial = exc.partial_result
+                    if partial is not None:
+                        reasoning_steps.extend(partial.reasoning_steps or [])
+                        response_id = partial.response_id or response_id
+                        partial_alert_id, _, partial_tool_called = _extract_alert_call(partial.tool_call_events or [])
+                        if partial_tool_called:
+                            alert_id_to_delete = partial_alert_id
                     exit_reason = LoopExit.CHAT_ERROR
                     break
                 # `turns_used` counts attempts (set above, so a failed send still shows the

packages/gooddata-eval/src/gooddata_eval/core/agentic/kda_skill.py

@@ -7,6 +7,8 @@
 import os
 from dataclasses import dataclass, field
 
+import httpx
+
 from gooddata_eval.core.agentic._gate import (
     DEFAULT_GATE,
     EvalGate,
@@ -25,7 +27,7 @@
     utc_now,
 )
 from gooddata_eval.core.chat.render import render_answer_text
-from gooddata_eval.core.chat.sse_client import ChatClient
+from gooddata_eval.core.chat.sse_client import ChatClient, ChatError
 from gooddata_eval.core.config import ReasoningEffort
 from gooddata_eval.core.models import (
     AgenticAssertionError,
@@ -294,7 +296,13 @@ def _accumulate(result: ChatResult) -> None:
             turns_used = iteration + 1
             try:
                 chat_result = client.send_message(conv_id, current_question)
-            except Exception as exc:  # noqa: BLE001 -- end this run, not the whole assertion
+            except (ChatError, httpx.HTTPError) as exc:
+                # Narrow on purpose. A bare `except Exception` here also swallowed bugs in
+                # this package -- a TypeError in the accumulator came back as a tidy failed
+                # run with exit_reason=CHAT_ERROR, indistinguishable from a real GoodData
+                # fault. ChatError covers what ChatClient raises deliberately; httpx.HTTPError
+                # covers the transport faults it re-raises untouched mid-stream
+                # (RemoteProtocolError, ReadError). Anything else is ours and should surface.
                 _log.warning("KDA send_message failed for conversation %s: %s", conv_id, exc)
                 partial = getattr(exc, "partial_result", None)
                 if partial is not None:

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

@@ -286,6 +286,17 @@ def _execute_single_metric_run(
                 # GoodData-side fault, so recorded like the simulated-user one below.
                 timings.agent_s += time.monotonic() - agent_started
                 print(f"[CHAT] send_message failed for conversation {conversation_id}: {exc}")
+                # The stream can break AFTER create_metric already succeeded server-side.
+                # Those ids only ever reached created_metric_ids from the normal path below,
+                # so breaking here left the metric in the workspace for the `finally` cleanup
+                # to miss -- a real object leaking out of a failed run, not a reporting gap.
+                partial = exc.partial_result
+                if partial is not None:
+                    reasoning_steps.extend(partial.reasoning_steps or [])
+                    response_id = partial.response_id or response_id
+                    for metric_id in _extract_created_metric_ids(partial.tool_call_events or []):
+                        if metric_id not in created_metric_ids:
+                            created_metric_ids.append(metric_id)
                 exit_reason = LoopExit.CHAT_ERROR
                 break
             agent_elapsed = time.monotonic() - agent_started

packages/gooddata-eval/tests/test_agentic_alert_skill.py

@@ -1269,3 +1269,44 @@ def _capture(_submit, _identity, **kwargs):
 
     assert scores["turns"] == 1
     assert scores["steps"] == 1
+
+
+def test_an_alert_created_before_the_stream_broke_is_still_cleaned_up():
+    """The stream can break AFTER create_metric_alert already succeeded server-side.
+
+    The id reached alert_id_to_delete only from the normal path, so the ChatError branch
+    used to leave a real alert behind in the workspace -- an object leaking out of a
+    failed run, which then fires for real.
+    """
+    partial = ChatResult.model_validate(
+        {
+            "text_response": "",
+            "tool_call_events": [
+                {
+                    "functionName": "create_metric_alert",
+                    "functionArguments": '{"operator": "GREATER_THAN", "threshold": 500}',
+                    "result": '{"id": "alert-1"}',
+                }
+            ],
+        }
+    )
+    mock_client = MagicMock()
+    mock_client.create_conversation.return_value = "conv-1"
+    mock_client.send_message.side_effect = ChatError("stream died after create", partial_result=partial)
+
+    with (
+        patch("gooddata_eval.core.agentic.alert_skill.ChatClient", return_value=mock_client),
+        patch("gooddata_eval.core.agentic.alert_skill._delete_alert") as mock_delete,
+    ):
+        run_agentic_alert_skill(
+            host="http://host",
+            token="tok",
+            workspace_id="ws1",
+            question="Notify me whenever the number of orders goes above 500",
+            expected_output={"operator": "GREATER_THAN", "threshold": 500},
+            k=1,
+            max_iterations=1,
+        )
+
+    assert mock_delete.call_count == 1
+    assert mock_delete.call_args.args[-1] == "alert-1"

packages/gooddata-eval/tests/test_agentic_kda_skill.py

@@ -1238,7 +1238,10 @@ def test_run_agentic_kda_skill_reports_no_turns_when_the_first_send_fails():
     """A run that never got a reply must not report a turn it did not take."""
     mock_client = MagicMock()
     mock_client.create_conversation.return_value = "conv-1"
-    mock_client.send_message.side_effect = RuntimeError("stream died")
+    # A transport fault, not a bare RuntimeError: the handler narrowed to what
+    # send_message actually raises, so a bare RuntimeError now (correctly) propagates as
+    # the bug in this package that it would be.
+    mock_client.send_message.side_effect = httpx.ReadError("stream died")
 
     with patch("gooddata_eval.core.agentic.kda_skill.ChatClient", return_value=mock_client):
         summary = run_agentic_kda_skill(

packages/gooddata-eval/tests/test_agentic_metric_skill.py

@@ -946,3 +946,34 @@ def _capture(_submit, _identity, **kwargs):
     # they are counts, and a float reads as though a fraction of a turn were possible.
     assert isinstance(scores["turns"], int)
     assert isinstance(scores["steps"], int)
+
+
+def test_a_metric_created_before_the_stream_broke_is_still_cleaned_up():
+    """The stream can break AFTER create_metric already succeeded server-side.
+
+    The id reached created_metric_ids only from the normal path, so the ChatError branch
+    used to leave a real metric behind in the workspace -- an object leaking out of a
+    failed run, which the next run then sees.
+    """
+    mock_client = _client()
+    partial = ChatResult.model_validate(
+        {
+            "textResponse": "",
+            "toolCallEvents": [_create_metric_call('{"data": {"metric_id": "m1", "maql": "SELECT {metric/foo}"}}')],
+        }
+    )
+    mock_client.send_message.side_effect = ChatError("stream died after create", partial_result=partial)
+
+    with _patched(mock_client, sdk=True) as (_, mock_sdk_cls):
+        run_agentic_metric_skill(
+            host="http://host/api/v1/actions/workspaces/ws1/ai",
+            token="tok",
+            workspace_id="ws1",
+            question="Create metric foo",
+            expected_output={"maql": "SELECT {metric/foo}"},
+            k=1,
+            max_iterations=1,
+        )
+
+    sdk = mock_sdk_cls.create.return_value
+    sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "m1")