Skip to content

Commit 3a76027

Browse files
committed
fix(gooddata-eval): detect KDA clarifying questions followed by an option list
_is_asking_kda_clarification only matched text ending literally on "?", to avoid false-positiving on a final answer that merely quotes a question elsewhere. That missed a real, common response shape: a clarifying question immediately followed by a bullet/numbered list of the options being offered (e.g. "Which metric?\n- metric A\n- metric B") -- the message doesn't end on "?" itself, so the run gave up after turn 1 instead of ever nudging the simulated user to pick one, scoring a genuinely-ambiguous case as triggered=False. Found via a real CI trace (QA-28800, gpt56luna_openai / globalmart): the chatbot asked to disambiguate between two "Total Net Revenue" metrics -- one of which was the expected answer -- but kda_disambiguated stayed False and the session never got a second turn, confirming the simulated-reply path was never reached. Fix: still match text ending on "?"; additionally match a "?" followed only by list-marker lines (nothing else after it) -- real prose after the list still means a final answer that merely enumerated something, not a still-open request. Also fixed: generate_simulated_kda_response only ever knew about measure candidates, even when the agent's clarifying question was about the PERIOD to compare instead (e.g. "Which period would you like to compare?") -- it had nothing period-specific to answer with. Now also builds a period_hint from expected_output's Date Attribute/Analyzed Period/Reference Period and passes it alongside the measure candidates, so the reply answers whichever the agent actually asked about. Tests: one end-to-end run_agentic_kda_skill test per shape -- the real captured question+option-list response, and a period-clarification response verifying the period hint reaches generate_simulated_kda_response correctly. JIRA: QA-28800
1 parent 8ad7eb3 commit 3a76027

2 files changed

Lines changed: 120 additions & 10 deletions

File tree

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

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,47 @@
2020
_DEFAULT_MAX_ITERATIONS = 3
2121

2222

23+
_LIST_ITEM_RE = re.compile(r"^\s*([-*]|\d+[.)])\s+\S")
24+
25+
2326
def _is_asking_kda_clarification(text: str) -> bool:
2427
"""True if ``text`` reads as the agent asking for input, not a final answer.
2528
2629
KDA-specific, not shared with metric_skill.py/conversation.py -- each skill's
2730
disambiguation heuristic has already drifted independently. Requires the text to
28-
end on "?" (a "?" anywhere also matches a final answer that merely quotes one).
31+
end on "?", or on "?" followed only by a bullet/numbered list of the options being
32+
offered (e.g. "Which metric?\n- a\n- b") -- a "?" anywhere else also matches a final
33+
answer that merely quotes one.
2934
"""
3035
if not text:
3136
return False
3237
t = text.strip().lower()
3338
if t.endswith("?"):
3439
return True
40+
if "?" in t:
41+
tail = t.rsplit("?", 1)[1]
42+
lines = [ln for ln in tail.splitlines() if ln.strip()]
43+
if lines and all(_LIST_ITEM_RE.match(ln) for ln in lines):
44+
return True
3545
# "To clarify, ..." means "in other words" (a final answer), not a request for one --
3646
# strip it first so "clarif" below only matches genuine clarification requests.
3747
t = re.sub(r"^(just )?to clarify,?\s*", "", t)
3848
return "could you" in t or "please provide" in t or "clarif" in t
3949

4050

