diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py index 3d3adb15a..b8415169d 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py @@ -25,6 +25,17 @@ _TRIGGER_DISPLAY_TO_API = {"Every time": "ALWAYS", "One time": "ONCE"} _ALWAYS_TRIGGER_VALUES = {"Every time", "ALWAYS", "not specified"} +_TRIGGER_UNSPECIFIED = "not specified" +_RANGE_OPERATORS = ("BETWEEN", "NOT_BETWEEN") + +# Plain-language phrasing of each expected trigger, for the sim-user to demand out loud. +_TRIGGER_INSTRUCTIONS = { + "ALWAYS": ( + "alert me EVERY TIME the condition is met — not once per day, week, month, quarter or year, " + "and do not set any trigger interval" + ), + "ONCE": "alert me ONLY THE FIRST TIME the condition is met, then stop", +} def _to_number(value: object) -> float | int | None: @@ -65,7 +76,7 @@ def _deep_subset(expected: object, actual: object) -> bool: def _check_threshold(expected: CatalogMetricAlert, actual_args: dict) -> bool: - if expected.operator in ("BETWEEN", "NOT_BETWEEN"): + if expected.operator in _RANGE_OPERATORS: exp_from = _to_number(expected.threshold_from) exp_to = _to_number(expected.threshold_to) act_from = _to_number(actual_args.get("from_value", actual_args.get("fromValue"))) @@ -109,9 +120,8 @@ def _check_metric(expected: CatalogMetricAlert, actual_args: dict) -> bool: return expected.metric_id == act_metric -def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool: - if not expected.recipients: - return True +def _actual_recipients(actual_args: dict) -> list: + """Recipient list from the tool arguments, whatever shape the agent used.""" act_recip_raw = actual_args.get("recipients", actual_args.get("external_recipients")) if isinstance(act_recip_raw, str): # external_recipients is JSON-encoded (e.g. '["email@example.com"]') @@ -124,7 +134,49 @@ def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool: act_recip = act_recip_raw else: act_recip = [] - return set(expected.recipients) == set(act_recip or []) + return act_recip or [] + + +def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool: + if not expected.recipients: + return True + return set(expected.recipients) == set(_actual_recipients(actual_args)) + + +def _trigger_instruction(expected: CatalogMetricAlert) -> str: + """What the sim-user must demand about the trigger, or '' when it should stay silent. + + Stated for every explicit trigger, ALWAYS included: the product prompt tells the agent to + align `trigger_interval` with any date granularity in play, so an ALWAYS expectation only + survives when the user asks for it out loud. Silent when the fixture omits Trigger (so those + cases still exercise the product default) and for ANOMALY, whose product default really is + ONCE_PER_INTERVAL and whose trigger the assertion does not check. + """ + if expected.operator == "ANOMALY" or expected.trigger == _TRIGGER_UNSPECIFIED: + return "" + return _TRIGGER_INSTRUCTIONS.get(expected.trigger, f"set the trigger to '{expected.trigger}'") + + +def _filter_rule(filters: list | str | None) -> str: + """Rule keeping the sim-user from accepting filters the fixture did not ask for. + + The agent volunteers a date filter for vague questions and then aligns the trigger interval + to it, which is how an expected-ALWAYS case drifts to ONCE_PER_INTERVAL. Refusing the extra + filter removes the reason the agent had to change the trigger. + """ + if filters: + return ( + f"The ONLY filters this alert may have are: {filters}. Do NOT accept any additional " + "filter the agent proposes — in particular no extra date, time-window or period " + "filter. If its proposal or summary adds one, tell it to drop the extra filter and " + "keep only the filters listed here." + ) + return ( + "This alert must have NO filters at all. If the agent asks about a time window, date " + "range or period, answer 'All time — no date range filter'. Do NOT accept any date, " + "time-window or period filter the agent proposes; if its proposal or summary adds one, " + "tell it to drop the filter." + ) def generate_simulated_alert_response( @@ -151,24 +203,26 @@ def generate_simulated_alert_response( trigger = expected.trigger filters = expected.filters - trigger_line = ( - f"5. Proactively tell the agent the trigger is '{trigger}' in your first reply.\n" - if trigger not in _ALWAYS_TRIGGER_VALUES - else "" - ) + rules = [ + f"Your goal: metric={metric}, operator={operator}, threshold={threshold}, " + f"recipients={recipients}, trigger={trigger}" + (f", filters={filters}" if filters else "") + ".", + "Never revert or change a decision that was already confirmed in a previous turn.", + "If the agent shows a proposal or final summary and asks for confirmation, verify that the " + "recipients, the trigger AND the filters all match your goal. Correct every mismatch in " + "your reply. Only once all three are correct, say 'Yes, please proceed to create the alert.'", + "Proactively include your email recipient in your first reply. Do not wait for the agent " + "to ask — state it alongside the metric and condition answers.", + ] + trigger_instruction = _trigger_instruction(expected) + if trigger_instruction: + rules.append(f"Proactively tell the agent in your first reply: {trigger_instruction}.") + rules.append(_filter_rule(filters)) + system_prompt = ( "You are a user requesting creation of an alert for a metric from an AI agent. " "Respond naturally but always steer toward the exact values you were given.\n" "Rules you MUST follow:\n" - f"1. Your goal: metric={metric}, operator={operator}, threshold={threshold}, " - f"recipients={recipients}, trigger={trigger}" + (f", filters={filters}" if filters else "") + ".\n" - "2. Never revert or change a decision that was already confirmed in a previous turn.\n" - "3. If the agent shows a final summary and asks for confirmation, verify that the " - " recipients match your goal. If they differ, correct them. " - " Once recipients are correct, say 'Yes, please proceed to create the alert.'\n" - "4. Proactively include your email recipient in your first reply. " - " Do not wait for the agent to ask — state it alongside the metric and condition answers.\n" - + trigger_line + + "".join(f"{i}. {rule}\n" for i, rule in enumerate(rules, start=1)) + "Reply concisely and directly." ) @@ -180,7 +234,9 @@ def generate_simulated_alert_response( response = openai_client.chat.completions.create( model="gpt-4o", messages=messages, - temperature=0.5, + # Deterministic as the model allows: the trigger/filter drift this prompt guards against + # was sampling-dependent, so temperature must not reintroduce it. + temperature=0, ) return response.choices[0].message.content or "" @@ -424,6 +480,48 @@ def _run_once(conv_id: str) -> AlertRunResult: ) +def _actual_trigger_display(actual_args: dict) -> str: + """Trigger as the agent set it, with the interval appended when one applies.""" + trigger = actual_args.get("trigger") or actual_args.get("triggerMode") or "ALWAYS" + interval = actual_args.get("trigger_interval") or actual_args.get("triggerInterval") + return f"{trigger}/{interval}" if interval else str(trigger) + + +def _score_comments(expected: CatalogMetricAlert, actual_args: dict, alert_created: bool) -> dict[str, str]: + """Expected-vs-actual detail per strict check, for the Langfuse score comments. + + Without these a failed check shows only 0.0 on the trace, and triaging means reading CI logs + or hunting the create_metric_alert arguments through nested trace observations. + """ + if not alert_created: + return {"alert_created": "create_metric_alert was never called"} + + if expected.operator in _RANGE_OPERATORS: + exp_threshold: object = f"{expected.threshold_from}..{expected.threshold_to}" + act_threshold: object = ( + f"{actual_args.get('from_value', actual_args.get('fromValue'))}.." + f"{actual_args.get('to_value', actual_args.get('toValue'))}" + ) + else: + exp_threshold = expected.threshold + act_threshold = actual_args.get("threshold") + + act_metric_raw = str(actual_args.get("metric_id", actual_args.get("metricId", ""))) + act_metric = _parse_metric_id(act_metric_raw) or act_metric_raw + return { + "alert_created": "create_metric_alert was called", + "operator_correct": f"expected {expected.operator!r}; actual {actual_args.get('operator')!r}", + "threshold_correct": f"expected {exp_threshold!r}; actual {act_threshold!r}", + "trigger_correct": f"expected {expected.trigger!r}; actual {_actual_trigger_display(actual_args)!r}", + "filters_correct": ( + f"expected {expected.filters!r}; " + f"actual {actual_args.get('filters', actual_args.get('attribute_filters'))!r}" + ), + "metric_correct": f"expected {expected.metric_id!r}; actual {act_metric!r}", + "recipients_correct": f"expected {expected.recipients!r}; actual {_actual_recipients(actual_args)!r}", + } + + class AlertSkillAssertionError(AssertionError): """Raised when an alert-skill evaluation fails.""" @@ -489,6 +587,7 @@ def evaluate_agentic_alert_skill( [r.conversation_id for r in summary.run_results], window_start, ) + expected = _normalize_expected_output(expected_output) suffix_needed = len(summary.run_results) > 1 for run_idx, run in enumerate(summary.run_results): pt = traces_by_conv.get(run.conversation_id) @@ -503,9 +602,17 @@ def evaluate_agentic_alert_skill( "metric_correct": ev.metric_correct, "recipients_correct": ev.recipients_correct, } + comments = _score_comments(expected, run.actual_alert_arguments, ev.alert_created) with observe(langfuse, pt.id if pt else None, dataset_item_id, run_name, run_metadata) as tid: for score_name, value in strict_checks.items(): - score_safe(langfuse, tid, name=score_name, value=float(value), data_type="BOOLEAN") + score_safe( + langfuse, + tid, + name=score_name, + value=float(value), + data_type="BOOLEAN", + comment=comments.get(score_name), + ) log_quality_and_value_scores( langfuse, tid, diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index d6026da04..7691cae41 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -1,18 +1,37 @@ # (C) 2026 GoodData Corporation. All rights reserved. # SPDX-License-Identifier: LicenseRef-GoodData-Enterprise +import json from unittest.mock import MagicMock, patch +import pytest from gooddata_eval.core.agentic.alert_skill import ( AlertEvaluation, + AlertSkillAssertionError, _check_trigger, _deep_subset, _normalize_expected_output, + _score_comments, _to_number, + evaluate_agentic_alert_skill, + generate_simulated_alert_response, run_agentic_alert_skill, ) from gooddata_eval.core.models import ChatResult +def _sim_user_prompt(expected_output: dict) -> tuple[str, dict]: + """Render the sim-user system prompt for a fixture, plus the kwargs sent to gpt-4o.""" + mock_openai = MagicMock() + mock_openai.return_value.chat.completions.create.return_value.choices = [MagicMock(message=MagicMock(content="ok"))] + with ( + patch("gooddata_eval.core.agentic.alert_skill._OpenAI", mock_openai), + patch.dict("os.environ", {"OPENAI_API_KEY": "test-key"}), + ): + generate_simulated_alert_response("Which metric?", _normalize_expected_output(expected_output), []) + kwargs = mock_openai.return_value.chat.completions.create.call_args.kwargs + return kwargs["messages"][0]["content"], kwargs + + def test_to_number_int(): assert _to_number("42") == 42 @@ -51,6 +70,156 @@ def test_check_trigger_once_needs_explicit_once(): assert _check_trigger(expected, {"trigger": "ONCE_PER_INTERVAL"}) is False # real model error stays a fail +def test_sim_user_states_always_trigger_proactively(): + # QA-28623: an expected-ALWAYS trigger used to be left unsaid, so the agent aligned + # trigger_interval with the date granularity and the sim-user accepted ONCE_PER_INTERVAL. + prompt, _ = _sim_user_prompt({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + assert "EVERY TIME" in prompt + assert "do not set any trigger interval" in prompt + + +def test_sim_user_states_once_trigger_proactively(): + prompt, _ = _sim_user_prompt({"Operator": "LESS_THAN", "Trigger": "One time"}) + assert "ONLY THE FIRST TIME" in prompt + + +def test_sim_user_silent_on_trigger_when_fixture_omits_it(): + # No Trigger key -> the case is meant to exercise the product default; stating one would + # turn it into a different test. + prompt, _ = _sim_user_prompt({"Operator": "GREATER_THAN", "Threshold": 100}) + assert "EVERY TIME" not in prompt + assert "trigger to" not in prompt + + +def test_sim_user_silent_on_trigger_for_anomaly(): + # ANOMALY legitimately defaults to ONCE_PER_INTERVAL and _check_trigger skips it. + prompt, _ = _sim_user_prompt({"Operator": "ANOMALY", "Trigger": "ONCE_PER_INTERVAL"}) + assert "EVERY TIME" not in prompt + + +def test_sim_user_refuses_extra_filters_when_none_expected(): + prompt, _ = _sim_user_prompt({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + assert "NO filters at all" in prompt + assert "All time" in prompt + + +def test_sim_user_refuses_extra_filters_beyond_expected_list(): + filters = [{"positiveAttributeFilter": {"label": {"identifier": {"id": "product_category"}}}}] + prompt, _ = _sim_user_prompt({"Operator": "LESS_THAN", "Trigger": "Every time", "Filters": filters}) + assert "ONLY filters this alert may have" in prompt + assert "product_category" in prompt + + +def test_sim_user_verifies_trigger_and_filters_at_confirmation(): + # Regression guard for QA-28113: rule 3 must not shrink back to recipients-only. + prompt, _ = _sim_user_prompt({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + assert "recipients, the trigger AND the filters" in prompt + + +def test_sim_user_rules_are_numbered_without_gaps(): + prompt, _ = _sim_user_prompt({"Operator": "ANOMALY", "Trigger": "ONCE_PER_INTERVAL"}) + numbers = [int(line.split(".", 1)[0]) for line in prompt.splitlines() if line[:1].isdigit()] + assert numbers == list(range(1, len(numbers) + 1)) + + +def test_sim_user_uses_deterministic_temperature(): + _, kwargs = _sim_user_prompt({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + assert kwargs["temperature"] == 0 + + +def test_score_comments_expose_trigger_and_filters(): + # QA-28623: a 0.0 score alone forced triage through CI logs / nested trace observations. + expected = _normalize_expected_output( + {"Operator": "GREATER_THAN", "Trigger": "Every time", "Threshold": 5000, "Metric": "Total Discounts (td)"} + ) + actual = { + "operator": "GREATER_THAN", + "threshold": 5000, + "trigger": "ONCE_PER_INTERVAL", + "trigger_interval": "DAY", + "filters": [{"relativeDateFilter": {"granularity": "DAY"}}], + "metric_id": "td", + "external_recipients": '["admin@gooddata.com"]', + } + comments = _score_comments(expected, actual, alert_created=True) + assert "'ALWAYS'" in comments["trigger_correct"] + assert "'ONCE_PER_INTERVAL/DAY'" in comments["trigger_correct"] + assert "relativeDateFilter" in comments["filters_correct"] + assert "admin@gooddata.com" in comments["recipients_correct"] + + +def test_score_comments_range_operator_reports_bounds(): + expected = _normalize_expected_output( + {"Operator": "BETWEEN", "threshold_from": 50000, "threshold_to": 200000, "Trigger": "Every time"} + ) + comments = _score_comments(expected, {"from_value": 50000, "to_value": 1}, alert_created=True) + assert "50000..200000" in comments["threshold_correct"] + assert "50000..1" in comments["threshold_correct"] + + +def test_score_comments_when_alert_never_created(): + expected = _normalize_expected_output({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + comments = _score_comments(expected, {}, alert_created=False) + assert comments == {"alert_created": "create_metric_alert was never called"} + + +def test_evaluate_attaches_score_comments_to_the_trace(): + # Covers the Langfuse branch, which needs both a client and a dataset_item_id to run -- + # the other tests stop at run_agentic_alert_skill and never enter it. + fake_langfuse = MagicMock() + mock_client = MagicMock() + mock_client.send_message.return_value = ChatResult.model_validate( + { + "text_response": "Created the alert", + "created_visualizations": None, + "tool_call_events": [ + { + "functionName": "create_metric_alert", + "functionArguments": json.dumps( + { + "operator": "GREATER_THAN", + "threshold": 5000, + "trigger": "ONCE_PER_INTERVAL", + "trigger_interval": "DAY", + "filters": [{"relativeDateFilter": {"granularity": "DAY"}}], + } + ), + "result": "{}", + } + ], + "reasoning_step_count": 1, + } + ) + trace = MagicMock(id="trace-1", latency=1.0, total_cost=0.01) + + with ( + patch("gooddata_eval.core.agentic.alert_skill.ChatClient", return_value=mock_client), + patch("gooddata_eval.core.agentic.alert_skill.GoodDataSdk"), + patch("gooddata_eval.core.agentic._langfuse.build_run_context", return_value=("run-name", {})), + patch( + "gooddata_eval.core.agentic._langfuse.find_traces_per_conversation", + return_value={"existing-conv": trace}, + ), + pytest.raises(AlertSkillAssertionError), + ): + evaluate_agentic_alert_skill( + host="http://host", + token="tok", + workspace_id="ws1", + question="Alert me when discounts get out of hand", + expected_output={"Operator": "GREATER_THAN", "Threshold": 5000, "Trigger": "Every time"}, + k=1, + max_iterations=1, + initial_conversation_id="existing-conv", + langfuse=fake_langfuse, + dataset_item_id="item-1", + ) + + comments = {call.kwargs["name"]: call.kwargs.get("comment") for call in fake_langfuse.create_score.call_args_list} + assert "ONCE_PER_INTERVAL/DAY" in comments["trigger_correct"] + assert "relativeDateFilter" in comments["filters_correct"] + + def test_alert_evaluation_strict_pass(): ev = AlertEvaluation( alert_created=True,