fix(eval): stop guessing whether the agent asked a question

Author: myhoai

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

@@ -25,6 +25,8 @@
 
 _REF_PATTERN = re.compile(r"\$ref:([\w_]+)\.([\w_]+)")
 
+_DEFAULT_MAX_CLARIFICATION_TURNS = 7
+
 
 class TurnDefinition(BaseModel):
     """Definition of a single turn in a multi-turn conversation evaluation."""
@@ -192,13 +194,6 @@ def _check_output_correct(turn: TurnDefinition, chat_result: ChatResult) -> bool
     return None
 
 
-def _is_asking_clarification(text: str) -> bool:
-    if not text:
-        return False
-    t = text.lower()
-    return "?" in t or "could you" in t or "please" in t or "clarif" in t
-
-
 def _get_sim_user_response(agent_message: str, turn: TurnDefinition, expected_output: dict | None) -> str:
     """Generate a simulated user reply to an agent clarification question."""
     otype = turn.expected_output_type
@@ -277,7 +272,7 @@ def run_agentic_conversation(
     token: str,
     workspace_id: str,
     fixture: ConversationFixture,
-    max_clarification_turns: int = 20,
+    max_clarification_turns: int = _DEFAULT_MAX_CLARIFICATION_TURNS,
     initial_conversation_id: str | None = None,
     reasoning_effort: ReasoningEffort | None = None,
 ) -> ConversationResult:
@@ -307,8 +302,22 @@ def run_agentic_conversation(
             owns_conversation = True
 
         for turn in fixture.turns:
-            # Resolve $ref placeholders using outputs captured from prior turns.
-            resolved_expected = _resolve_refs(turn.expected_output, turn_outputs)
+            try:
+                resolved_expected = _resolve_refs(turn.expected_output, turn_outputs)
+            except ValueError as exc:
+                print(f"[SKIP] turn '{turn.turn_id}': {exc}")
+                turn_results.append(
+                    TurnResult(
+                        turn_id=turn.turn_id,
+                        expected_skill=turn.expected_skill,
+                        skill_routing=False,
+                        output_present=False,
+                        no_error=False,
+                        activated_skills=[],
+                        output_correct=False,
+                    )
+                )
+                continue
             resolved_turn = turn.model_copy(update={"expected_output": resolved_expected})
 
             clarification_turns = 0
@@ -327,13 +336,13 @@ def run_agentic_conversation(
                 response_text = (chat_result.text_response or "").strip()
                 if not response_text and chat_result.alert_proposals:
                     response_text = render_alert_proposal(chat_result.alert_proposals[-1])
-                asking = _is_asking_clarification(response_text) or bool(chat_result.alert_proposals)
-                if asking and clarification_turns < max_clarification_turns:
-                    clarification_turns += 1
-                    total_clarification_turns += 1
-                    current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)
-                else:
+                if not response_text and not chat_result.tool_call_events:
+                    break
+                if clarification_turns >= max_clarification_turns:
                     break
+                clarification_turns += 1
+                total_clarification_turns += 1
+                current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)
 
             activated = _activated_skills(all_tool_calls)
             skill_routing = turn.expected_skill in activated if activated else False
