Skip to content

Commit dbbe6c7

Browse files
committed
fix(gooddata-eval): keep alert sim-user from accepting trigger drift
The staging nightly repeatedly failed agent_alert_skill_2 and _15: the agent created ONCE_PER_INTERVAL/DAY alerts where the fixture expects ALWAYS, plus date filters nobody asked for. The chatbot was following the simulated user, whose prompt had three gaps: - The expected trigger was stated only when it differed from ALWAYS, so exactly the ALWAYS cases went unsaid. The alert prompt tells the agent to align trigger_interval with any date granularity in play "unless the user explicitly chooses a different interval", so an ALWAYS expectation only survives when the sim-user asks for it out loud. Now stated for every explicit trigger, and still silent both when the fixture omits Trigger (those cases exercise the product default) and for ANOMALY, whose product default really is ONCE_PER_INTERVAL. - The final-summary verification rule checked recipients only, so a summary showing a wrong trigger got confirmed anyway. It checked the trigger too until the logic moved into this SDK; restored, and extended to filters. - Nothing forbade extra filters. The agent volunteers a date filter for a vague question and then aligns the trigger interval to it, so refusing the filter removes the reason it had to change the trigger in the first place. Fixing the trigger rule alone would be fragile. Sampling temperature drops to 0 as well, since the drift was sampling-dependent. Each Langfuse score now carries expected-vs-actual detail. These comments also existed before the same migration and were lost with it: without them a failed trigger/filters check shows only 0.0 on the trace, and the create_metric_alert arguments have to be dug out of CI logs or of nested observations on a different trace of the session than the one the scores land on. JIRA: QA-28623 risk: nonprod
1 parent 0382d27 commit dbbe6c7

2 files changed

Lines changed: 297 additions & 21 deletions

File tree

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

Lines changed: 128 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@
2525

2626
_TRIGGER_DISPLAY_TO_API = {"Every time": "ALWAYS", "One time": "ONCE"}
2727
_ALWAYS_TRIGGER_VALUES = {"Every time", "ALWAYS", "not specified"}
28+
_TRIGGER_UNSPECIFIED = "not specified"
29+
_RANGE_OPERATORS = ("BETWEEN", "NOT_BETWEEN")
30+
31+
# Plain-language phrasing of each expected trigger, for the sim-user to demand out loud.
32+
_TRIGGER_INSTRUCTIONS = {
33+
"ALWAYS": (
34+
"alert me EVERY TIME the condition is met — not once per day, week, month, quarter or year, "
35+
"and do not set any trigger interval"
36+
),
37+
"ONCE": "alert me ONLY THE FIRST TIME the condition is met, then stop",
38+
}
2839

2940

3041
def _to_number(value: object) -> float | int | None:
@@ -65,7 +76,7 @@ def _deep_subset(expected: object, actual: object) -> bool:
6576

6677

6778
def _check_threshold(expected: CatalogMetricAlert, actual_args: dict) -> bool:
68-
if expected.operator in ("BETWEEN", "NOT_BETWEEN"):
79+
if expected.operator in _RANGE_OPERATORS:
6980
exp_from = _to_number(expected.threshold_from)
7081
exp_to = _to_number(expected.threshold_to)
7182
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:
109120
return expected.metric_id == act_metric
110121

111122

