Skip to content

Commit acfcc1a

Browse files
authored
Merge pull request #1699 from gooddata/QA-28774-alert-proposal-fallback
fix(gooddata-eval): treat alert proposal part as confirmation signal
2 parents 0382d27 + 56cbe09 commit acfcc1a

10 files changed

Lines changed: 244 additions & 11 deletions

File tree

Dockerfile

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ WORKDIR /data
3939
# to ensure consistent dependencies
4040
COPY pyproject.toml uv.lock ./
4141

42-
# Install tox and tox-uv as system packages so they're available globally
43-
# We use uv pip install to install packages from the tox dependency group in pyproject.toml
44-
# by reading from the lock file which ensures consistent versions
42+
# Install tox and tox-uv as system packages so they're available globally.
43+
# NOTE: `uv pip install --group` reads the group's requirements from pyproject.toml but
44+
# resolves them FRESH from the index -- it does NOT read uv.lock. Every version that must
45+
# stay fixed therefore needs an explicit bound in the group itself; in particular `uv`,
46+
# whose console script installs over the binary copied above.
4547
# Clean up dependency files after installation to reduce image size
4648
RUN set -x \
4749
&& uv pip install --system --group tox \

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,11 +311,26 @@ def _extract_alert_call(tool_call_events: list[ToolCallEvent]) -> tuple[str | No
311311
return None, {}, False
312312

313313

314-
def _is_asking_clarification(text: str) -> bool:
315-
if not text:
316-
return False
317-
t = text.lower()
318-
return "?" in t or "could you" in t or "please" in t or "clarif" in t
314+
def render_alert_proposal(proposal: dict) -> str:
315+
"""Render an alert-proposal part as the text the simulated user reacts to.
316+
317+
The alert skill's confirmation step deliberately emits no text part (GDAI-2032) — the
318+
prompt and the CTA live only in the proposal payload, which the frontend renders as a
319+
widget. Dumping the payload (rather than prose) keeps recipients, condition, trigger and
320+
dashboard visible so the simulated user can still verify them against its goal, and does
321+
not need updating whenever ``AlertProposal`` grows a field.
322+
"""
323+
cta = proposal.get("cta") or "Should I create this alert?"
324+
summary = {k: v for k, v in proposal.items() if k != "cta"}
325+
alert = dict(summary.get("alert") or {})
326+
# The AFM execution block is opaque wire dicts — noise that would crowd out the fields
327+
# the simulated user actually has to check.
328+
alert.pop("execution", None)
329+
if "alert" in summary:
330+
# Key off presence, not truthiness: an alert whose only key was `execution` must
331+
# still be replaced, otherwise the original (execution-bearing) dict survives.
332+
summary["alert"] = alert
333+
return f"{cta}\n\nAlert proposal:\n{json.dumps(summary, indent=2, sort_keys=True)}"
319334

320335

321336
def run_agentic_alert_skill(
@@ -352,6 +367,8 @@ def _run_once(conv_id: str) -> AlertRunResult:
352367
alert_id_to_delete = alert_id
353368
break
354369
response_text = (chat_result.text_response or "").strip()
370+
if not response_text and chat_result.alert_proposals:
371+
response_text = render_alert_proposal(chat_result.alert_proposals[-1])
355372
# Stop if agent gave a completely empty response (stuck)
356373
if not response_text and not chat_result.tool_call_events:
357374
break

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from gooddata_sdk import GoodDataSdk
1212
from pydantic import BaseModel
1313

14+
from gooddata_eval.core.agentic.alert_skill import render_alert_proposal
1415
from gooddata_eval.core.agentic.metric_skill import _delete_metric, _extract_created_metric_ids
1516
from gooddata_eval.core.chat.sse_client import ChatClient
1617
from gooddata_eval.core.models import ChatResult, ToolCallEvent
@@ -322,7 +323,10 @@ def run_agentic_conversation(
322323
break
323324

324325
response_text = (chat_result.text_response or "").strip()
325-
if _is_asking_clarification(response_text) and clarification_turns < max_clarification_turns:
326+
if not response_text and chat_result.alert_proposals:
327+
response_text = render_alert_proposal(chat_result.alert_proposals[-1])
328+
asking = _is_asking_clarification(response_text) or bool(chat_result.alert_proposals)
329+
if asking and clarification_turns < max_clarification_turns:
326330
clarification_turns += 1
327331
total_clarification_turns += 1
328332
current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)

packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class _SseAccumulator:
102102
text_parts: list[str] = field(default_factory=list)
103103
viz_reasoning_parts: list[str] = field(default_factory=list)
104104
visualizations: list[dict[str, Any]] = field(default_factory=list)
105+
alert_proposals: list[dict[str, Any]] = field(default_factory=list)
105106
tool_call_events: list[dict[str, Any]] = field(default_factory=list)
106107
call_id_to_event_index: dict[str, int] = field(default_factory=dict)
107108
reasoning_steps: list[dict[str, Any]] = field(default_factory=list)
@@ -125,6 +126,11 @@ def _handle_multipart(content: dict[str, Any], acc: _SseAccumulator) -> None:
125126
acc.viz_reasoning_parts.append(t)
126127
elif ptype == "visualization" and part.get("visualization"):
127128
acc.visualizations.append(part["visualization"])
129+
elif ptype == "alertProposal":
130+
# Record the part even when the server could not resolve the proposal payload
131+
# (``alertProposal: null``) — its mere presence is the confirmation signal, and
132+
# the reader falls back to a default CTA.
133+
acc.alert_proposals.append(part.get("alertProposal") or {})
128134

129135

130136
def _handle_reasoning(content: dict[str, Any], acc: _SseAccumulator) -> None:
@@ -161,6 +167,7 @@ def _handle_tool_result(content: dict[str, Any], acc: _SseAccumulator) -> None:
161167
def _build_chat_result(acc: _SseAccumulator) -> ChatResult:
162168
payload: dict[str, Any] = {
163169
"textResponse": "\n".join(acc.text_parts) or None,
170+
"alertProposals": acc.alert_proposals,
164171
"toolCallEvents": acc.tool_call_events,
165172
"reasoningStepCount": len(acc.reasoning_steps),
166173
}

packages/gooddata-eval/src/gooddata_eval/core/models.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ class ChatResult(BaseModel):
9292

9393
text_response: str | None = Field(default=None, alias="textResponse")
9494
created_visualizations: CreatedVisualizations | None = Field(default=None, alias="createdVisualizations")
95+
# Alert-proposal parts of the agent's multipart response. The alert skill's confirmation
96+
# step emits ONLY this part (no text part), so its `cta` is the only "the agent is asking
97+
# a question" signal the simulated-user loops can key off.
98+
alert_proposals: list[dict] = Field(default_factory=list, alias="alertProposals")
9599
tool_call_events: list[ToolCallEvent] = Field(default_factory=list, alias="toolCallEvents")
96100
reasoning_step_count: int = Field(default=0, alias="reasoningStepCount")
97101
conversation_id: str | None = Field(default=None, alias="conversationId")

packages/gooddata-eval/tests/test_agentic_alert_skill.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,23 @@
88
_deep_subset,
99
_normalize_expected_output,
1010
_to_number,
11+
render_alert_proposal,
1112
run_agentic_alert_skill,
1213
)
1314
from gooddata_eval.core.models import ChatResult
1415

16+
_PROPOSAL = {
17+
"title": "# of Orders Alert - Greater Than 500",
18+
"cta": "Should I create this alert?",
19+
"recipients": [{"email": "admin@gooddata.com"}],
20+
"dashboard": {"id": "dash-1", "title": "Orders overview"},
21+
"alert": {
22+
"trigger": "ALWAYS",
23+
"condition": {"comparison": {"operator": "GREATER_THAN", "right": {"value": 500}}},
24+
"execution": {"measures": [{"opaque": "afm"}]},
25+
},
26+
}
27+
1528

1629
def test_to_number_int():
1730
assert _to_number("42") == 42
@@ -156,3 +169,82 @@ def test_run_agentic_alert_skill_creates_fresh_conversations_for_remaining_runs(
156169
)
157170
assert mock_client.create_conversation.call_count == 2
158171
assert mock_client.delete_conversation.call_count == 2
172+
173+
174+
def test_render_alert_proposal_keeps_verifiable_fields_and_drops_afm():
175+
rendered = render_alert_proposal(_PROPOSAL)
176+
# The CTA leads so the simulated user reads it as a question.
177+
assert rendered.startswith("Should I create this alert?")
178+
# Rule 3 of the sim-user prompt requires verifying recipients against its goal.
179+
assert "admin@gooddata.com" in rendered
180+
assert "GREATER_THAN" in rendered
181+
assert "Orders overview" in rendered
182+
# Opaque AFM wire dicts must not crowd out the fields above.
183+
assert "execution" not in rendered
184+
185+
186+
def test_render_alert_proposal_drops_afm_when_execution_is_the_only_alert_field():
187+
# Truthiness-gated replacement used to leave the original execution-bearing dict in place.
188+
rendered = render_alert_proposal({"alert": {"execution": {"measures": [{"opaque": "afm"}]}}})
189+
assert "execution" not in rendered
190+
assert "opaque" not in rendered
191+
192+
193+
def test_render_alert_proposal_falls_back_to_default_cta():
194+
assert render_alert_proposal({}).startswith("Should I create this alert?")
195+
196+
197+
def test_run_agentic_alert_skill_answers_proposal_only_confirmation_turn():
198+
"""GDAI-2032 regression: confirmation turn has no text part, only an alertProposal.
199+
200+
Without the fallback the simulated user is handed an empty agent message, so the agent
201+
never receives an explicit "yes" and create_metric_alert is never called.
202+
"""
203+
proposal_turn = ChatResult.model_validate(
204+
{
205+
"text_response": None,
206+
"alertProposals": [_PROPOSAL],
207+
"tool_call_events": [
208+
{"functionName": "prepare_metric_alert_proposal", "functionArguments": "{}", "result": None}
209+
],
210+
}
211+
)
212+
created_turn = ChatResult.model_validate(
213+
{
214+
"text_response": "Alert created.",
215+
"tool_call_events": [
216+
{
217+
"functionName": "create_metric_alert",
218+
"functionArguments": '{"operator": "GREATER_THAN", "threshold": 500}',
219+
"result": '{"id": "alert-1"}',
220+
}
221+
],
222+
}
223+
)
224+
mock_client = MagicMock()
225+
mock_client.send_message.side_effect = [proposal_turn, created_turn]
226+
227+
with (
228+
patch("gooddata_eval.core.agentic.alert_skill.ChatClient", return_value=mock_client),
229+
patch(
230+
"gooddata_eval.core.agentic.alert_skill.generate_simulated_alert_response",
231+
return_value="Yes, please proceed to create the alert.",
232+
) as mock_sim,
233+
patch("gooddata_eval.core.agentic.alert_skill._delete_alert"),
234+
):
235+
summary = run_agentic_alert_skill(
236+
host="http://host",
237+
token="tok",
238+
workspace_id="ws1",
239+
question="Notify me whenever the number of orders goes above 500",
240+
expected_output={"operator": "GREATER_THAN", "threshold": 500},
241+
k=1,
242+
max_iterations=6,
243+
initial_conversation_id="conv-1",
244+
)
245+
246+
agent_message = mock_sim.call_args.args[0]
247+
assert "Should I create this alert?" in agent_message
248+
assert "admin@gooddata.com" in agent_message
249+
assert summary.best.eval.alert_created is True
250+
assert summary.best.alert_id == "alert-1"

packages/gooddata-eval/tests/test_agentic_conversation.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
_resolve_refs,
1111
run_agentic_conversation,
1212
)
13-
from gooddata_eval.core.models import ToolCallEvent
13+
from gooddata_eval.core.models import ChatResult, ToolCallEvent
1414

