Skip to content

Commit f8f7256

Browse files
Merge pull request #1683 from gooddata/qa-28465-alert-trigger-default-always
fix(eval): treat missing/null alert trigger as ALWAYS default
2 parents 4b2c4f3 + 6c7e272 commit f8f7256

4 files changed

Lines changed: 45 additions & 2 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ def _check_trigger(expected: CatalogMetricAlert, actual_args: dict) -> bool:
7878
if expected.operator == "ANOMALY":
7979
return True
8080
exp_trigger = expected.trigger
81-
act_trigger = actual_args.get("trigger", actual_args.get("triggerMode", "ALWAYS"))
81+
# A missing OR explicit-null trigger means "unset" -> the product persists the
82+
# default ALWAYS ("Every time"). `.get(k, default)` only returns the default when
83+
# the key is ABSENT, but create_metric_alert serialises unset params as
84+
# `trigger: null`, so chain with `or` to also cover the present-but-None case.
85+
act_trigger = actual_args.get("trigger") or actual_args.get("triggerMode") or "ALWAYS"
8286
if exp_trigger in _ALWAYS_TRIGGER_VALUES:
8387
return act_trigger in {"ALWAYS", "Every time"}
8488
act_api = _TRIGGER_DISPLAY_TO_API.get(act_trigger, act_trigger)

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation
7878

7979
if "Trigger" in expected:
8080
expected_trigger = _TRIGGER_MAP.get(expected["Trigger"], expected["Trigger"])
81-
trigger_correct = args.get("trigger") == expected_trigger
81+
# Omitted/null trigger persists as the product default ALWAYS ("Every time").
82+
actual_trigger = args.get("trigger") or "ALWAYS"
83+
trigger_correct = actual_trigger == expected_trigger
8284

8385
if "Filters" in expected:
8486
actual_filters = args.get("filters") or []

packages/gooddata-eval/tests/test_agentic_alert_skill.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55
from gooddata_eval.core.agentic.alert_skill import (
66
AlertEvaluation,
7+
_check_trigger,
78
_deep_subset,
9+
_normalize_expected_output,
810
_to_number,
911
run_agentic_alert_skill,
1012
)
@@ -31,6 +33,24 @@ def test_deep_subset_missing_key():
3133
assert _deep_subset({"a": 1, "c": 3}, {"a": 1}) is False
3234

3335

36+
def test_check_trigger_missing_or_null_defaults_to_always():
37+
# "Every time" is the product default -> the agent may omit the trigger arg
38+
# entirely, or serialise it as null. Both must count as ALWAYS, not a mismatch.
39+
expected = _normalize_expected_output({"Operator": "GREATER_THAN", "Trigger": "Every time"})
40+
assert _check_trigger(expected, {"operator": "GREATER_THAN"}) is True # key absent
41+
assert _check_trigger(expected, {"trigger": None}) is True # present-but-null (the bug)
42+
assert _check_trigger(expected, {"trigger": "ALWAYS"}) is True
43+
44+
45+
def test_check_trigger_once_needs_explicit_once():
46+
# A "One time" expectation must still require an explicit ONCE - the null-default
47+
# fix must not turn a wrong/absent trigger into a pass here.
48+
expected = _normalize_expected_output({"Operator": "LESS_THAN", "Trigger": "One time"})
49+
assert _check_trigger(expected, {"trigger": "ONCE"}) is True
50+
assert _check_trigger(expected, {"trigger": None}) is False # null != ONCE
51+
assert _check_trigger(expected, {"trigger": "ONCE_PER_INTERVAL"}) is False # real model error stays a fail
52+
53+
3454
def test_alert_evaluation_strict_pass():
3555
ev = AlertEvaluation(
3656
alert_created=True,

packages/gooddata-eval/tests/test_alert_skill_evaluator.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,23 @@ def test_alert_evaluator_skips_absent_field():
5858
assert result.passed is True
5959

6060

61+
def test_alert_evaluator_defaults_missing_trigger_to_always():
62+
# The agent may omit `trigger` (or send null) -> backend default is ALWAYS,
63+
# so an "Every time" expectation must still pass instead of a false-negative.
64+
expected = {"Operator": "GREATER_THAN", "Threshold": "150", "Trigger": "Every time"}
65+
ev = get_evaluator("alert_skill")
66+
67+
absent = ev.evaluate(_item(expected), _chat_with_alert({"operator": "GREATER_THAN", "threshold": 150}))
68+
assert absent.passed is True
69+
assert absent.detail["trigger_correct"] is True
70+
71+
null = ev.evaluate(
72+
_item(expected), _chat_with_alert({"operator": "GREATER_THAN", "threshold": 150, "trigger": None})
73+
)
74+
assert null.passed is True
75+
assert null.detail["trigger_correct"] is True
76+
77+
6178
def test_alert_evaluator_fails_when_no_tool_call():
6279
result = get_evaluator("alert_skill").evaluate(
6380
_item({"Operator": "LESS_THAN"}), ChatResult.model_validate({"textResponse": "here is the alert"})

0 commit comments

Comments
 (0)