@@ -397,7 +406,7 @@ def evaluate_agentic_conversation(
     token: str,
     workspace_id: str,
     fixture: ConversationFixture,
-    max_clarification_turns: int = 20,
+    max_clarification_turns: int = _DEFAULT_MAX_CLARIFICATION_TURNS,
     initial_conversation_id: str | None = None,
     langfuse: object | None = None,
     dataset_item_id: str = "",

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

@@ -72,16 +72,29 @@ def _best_maql_match(actual_maql: str, expected_outputs: list[dict]) -> tuple[bo
     return False, expected_outputs[0].get("maql", "") if expected_outputs else ""
 
 
+class SimulatedResponseError(RuntimeError):
+    """The simulated user could not reply: openai missing, no API key, or the provider failed.
+
+    Carries every expected setup/provider failure so callers can end the run without
+    swallowing programming errors raised from the same call.
+    """
+
+
 def generate_simulated_response(agent_message: str, expected_output: dict) -> str:
-    """Generate a user reply to keep the metric-skill conversation going (gpt-4o-mini)."""
+    """Generate a user reply to keep the metric-skill conversation going (gpt-4o-mini).
+
+    Raises:
+        SimulatedResponseError: openai is not installed, OPENAI_API_KEY is unset, or the
+            provider call failed.
+    """
     try:
-        from openai import OpenAI  # noqa: PLC0415
+        from openai import OpenAI, OpenAIError  # noqa: PLC0415
     except ImportError as exc:
-        raise RuntimeError("openai package is required for generate_simulated_response") from exc
+        raise SimulatedResponseError("openai package is required for generate_simulated_response") from exc
 
     api_key = os.environ.get("OPENAI_API_KEY")
     if not api_key:
-        raise OSError("OPENAI_API_KEY environment variable is not set")
+        raise SimulatedResponseError("OPENAI_API_KEY environment variable is not set")
 
     client = OpenAI(api_key=api_key)
     expected_maql = expected_output.get("maql", "")
@@ -91,12 +104,15 @@ def generate_simulated_response(agent_message: str, expected_output: dict) -> st
         f"The user originally asked to create a metric with MAQL: {expected_maql}. "
         f"Reply briefly as the user, providing any clarification the assistant needs."
     )
-    response = client.chat.completions.create(
-        model="gpt-4o-mini",
-        messages=[{"role": "user", "content": prompt}],
-        max_tokens=150,
-        temperature=0,
-    )
+    try:
+        response = client.chat.completions.create(
+            model="gpt-4o-mini",
+            messages=[{"role": "user", "content": prompt}],
+            max_tokens=150,
+            temperature=0,
+        )
+    except OpenAIError as exc:
+        raise SimulatedResponseError(f"simulated user reply failed: {exc}") from exc
     return response.choices[0].message.content or "Please proceed."
 
 
@@ -166,13 +182,6 @@ def _delete_metric(sdk: GoodDataSdk, workspace_id: str, metric_id: str) -> None:
         print(f"[CLEANUP] Failed to delete metric {metric_id}: {exc}")
 
 
-def _is_asking_clarification(text: str) -> bool:
-    if not text:
-        return False
-    t = text.lower()
-    return "?" in t or "could you" in t or "please provide" in t or "clarif" in t
-
-
 def _execute_single_metric_run(
     client: ChatClient,
     sdk: GoodDataSdk,
@@ -204,9 +213,14 @@ def _execute_single_metric_run(
                 metric_id_to_delete = candidate.get("metric_id")
                 break
             response_text = (chat_result.text_response or "").strip()
-            if _is_asking_clarification(response_text):
+            if not response_text and not chat_result.tool_call_events:
+                break
+            if _iteration >= max_iterations - 1:
+                break
+            try:
                 current_question = generate_simulated_response(response_text, primary_expected)
-            else:
+            except SimulatedResponseError as exc:
+                print(f"[SIM-USER] Simulated reply failed for conversation {conversation_id}: {exc}")
                 break
 
         actual_maql = (metric_result or {}).get("maql", "")

packages/gooddata-eval/tests/test_agentic_conversation.py

@@ -352,3 +352,118 @@ def test_run_agentic_conversation_treats_alert_proposal_as_a_clarification():
     assert "Should I create this alert?" in mock_sim.call_args.args[0]
     assert result.turn_results[0].clarification_turns_used == 1
     assert result.turn_results[0].skill_success is True
+
+
+def _viz_turn_result(text=None, viz=None, tool_calls=()):
+    r = MagicMock()
+    r.text_response = text
+    r.created_visualizations = viz
+    r.tool_call_events = list(tool_calls)
+    r.alert_proposals = []
+    return r
+
+
+def test_run_agentic_conversation_replies_to_a_statement_without_a_question_mark():
+    """QA-28982 regression: gpt-5.2 answered "I need to confirm ... Next I'll:" -- no question
+    mark, so the old substring heuristic ended the turn and no metric was ever created."""
+    mock_client = MagicMock()
+    mock_client.create_conversation.return_value = "conv-1"
+    stalling_turn = _viz_turn_result(
+        text="I can create that, but first I need to confirm which Net Sales calculation to use. Next I'll: ...",
+        tool_calls=[_skills_tc("metric")],
+    )
+    mock_client.send_message.side_effect = [
+        stalling_turn,
+        _metric_turn_result([_skills_tc("metric"), _create_metric_tc("m1")]),
+    ]
+
+    with (
+        patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
+        patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
+        patch(
+            "gooddata_eval.core.agentic.conversation._get_sim_user_response",
+            return_value="Go ahead with Net Sales.",
+        ) as mock_sim,
+    ):
+        result = run_agentic_conversation(
+            host="http://host/api/v1/actions/workspaces/ws1/ai",
+            token="tok",
+            workspace_id="ws1",
+            fixture=_two_metric_turn_fixture().model_copy(update={"turns": _two_metric_turn_fixture().turns[:1]}),
+        )
+
+    mock_sim.assert_called_once()
+    assert result.turn_results[0].clarification_turns_used == 1
+    assert result.turn_results[0].skill_success is True
+
+
+def test_run_agentic_conversation_stops_when_the_agent_says_nothing():
+    """An agent that returns neither text nor tool calls is stuck -- no point replying to it."""
+    mock_client = MagicMock()
+    mock_client.create_conversation.return_value = "conv-1"
+    mock_client.send_message.return_value = _viz_turn_result(text=None)
+
+    with (
+        patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
+        patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
+        patch("gooddata_eval.core.agentic.conversation._get_sim_user_response") as mock_sim,
+    ):
+        result = run_agentic_conversation(
+            host="http://host/api/v1/actions/workspaces/ws1/ai",
+            token="tok",
+            workspace_id="ws1",
+            fixture=_two_metric_turn_fixture().model_copy(update={"turns": _two_metric_turn_fixture().turns[:1]}),
+        )
+
+    mock_sim.assert_not_called()
+    assert mock_client.send_message.call_count == 1
+    assert result.turn_results[0].skill_success is False
+
+
+def test_run_agentic_conversation_records_a_failed_turn_when_a_ref_cannot_be_resolved():
+    """QA-28982 regression: turn 1 producing no metric used to raise ValueError out of the whole
+    run, hiding which turn broke and skipping every later turn."""
+    mock_client = MagicMock()
+    mock_client.create_conversation.return_value = "conv-1"
+    mock_client.send_message.side_effect = [
+        _viz_turn_result(text="Which Net Sales metric?", tool_calls=[_skills_tc("metric")]),
+        _viz_turn_result(text="Working on it.", tool_calls=[_skills_tc("metric")]),
+        _metric_turn_result([_skills_tc("metric"), _create_metric_tc("m2")]),
+    ]
+    fixture = ConversationFixture(
+        id="test-ref",
+        expected_skills=["metric"],
+        turns=[
+            TurnDefinition(
+                turn_id="t1", message="Create shared", expected_skill="metric", expected_output_type="metric"
+            ),
+            TurnDefinition(
+                turn_id="t2",
+                message="Chart it",
+                expected_skill="visualization",
+                expected_output={"metrics": ["metric/$ref:t1.metric_id"]},
+            ),
+            TurnDefinition(
+                turn_id="t3", message="Create another", expected_skill="metric", expected_output_type="metric"
+            ),
+        ],
+    )
+
+    with (
+        patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
+        patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
+        patch("gooddata_eval.core.agentic.conversation._get_sim_user_response", return_value="Go ahead."),
+    ):
+        result = run_agentic_conversation(
+            host="http://host/api/v1/actions/workspaces/ws1/ai",
+            token="tok",
+            workspace_id="ws1",
+            fixture=fixture,
+            max_clarification_turns=1,
+        )
+
+    assert [t.turn_id for t in result.turn_results] == ["t1", "t2", "t3"]
+    assert result.turn_results[0].skill_success is False
+    assert result.turn_results[1].no_error is False
+    assert result.turn_results[2].skill_success is True
+    assert result.conversation_success is False

packages/gooddata-eval/tests/test_agentic_metric_skill.py

@@ -1,13 +1,17 @@
 # (C) 2026 GoodData Corporation. All rights reserved.
 # SPDX-License-Identifier: LicenseRef-GoodData-Enterprise
+import os
+import sys
 from unittest.mock import MagicMock, patch
 
 import pytest
 from gooddata_eval.core.agentic.metric_skill import (
     AgenticMetricSummary,
     MetricRunResult,
+    SimulatedResponseError,
     _delete_metric,
     _normalize_maql,
+    generate_simulated_response,
     run_agentic_metric_skill,
 )
 from gooddata_eval.core.models import ChatResult
@@ -83,7 +87,13 @@ def test_run_agentic_metric_skill_closes_client_on_no_result():
             "reasoningStepCount": 1,
         }
     )
-    with patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client):
+    with (
+        patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client),
+        patch(
+            "gooddata_eval.core.agentic.metric_skill.generate_simulated_response",
+            return_value="Go ahead and create it.",
+        ) as mock_sim,
+    ):
         summary = run_agentic_metric_skill(
             host="http://host/api/v1/actions/workspaces/ws1/ai",
             token="tok",
@@ -96,6 +106,7 @@ def test_run_agentic_metric_skill_closes_client_on_no_result():
     mock_client.close.assert_called_once()
     assert summary.pass_at_k is False
     assert summary.best.metric_created is False
+    mock_sim.assert_called_once_with("I will work on that.", {"maql": "SELECT {metric/foo}"})
 
 
 def test_run_agentic_metric_skill_uses_initial_conversation_for_run_0():
@@ -224,3 +235,52 @@ def test_run_agentic_metric_skill_deletes_metric_even_when_teardown_fails():
         )
 
     mock_sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "foo_metric")
