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 1b4ecf465..c3a9df936 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 @@ -78,7 +78,11 @@ def _check_trigger(expected: CatalogMetricAlert, actual_args: dict) -> bool: if expected.operator == "ANOMALY": return True exp_trigger = expected.trigger - act_trigger = actual_args.get("trigger", actual_args.get("triggerMode", "ALWAYS")) + # A missing OR explicit-null trigger means "unset" -> the product persists the + # default ALWAYS ("Every time"). `.get(k, default)` only returns the default when + # the key is ABSENT, but create_metric_alert serialises unset params as + # `trigger: null`, so chain with `or` to also cover the present-but-None case. + act_trigger = actual_args.get("trigger") or actual_args.get("triggerMode") or "ALWAYS" if exp_trigger in _ALWAYS_TRIGGER_VALUES: return act_trigger in {"ALWAYS", "Every time"} act_api = _TRIGGER_DISPLAY_TO_API.get(act_trigger, act_trigger) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py index 900005351..3933e7dc4 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py @@ -78,7 +78,9 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation if "Trigger" in expected: expected_trigger = _TRIGGER_MAP.get(expected["Trigger"], expected["Trigger"]) - trigger_correct = args.get("trigger") == expected_trigger + # Omitted/null trigger persists as the product default ALWAYS ("Every time"). + actual_trigger = args.get("trigger") or "ALWAYS" + trigger_correct = actual_trigger == expected_trigger if "Filters" in expected: actual_filters = args.get("filters") or [] diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index 223949d3b..d6026da04 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -4,7 +4,9 @@ from gooddata_eval.core.agentic.alert_skill import ( AlertEvaluation, + _check_trigger, _deep_subset, + _normalize_expected_output, _to_number, run_agentic_alert_skill, ) @@ -31,6 +33,24 @@ def test_deep_subset_missing_key(): assert _deep_subset({"a": 1, "c": 3}, {"a": 1}) is False +def test_check_trigger_missing_or_null_defaults_to_always(): + # "Every time" is the product default -> the agent may omit the trigger arg + # entirely, or serialise it as null. Both must count as ALWAYS, not a mismatch. + expected = _normalize_expected_output({"Operator": "GREATER_THAN", "Trigger": "Every time"}) + assert _check_trigger(expected, {"operator": "GREATER_THAN"}) is True # key absent + assert _check_trigger(expected, {"trigger": None}) is True # present-but-null (the bug) + assert _check_trigger(expected, {"trigger": "ALWAYS"}) is True + + +def test_check_trigger_once_needs_explicit_once(): + # A "One time" expectation must still require an explicit ONCE - the null-default + # fix must not turn a wrong/absent trigger into a pass here. + expected = _normalize_expected_output({"Operator": "LESS_THAN", "Trigger": "One time"}) + assert _check_trigger(expected, {"trigger": "ONCE"}) is True + assert _check_trigger(expected, {"trigger": None}) is False # null != ONCE + assert _check_trigger(expected, {"trigger": "ONCE_PER_INTERVAL"}) is False # real model error stays a fail + + def test_alert_evaluation_strict_pass(): ev = AlertEvaluation( alert_created=True, diff --git a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py index 23dcd67fc..6397220a8 100644 --- a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py +++ b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py @@ -58,6 +58,23 @@ def test_alert_evaluator_skips_absent_field(): assert result.passed is True +def test_alert_evaluator_defaults_missing_trigger_to_always(): + # The agent may omit `trigger` (or send null) -> backend default is ALWAYS, + # so an "Every time" expectation must still pass instead of a false-negative. + expected = {"Operator": "GREATER_THAN", "Threshold": "150", "Trigger": "Every time"} + ev = get_evaluator("alert_skill") + + absent = ev.evaluate(_item(expected), _chat_with_alert({"operator": "GREATER_THAN", "threshold": 150})) + assert absent.passed is True + assert absent.detail["trigger_correct"] is True + + null = ev.evaluate( + _item(expected), _chat_with_alert({"operator": "GREATER_THAN", "threshold": 150, "trigger": None}) + ) + assert null.passed is True + assert null.detail["trigger_correct"] is True + + def test_alert_evaluator_fails_when_no_tool_call(): result = get_evaluator("alert_skill").evaluate( _item({"Operator": "LESS_THAN"}), ChatResult.model_validate({"textResponse": "here is the alert"})