112-
def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool:
113-
if not expected.recipients:
114-
return True
123+
def _actual_recipients(actual_args: dict) -> list:
124+
"""Recipient list from the tool arguments, whatever shape the agent used."""
115125
act_recip_raw = actual_args.get("recipients", actual_args.get("external_recipients"))
116126
if isinstance(act_recip_raw, str):
117127
# external_recipients is JSON-encoded (e.g. '["email@example.com"]')
@@ -124,7 +134,49 @@ def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool:
124134
act_recip = act_recip_raw
125135
else:
126136
act_recip = []
127-
return set(expected.recipients) == set(act_recip or [])
137+
return act_recip or []
138+
139+
140+
def _check_recipients(expected: CatalogMetricAlert, actual_args: dict) -> bool:
141+
if not expected.recipients:
142+
return True
143+
return set(expected.recipients) == set(_actual_recipients(actual_args))
144+
145+
146+
def _trigger_instruction(expected: CatalogMetricAlert) -> str:
147+
"""What the sim-user must demand about the trigger, or '' when it should stay silent.
148+
149+
Stated for every explicit trigger, ALWAYS included: the product prompt tells the agent to
150+
align `trigger_interval` with any date granularity in play, so an ALWAYS expectation only
151+
survives when the user asks for it out loud. Silent when the fixture omits Trigger (so those
152+
cases still exercise the product default) and for ANOMALY, whose product default really is
153+
ONCE_PER_INTERVAL and whose trigger the assertion does not check.
154+
"""
155+
if expected.operator == "ANOMALY" or expected.trigger == _TRIGGER_UNSPECIFIED:
156+
return ""
157+
return _TRIGGER_INSTRUCTIONS.get(expected.trigger, f"set the trigger to '{expected.trigger}'")
158+
159+
160+
def _filter_rule(filters: list | str | None) -> str:
161+
"""Rule keeping the sim-user from accepting filters the fixture did not ask for.
162+
163+
The agent volunteers a date filter for vague questions and then aligns the trigger interval
164+
to it, which is how an expected-ALWAYS case drifts to ONCE_PER_INTERVAL. Refusing the extra
165+
filter removes the reason the agent had to change the trigger.
166+
"""
167+
if filters:
168+
return (
169+
f"The ONLY filters this alert may have are: {filters}. Do NOT accept any additional "
170+
"filter the agent proposes — in particular no extra date, time-window or period "
171+
"filter. If its proposal or summary adds one, tell it to drop the extra filter and "
172+
"keep only the filters listed here."
173+
)
174+
return (
175+
"This alert must have NO filters at all. If the agent asks about a time window, date "
176+
"range or period, answer 'All time — no date range filter'. Do NOT accept any date, "
177+
"time-window or period filter the agent proposes; if its proposal or summary adds one, "
178+
"tell it to drop the filter."
179+
)
128180

129181

