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 6d5fa4613..876bbb1c6 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/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 = "", 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 8e78a3acf..dc12a2b7f 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 @@ -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", "") diff --git a/packages/gooddata-eval/tests/test_agentic_conversation.py b/packages/gooddata-eval/tests/test_agentic_conversation.py index 9d2234f33..cb2970272 100644 --- a/packages/gooddata-eval/tests/test_agentic_conversation.py +++ b/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 diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index 67a163e92..f00212e1c 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/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}"})