41-
def generate_simulated_kda_response(agent_message: str, measure_candidates: dict | list[dict] | None) -> str:
51+
def generate_simulated_kda_response(
52+
agent_message: str,
53+
measure_candidates: dict | list[dict] | None,
54+
period_hint: str | None = None,
55+
) -> str:
4256
"""Generate a user reply to keep the KDA-skill conversation going (gpt-4o-mini).
4357
4458
Used only when the agent asks a clarifying question instead of triggering KDA
45-
directly. Picks *any* candidate from ``measure_candidates`` -- scope only needs KDA
46-
to trigger, not the resulting measure to be exactly right. Always OpenAI regardless
47-
of the combo's own provider -- this is test-harness plumbing, not the system under test.
59+
directly -- the question may be about which measure to use, which period to
60+
compare, or both, so both are given as reference and the reply answers whichever
61+
was actually asked. Scope only needs KDA to trigger, not the resulting
62+
measure/period to be exactly right. Always OpenAI regardless of the combo's own
63+
provider -- this is test-harness plumbing, not the system under test.
4864
"""
4965
try:
5066
from openai import OpenAI # noqa: PLC0415
@@ -61,11 +77,14 @@ def generate_simulated_kda_response(agent_message: str, measure_candidates: dict
6177
f"{c.get('type')} '{c.get('id')}'" + (f" (aggregation {c['aggregation']})" if c.get("aggregation") else "")
6278
for c in candidates
6379
)
80+
reference = f"an acceptable metric/fact is {candidate_desc}"
81+
if period_hint:
82+
reference += f"; the intended time period is {period_hint}"
6483
prompt = (
6584
f"You are simulating a user in a conversation with a BI assistant that runs key driver "
66-
f"analysis. The assistant said: '{agent_message}'. "
67-
f"The user is happy to proceed with any of the following: {candidate_desc}. "
68-
f"Reply briefly as the user, picking whichever of those the assistant offered."
85+
f"analysis. The assistant asked: '{agent_message}'. "
86+
f"For reference, {reference}. "
87+
f"Reply briefly as the user, answering whichever of those the assistant actually asked about."
6988
)
7089
response = client.chat.completions.create(
7190
model="gpt-4o-mini",
@@ -216,8 +235,15 @@ def _run_once(conv_id: str) -> KdaRunResult:
216235
break
217236
if _is_asking_kda_clarification(response_text):
218237
measure_candidates = expected_output.get("Measure") if isinstance(expected_output, dict) else None
238+
period_hint = None
239+
if isinstance(expected_output, dict):
240+
date_attr = expected_output.get("Date Attribute")
241+
analyzed = expected_output.get("Analyzed Period")
242+
reference_period = expected_output.get("Reference Period")
243+
if date_attr and analyzed and reference_period:
244+
period_hint = f"{date_attr}, comparing {analyzed} to {reference_period}"
219245
try:
220-
current_question = generate_simulated_kda_response(response_text, measure_candidates)
246+
current_question = generate_simulated_kda_response(response_text, measure_candidates, period_hint)
221247
disambiguated = True
222248
except Exception as exc: # noqa: BLE001 -- safety net, not the assertion; end only this run
223249
_log.warning("Simulated KDA user reply failed for conversation %s: %s", conv_id, exc)

packages/gooddata-eval/tests/test_agentic_kda_skill.py

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,90 @@ def test_run_agentic_kda_skill_marks_disambiguated_after_a_simulated_reply():
427427
assert summary.best.evaluation.triggered is True
428428

429429

430+
def test_run_agentic_kda_skill_disambiguates_on_question_followed_by_option_list():
431+
# Regression (QA-28800): the real captured response ends with a bullet list of
432+
# candidate metrics, not literally on "?" -- before the _is_asking_kda_clarification
433+
# fix, this run gave up after turn 1 (triggered=False) instead of ever nudging the
434+
# simulated user to pick one.
435+
mock_client = MagicMock()
436+
mock_client.create_conversation.return_value = "conv-1"
437+
mock_client.send_message.side_effect = [
438+
_no_kda_chat_result(
439+
'I found two different "Total Net Revenue" metrics in your data model. '
440+
"Which one should I analyze for the 2024 vs 2023 drop?\n\n"
441+
"- {metric/metric_l1_sql_net_sales_summary_net_revenue}\n"
442+
"- {metric/metric_l1_total_net_revenue}"
443+
),
444+
_kda_chat_result(success=True),
445+
]
446+
447+
with (
448+
patch("gooddata_eval.core.agentic.kda_skill.ChatClient", return_value=mock_client),
449+
patch(
450+
"gooddata_eval.core.agentic.kda_skill.generate_simulated_kda_response",
451+
return_value="Use metric_l1_sql_net_sales_summary_net_revenue.",
452+
) as mock_simulate,
453+
):
454+
summary = run_agentic_kda_skill(
455+
host="http://host/api/v1/actions/workspaces/ws1/ai",
456+
token="tok",
457+
workspace_id="ws1",
458+
question="Why did Total Net Revenue of Net Sales Summary drop in 2024 compared to 2023?",
459+
expected_output=_EXPECTED,
460+
k=1,
461+
max_iterations=2,
462+
)
463+
464+
mock_simulate.assert_called_once()
465+
assert summary.best.evaluation.disambiguated is True
466+
assert summary.best.evaluation.triggered is True
467+
assert mock_client.send_message.call_count == 2
468+
469+
470+
def test_run_agentic_kda_skill_disambiguates_on_period_clarification():
471+
# generate_simulated_kda_response used to only know about measure candidates -- if the
472+
# agent asked about the PERIOD instead, it had nothing period-specific to answer with.
473+
# Verify the period hint built from expected_output's Date Attribute/Analyzed
474+
# Period/Reference Period reaches the simulated-reply call.
475+
expected_output = {
476+
"Measure": {"type": "metric", "id": "revenue"},
477+
"Date Attribute": "transaction_date.quarter",
478+
"Analyzed Period": "2026-2",
479+
"Reference Period": "2026-1",
480+
}
481+
mock_client = MagicMock()
482+
mock_client.create_conversation.return_value = "conv-1"
483+
mock_client.send_message.side_effect = [
484+
_no_kda_chat_result("Which period would you like to compare?"),
485+
_kda_chat_result(success=True),
486+
]
487+
488+
with (
489+
patch("gooddata_eval.core.agentic.kda_skill.ChatClient", return_value=mock_client),
490+
patch(
491+
"gooddata_eval.core.agentic.kda_skill.generate_simulated_kda_response",
492+
return_value="Compare 2026-2 to 2026-1.",
493+
) as mock_simulate,
494+
):
495+
summary = run_agentic_kda_skill(
496+
host="http://host/api/v1/actions/workspaces/ws1/ai",
497+
token="tok",
498+
workspace_id="ws1",
499+
question="Why did revenue drop?",
500+
expected_output=expected_output,
501+
k=1,
502+
max_iterations=2,
503+
)
504+
505+
mock_simulate.assert_called_once_with(
506+
"Which period would you like to compare?",
507+
{"type": "metric", "id": "revenue"},
508+
"transaction_date.quarter, comparing 2026-2 to 2026-1",
509+
)
510+
assert summary.best.evaluation.disambiguated is True
511+
assert summary.best.evaluation.triggered is True
512+
513+
430514
def test_run_agentic_kda_skill_disambiguates_when_expected_output_is_not_a_dict():
431515
# DatasetItem.expected_output on the gdc-nas side allows str/list, not just dict.
432516
# expected_output.get("Measure") would raise AttributeError on those shapes, silently
@@ -457,7 +541,7 @@ def test_run_agentic_kda_skill_disambiguates_when_expected_output_is_not_a_dict(
457541
max_iterations=2,
458542
)
459543

460-
mock_generate.assert_called_once_with("Could you clarify which measure?", None)
544+
mock_generate.assert_called_once_with("Could you clarify which measure?", None, None)
461545
assert summary.best.evaluation.disambiguated is True
462546
assert summary.best.evaluation.triggered is True
463547

0 commit comments

Comments
 (0)