+
+
+def test_generate_simulated_response_without_an_api_key():
+    with (
+        patch.dict(sys.modules, {"openai": MagicMock()}),
+        patch.dict(os.environ, {}, clear=True),
+        pytest.raises(SimulatedResponseError, match="OPENAI_API_KEY"),
+    ):
+        generate_simulated_response("Which brand field?", {"maql": "SELECT {metric/foo}"})
+
+
+def test_generate_simulated_response_without_the_openai_package():
+    with (
+        patch.dict(sys.modules, {"openai": None}),
+        pytest.raises(SimulatedResponseError, match="openai package is required"),
+    ):
+        generate_simulated_response("Which brand field?", {"maql": "SELECT {metric/foo}"})
+
+
+def test_run_agentic_metric_skill_fails_the_run_when_the_simulated_reply_cannot_be_generated():
+    exc = SimulatedResponseError("OPENAI_API_KEY environment variable is not set")
+    mock_client = MagicMock()
+    mock_client.create_conversation.return_value = "conv-1"
+    mock_client.send_message.return_value = ChatResult.model_validate(
+        {
+            "textResponse": "Which brand field should I count?",
+            "toolCallEvents": [],
+            "reasoningStepCount": 1,
+        }
+    )
+    with (
+        patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client),
+        patch("gooddata_eval.core.agentic.metric_skill.generate_simulated_response", side_effect=exc) as mock_sim,
+    ):
+        summary = 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=3,
+        )
+
+    assert summary.pass_at_k is False
+    assert summary.best.metric_created is False
+    assert summary.best.total_turns == 1.0
+    mock_client.close.assert_called_once()
+    mock_sim.assert_called_once_with("Which brand field should I count?", {"maql": "SELECT {metric/foo}"})