1515

1616
def _skills_tc(*skills):
@@ -291,3 +291,64 @@ def test_run_agentic_conversation_deletes_metrics_even_when_a_later_turn_raises(
291291
)
292292

293293
mock_sdk._client.entities_api.delete_entity_metrics.assert_called_once_with("ws1", "m1")
294+
295+
296+
def _alert_turn_fixture():
297+
return ConversationFixture(
298+
id="conv-alert",
299+
expected_skills=["alert"],
300+
turns=[
301+
TurnDefinition(
302+
turn_id="create_alert",
303+
message="Now alert me when the metric drops below 100.",
304+
expected_skill="alert",
305+
expected_output_type="tool_call",
306+
expected_tool_name="create_metric_alert",
307+
)
308+
],
309+
)
310+
311+
312+
def test_run_agentic_conversation_treats_alert_proposal_as_a_clarification():
313+
"""GDAI-2032 regression: a proposal-only turn has no text, so the old text-only check
314+
stopped the turn instead of replying, and create_metric_alert never happened."""
315+
proposal_turn = ChatResult.model_validate(
316+
{
317+
"text_response": None,
318+
"alertProposals": [{"cta": "Should I create this alert?", "recipients": [{"email": "a@b.com"}]}],
319+
"toolCallEvents": [
320+
{"functionName": "set_skills", "functionArguments": '{"skills": ["alert"]}', "result": None},
321+
{"functionName": "prepare_metric_alert_proposal", "functionArguments": "{}", "result": None},
322+
],
323+
}
324+
)
325+
created_turn = ChatResult.model_validate(
326+
{
327+
"text_response": "Alert created.",
328+
"toolCallEvents": [
329+
{"functionName": "create_metric_alert", "functionArguments": "{}", "result": '{"id": "alert-1"}'}
330+
],
331+
}
332+
)
333+
mock_client = MagicMock()
334+
mock_client.create_conversation.return_value = "conv-1"
335+
mock_client.send_message.side_effect = [proposal_turn, created_turn]
336+
337+
with (
338+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
339+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
340+
patch(
341+
"gooddata_eval.core.agentic.conversation._get_sim_user_response",
342+
return_value="Yes, please create it.",
343+
) as mock_sim,
344+
):
345+
result = run_agentic_conversation(
346+
host="http://host/api/v1/actions/workspaces/ws1/ai",
347+
token="tok",
348+
workspace_id="ws1",
349+
fixture=_alert_turn_fixture(),
350+
)
351+
352+
assert "Should I create this alert?" in mock_sim.call_args.args[0]
353+
assert result.turn_results[0].clarification_turns_used == 1
354+
assert result.turn_results[0].skill_success is True

