fix(eval): treat missing/null alert trigger as ALWAYS default

[henrynguyengooddata]

Summary

agent_alert_skill intermittently failed on trigger_correct=False for "Every time" alert cases whenever the agent created the alert without an explicit trigger. create_metric_alert serialises an unset trigger as trigger: null, so actual_args.get("trigger", "ALWAYS") returned None — the intended ALWAYS fallback only applies when the key is absent, not when it is present-but-null. Since the product default for an unset trigger is ALWAYS ("Every time"), this was a false-negative.

Fix: default a missing or null trigger to ALWAYS in both scoring paths:

• core/agentic/alert_skill.py::_check_trigger (used by the agentic runner that CI runs)
• core/evaluators/alert_skill.py::AlertSkillEvaluator (registered evaluator)

A genuinely wrong trigger (e.g. ONCE_PER_INTERVAL when ONCE is expected) still fails — the fix does not mask real model errors.

Evidence — replayed the exact actual_args from failing CI runs

Case (real args captured from CI)	Expected	Before (master)	After (this PR)
"every time returns > 150", trigger=None	pass	❌ False	✅ True
"returns by brand > 30 last day", trigger=None	pass	❌ False	✅ True
"the first time customers < 200", ONCE_PER_INTERVAL	fail	❌ False	❌ False

Both scoring paths (_check_trigger and AlertSkillEvaluator) agree. The first two rows reproduce the exact CI false-negatives and flip to pass; the third (a real model error) correctly stays failing.

Test plan

• New regression tests:
  • tests/test_agentic_alert_skill.py — _check_trigger: missing/null trigger → ALWAYS passes; a "One time" expectation still requires an explicit ONCE.
  • tests/test_alert_skill_evaluator.py — evaluator defaults missing/null trigger to ALWAYS.
• pytest packages/gooddata-eval/tests/test_agentic_alert_skill.py packages/gooddata-eval/tests/test_alert_skill_evaluator.py → 17 passed.

Downstream

gdc-nas tests/tavern-e2e/ pins gooddata-eval[llm-judge]. After this is released, a follow-up bumps the pin + re-locks uv.lock so the fix reaches the nightly agent-evaluation.

References

• Failing CI (gooddata/gdc-nas): run 28479836676 (2026-06-30), run 28902805843 (2026-07-07)
• Internal tracking: QA-28465

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved alert trigger handling so missing or empty trigger values now behave like the default “Every time” setting.
  • Ensured trigger checks consistently require an explicit one-time trigger when that option is expected.
  • Added coverage for missing and null trigger cases to prevent regressions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29a4992a-01fa-43dc-9b91-48770226eff1

📥 Commits

Reviewing files that changed from the base of the PR and between 4b2c4f3 and 6c7e272.

📒 Files selected for processing (4)

• packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py
• packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py
• packages/gooddata-eval/tests/test_agentic_alert_skill.py
• packages/gooddata-eval/tests/test_alert_skill_evaluator.py

---

📝 Walkthrough

Walkthrough

This PR updates trigger-value defaulting logic in the alert skill agentic checker and evaluator so that a missing or explicit null trigger is treated as the "ALWAYS" default, with new unit tests covering both modules.

Changes

Trigger default handling for alert skill evaluation

Layer / File(s)	Summary
Agentic trigger normalization packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py, packages/gooddata-eval/tests/test_agentic_alert_skill.py	_check_trigger derives act_trigger via or-chained get calls across trigger and triggerMode, defaulting to "ALWAYS" for both absent and explicit None values; new tests validate _check_trigger for "Every time" and "One time" expectations.
Evaluator trigger defaulting packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py, packages/gooddata-eval/tests/test_alert_skill_evaluator.py	AlertSkillEvaluator.evaluate computes actual_trigger with a fallback to "ALWAYS" when trigger is missing/null; a new test asserts trigger_correct and overall pass status for these cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A trigger gone missing, a null in the mix,
No worries, this rabbit knows all the right tricks 🐇
"ALWAYS" it whispers, when none can be found,
With tests hopping in to make the logic sound!
Click-click go my paws on the keyboard tonight.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: handling missing or null alert triggers as the ALWAYS default.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.85%. Comparing base (4b2c4f3) to head (6c7e272).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1683      +/-   ##
==========================================
+ Coverage   77.80%   77.85%   +0.04%     
==========================================
  Files         271      271              
  Lines       18602    18603       +1     
==========================================
+ Hits        14474    14483       +9     
+ Misses       4128     4120       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hkad98]

Suggested change

	actual_trigger = args.get("trigger") or "ALWAYS"
	actual_trigger = args.get("trigger","ALWAYS")

[henrynguyengooddata]

Thanks @hkad98! I used or "ALWAYS" deliberately instead of .get("trigger", "ALWAYS"). .get(key, default) only falls back when the key is absent, but gpt-5.2 calls create_metric_alert with "trigger": null — the key is present with a null value (parsed_arguments() is a raw json.loads, no schema merge). So args.get("trigger", "ALWAYS") returns None, and None == "ALWAYS" → trigger_correct=False, i.e. the exact false-negative this PR fixes would come back.

For the real failing args {"trigger": None, ...}:

• args.get("trigger", "ALWAYS") → None → fails
• args.get("trigger") or "ALWAYS" → "ALWAYS" → passes

(The original agentic _check_trigger already used .get("trigger", …default…) and still produced the CI false-negatives for the same reason.) Since no valid trigger value is falsy (ALWAYS/ONCE/ONCE_PER_INTERVAL), or safely covers both the null and absent cases. Happy to make it more explicit if you prefer: raw = args.get("trigger"); actual_trigger = raw if raw is not None else "ALWAYS".