130182
def generate_simulated_alert_response(
@@ -151,24 +203,26 @@ def generate_simulated_alert_response(
151203
trigger = expected.trigger
152204
filters = expected.filters
153205

154-
trigger_line = (
155-
f"5. Proactively tell the agent the trigger is '{trigger}' in your first reply.\n"
156-
if trigger not in _ALWAYS_TRIGGER_VALUES
157-
else ""
158-
)
206+
rules = [
207+
f"Your goal: metric={metric}, operator={operator}, threshold={threshold}, "
208+
f"recipients={recipients}, trigger={trigger}" + (f", filters={filters}" if filters else "") + ".",
209+
"Never revert or change a decision that was already confirmed in a previous turn.",
210+
"If the agent shows a proposal or final summary and asks for confirmation, verify that the "
211+
"recipients, the trigger AND the filters all match your goal. Correct every mismatch in "
212+
"your reply. Only once all three are correct, say 'Yes, please proceed to create the alert.'",
213+
"Proactively include your email recipient in your first reply. Do not wait for the agent "
214+
"to ask — state it alongside the metric and condition answers.",
215+
]
216+
trigger_instruction = _trigger_instruction(expected)
217+
if trigger_instruction:
218+
rules.append(f"Proactively tell the agent in your first reply: {trigger_instruction}.")
219+
rules.append(_filter_rule(filters))
220+
159221
system_prompt = (
160222
"You are a user requesting creation of an alert for a metric from an AI agent. "
161223
"Respond naturally but always steer toward the exact values you were given.\n"
162224
"Rules you MUST follow:\n"
163-
f"1. Your goal: metric={metric}, operator={operator}, threshold={threshold}, "
164-
f"recipients={recipients}, trigger={trigger}" + (f", filters={filters}" if filters else "") + ".\n"
165-
"2. Never revert or change a decision that was already confirmed in a previous turn.\n"
166-
"3. If the agent shows a final summary and asks for confirmation, verify that the "
167-
" recipients match your goal. If they differ, correct them. "
168-
" Once recipients are correct, say 'Yes, please proceed to create the alert.'\n"
169-
"4. Proactively include your email recipient in your first reply. "
170-
" Do not wait for the agent to ask — state it alongside the metric and condition answers.\n"
171-
+ trigger_line
225+
+ "".join(f"{i}. {rule}\n" for i, rule in enumerate(rules, start=1))
172226
+ "Reply concisely and directly."
173227
)
174228

@@ -180,7 +234,9 @@ def generate_simulated_alert_response(
180234
response = openai_client.chat.completions.create(
181235
model="gpt-4o",
182236
messages=messages,
183-
temperature=0.5,
237+
# Deterministic as the model allows: the trigger/filter drift this prompt guards against
238+
# was sampling-dependent, so temperature must not reintroduce it.
239+
temperature=0,
184240
)
185241
return response.choices[0].message.content or ""
186242

@@ -424,6 +480,48 @@ def _run_once(conv_id: str) -> AlertRunResult:
424480
)
425481

426482

483+
def _actual_trigger_display(actual_args: dict) -> str:
484+
"""Trigger as the agent set it, with the interval appended when one applies."""
485+
trigger = actual_args.get("trigger") or actual_args.get("triggerMode") or "ALWAYS"
486+
interval = actual_args.get("trigger_interval") or actual_args.get("triggerInterval")
487+
return f"{trigger}/{interval}" if interval else str(trigger)
488+
489+
490+
def _score_comments(expected: CatalogMetricAlert, actual_args: dict, alert_created: bool) -> dict[str, str]:
491+
"""Expected-vs-actual detail per strict check, for the Langfuse score comments.
492+
493+
Without these a failed check shows only 0.0 on the trace, and triaging means reading CI logs
494+
or hunting the create_metric_alert arguments through nested trace observations.
495+
"""
496+
if not alert_created:
497+
return {"alert_created": "create_metric_alert was never called"}
498+
499+
if expected.operator in _RANGE_OPERATORS:
500+
exp_threshold: object = f"{expected.threshold_from}..{expected.threshold_to}"
501+
act_threshold: object = (
502+
f"{actual_args.get('from_value', actual_args.get('fromValue'))}.."
503+
f"{actual_args.get('to_value', actual_args.get('toValue'))}"
504+
)
505+
else:
506+
exp_threshold = expected.threshold
507+
act_threshold = actual_args.get("threshold")
508+
509+
act_metric_raw = str(actual_args.get("metric_id", actual_args.get("metricId", "")))
510+
act_metric = _parse_metric_id(act_metric_raw) or act_metric_raw
511+
return {
512+
"alert_created": "create_metric_alert was called",
513+
"operator_correct": f"expected {expected.operator!r}; actual {actual_args.get('operator')!r}",
514+
"threshold_correct": f"expected {exp_threshold!r}; actual {act_threshold!r}",
515+
"trigger_correct": f"expected {expected.trigger!r}; actual {_actual_trigger_display(actual_args)!r}",
516+
"filters_correct": (
517+
f"expected {expected.filters!r}; "
518+
f"actual {actual_args.get('filters', actual_args.get('attribute_filters'))!r}"
519+
),
520+
"metric_correct": f"expected {expected.metric_id!r}; actual {act_metric!r}",
521+
"recipients_correct": f"expected {expected.recipients!r}; actual {_actual_recipients(actual_args)!r}",
522+
}
523+
524+
427525
class AlertSkillAssertionError(AssertionError):
428526
"""Raised when an alert-skill evaluation fails."""
429527

@@ -489,6 +587,7 @@ def evaluate_agentic_alert_skill(
489587
[r.conversation_id for r in summary.run_results],
490588
window_start,
491589
)
590+
expected = _normalize_expected_output(expected_output)
492591
suffix_needed = len(summary.run_results) > 1
493592
for run_idx, run in enumerate(summary.run_results):
494593
pt = traces_by_conv.get(run.conversation_id)
@@ -503,9 +602,17 @@ def evaluate_agentic_alert_skill(
503602
"metric_correct": ev.metric_correct,
504603
"recipients_correct": ev.recipients_correct,
505604
}
605+
comments = _score_comments(expected, run.actual_alert_arguments, ev.alert_created)
506606
with observe(langfuse, pt.id if pt else None, dataset_item_id, run_name, run_metadata) as tid:
507607
for score_name, value in strict_checks.items():
508-
score_safe(langfuse, tid, name=score_name, value=float(value), data_type="BOOLEAN")
608+
score_safe(
609+
langfuse,
610+
tid,
611+
name=score_name,
612+
value=float(value),
613+
data_type="BOOLEAN",
614+
comment=comments.get(score_name),
615+
)
509616
log_quality_and_value_scores(
510617
langfuse,
511618
tid,

0 commit comments

Comments
 (0)