packages/gooddata-eval/tests/test_sse_client.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,44 @@ def test_parse_sse_lines_prefers_multipart_viz_over_adhoc_fallback():
8282
assert result.created_visualizations.objects[0].id == "real"
8383

8484

85+
def test_parse_sse_lines_collects_alert_proposal_without_text_part():
86+
"""The alert skill's confirmation turn emits ONLY an alertProposal part (GDAI-2032).
87+
88+
Pins the wire contract the simulated-user loops depend on: part ``type`` is
89+
``alertProposal`` and the payload lives under the ``alertProposal`` key.
90+
"""
91+
proposal = {
92+
"title": "# of Orders Alert - Greater Than 500",
93+
"cta": "Should I create this alert?",
94+
"recipients": [{"email": "admin@gooddata.com"}],
95+
"alert": {"trigger": "ALWAYS", "execution": {"measures": [{"opaque": "afm"}]}},
96+
}
97+
lines = [
98+
'data: {"item": {"role": "assistant", "content": {"type": "toolCall", "callId": "c1", '
99+
'"name": "prepare_metric_alert_proposal", "arguments": {}}}}',
100+
f'data: {{"item": {{"role": "assistant", "content": {{"type": "multipart", '
101+
f'"parts": [{{"type": "alertProposal", "alertProposal": {json.dumps(proposal)}}}]}}}}}}',
102+
]
103+
result = parse_sse_lines(lines)
104+
assert result.text_response is None
105+
assert result.alert_proposals == [proposal]
106+
107+
108+
def test_parse_sse_lines_keeps_alert_proposal_part_when_payload_is_null():
109+
"""Presence of the part is the confirmation signal even if the server did not resolve it."""
110+
lines = [
111+
'data: {"item": {"role": "assistant", "content": {"type": "multipart", '
112+
'"parts": [{"type": "alertProposal", "alertProposal": null}]}}}',
113+
]
114+
result = parse_sse_lines(lines)
115+
assert result.alert_proposals == [{}]
116+
117+
118+
def test_parse_sse_lines_has_no_alert_proposals_by_default():
119+
lines = ['data: {"item": {"role": "assistant", "content": {"type": "text", "text": "Done"}}}']
120+
assert parse_sse_lines(lines).alert_proposals == []
121+
122+
85123
@pytest.mark.parametrize("code", [429, 502, 503, 504])
86124
def test_parse_sse_lines_transient_status_codes(code):
87125
with pytest.raises(TransientChatError) as ei:

pyproject.toml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,13 @@ release = [
7676
]
7777
tox = [
7878
"tox~=4.56.1",
79-
"tox-uv~=1.35.2"
79+
"tox-uv~=1.35.2",
80+
# tox-uv depends on the uv PyPI package without a version bound, and the Dockerfile
81+
# installs this group with `uv pip install`, which resolves fresh instead of reading
82+
# uv.lock. Without this bound the resolver picks the newest uv, whose console script
83+
# then shadows the pinned binary in the image and trips required-version at runtime.
84+
# Keep in sync with [tool.uv] required-version above.
85+
"uv~=0.11.0",
8086
]
8187

8288
[tool.ruff]

uv.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)