Skip to content

Commit a182e2c

Browse files
committed
fix(gooddata-eval): reject malformed alert group-by arguments (GDAI-2175)
`actual_args.get("attributes") or []` collapsed a falsy non-list -- `{}`, `""`, `0` -- into "no grouping", so those passed an `Attributes: []` expectation. `_attribute_label_ids` also never checked that its input was a list, so a truthy non-list was iterated instead: a dict yields its keys, a string yields its characters. That produced a false pass, an actual of `"x"` scoring correct against an expectation of `[{"using": "label/x"}]`. The two malformations are not the same kind of event, and the split is structural rather than by side: - Not a list of groupings at all -> False. The agent answered wrongly, and a wrong answer is a FAIL. Raising would record an ERROR, which `json_report.py` excludes from the failure count, so a malformed answer would rank above a merely wrong one. - A list holding an unreadable entry -> raises, as before. Entries are typed `AttributeItem` at the tool boundary, so the plausible cause is the wire format moving, and that has to be unmissable rather than read as every agent regressing at once. The expectation side needs no equivalent guard: `_normalize_expected_output` already rejects a non-list before the run starts, and `from_dict`, the other way into the field, has no callers. risk: low
1 parent b090999 commit a182e2c

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ def _attribute_label_ids(items: list, *, side: str) -> list[str]:
140140
``attribute/x`` are different objects, and an unknown spelling must fail loudly rather
141141
than quietly compare unequal.
142142
"""
143+
if not isinstance(items, list):
144+
raise ValueError(f"Unrecognised {side} group-by attributes, expected a list: {items!r}")
143145
ids: list[str] = []
144146
for item in items:
145147
raw: object = None
@@ -174,9 +176,20 @@ def _check_attributes(expected: CatalogMetricAlert, actual_args: dict) -> bool:
174176
matter.
175177
"""
176178
exp_attributes = expected.attributes
177-
act_attributes = actual_args.get("attributes") or []
178179
if exp_attributes is None:
179180
return True
181+
# An unset argument serialises as null, so both spellings of "no grouping" land on [].
182+
act_attributes = actual_args.get("attributes")
183+
if act_attributes is None:
184+
act_attributes = []
185+
if not isinstance(act_attributes, list):
186+
# An argument that is not a list of groupings is the agent answering wrongly, so it
187+
# scores False. Raising instead would make the runner record an ERROR, and errored
188+
# items are excluded from the failure count — a malformed answer must not rank above
189+
# a merely wrong one. An unreadable *entry* still raises, in `_attribute_label_ids`:
190+
# entries are typed at the tool boundary, so the plausible cause there is the wire
191+
# format moving, which has to be unmissable.
192+
return False
180193
if not exp_attributes:
181194
return not act_attributes
182195
exp_ids = sorted(_attribute_label_ids(exp_attributes, side="expected"))

‎packages/gooddata-eval/tests/test_agentic_alert_skill.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,21 @@ def test_unrecognised_expectation_shape_raises():
843843
_normalize_expected_output({"Attributes": [{"using": "attribute/product_brand"}]})
844844

845845

846+
def test_malformed_actual_attributes_fail_rather_than_error():
847+
"""A non-list argument is the agent answering wrongly, and a wrong answer is a FAIL.
848+
849+
An ERROR would be excluded from the run's failure count, ranking a malformed answer
850+
above a merely wrong one.
851+
"""
852+
expected_none = _normalize_expected_output({"Attributes": []})
853+
for malformed in ({}, "", 0, {"using": "label/product_brand"}, "product_brand"):
854+
assert _check_attributes(expected_none, {"attributes": malformed}) is False
855+
856+
expected_brand = _normalize_expected_output({"Attributes": [_AAC_BRAND_GROUPING]})
857+
for malformed in ({}, "", 0, "product_brand", {"using": "label/product_brand"}):
858+
assert _check_attributes(expected_brand, {"attributes": malformed}) is False
859+
860+
846861
def test_unrecognised_actual_shape_raises():
847862
expected = _normalize_expected_output({"Attributes": [_AAC_BRAND_GROUPING]})
848863
with pytest.raises(ValueError, match="actual group-by"):

0 commit comments

Comments
 (0)