From 0bd89389672853c1417d86fb977b2f7fc7bf8f6e Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 22 Jul 2026 15:48:15 +0200 Subject: [PATCH 1/2] feat(gooddata-eval): report created metric/automation id from single-turn evaluators metric_skill and alert_skill evaluators already parse the create_metric / create_metric_alert tool call's result to grade the run, but discarded the real server-assigned id. Downstream consumers running these single-turn evals against a shared workspace (e.g. a CI orchestrator) currently have no way to know exactly what was created, and have to diff the workspace catalog before/after to guess at cleanup -- fragile under concurrent activity. Surface the id gd-eval already has in memory instead. --- .../core/evaluators/alert_skill.py | 20 ++++++++++- .../core/evaluators/metric_skill.py | 7 +++- .../tests/test_alert_skill_evaluator.py | 34 +++++++++++++++++-- .../tests/test_metric_skill_evaluator.py | 2 ++ 4 files changed, 59 insertions(+), 4 deletions(-) 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 3933e7dc4..f64a74854 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 @@ -25,6 +25,19 @@ def _extract_metric_id(metric_str: str) -> str | None: return match.group(1) if match else None +def _extract_automation_id(tool_event) -> str | None: + """Real server-assigned id of the automation this call created. + + Mirrors ``core/agentic/alert_skill.py``'s ``_extract_alert_call`` — the id + lives in the tool *result* (what the server assigned), not the tool call + *arguments* (what the agent asked for). + """ + result_data = tool_event.parsed_result() + if not isinstance(result_data, dict): + return None + return result_data.get("id") or (result_data.get("data") or {}).get("id") + + def _check_threshold(expected: dict, actual_args: dict) -> bool: operator = expected.get("Operator", "") if operator == "ANOMALY": @@ -58,10 +71,11 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation return ItemEvaluation( passed=False, rank_key=(False,) * 7, - detail={"alert_created": False}, + detail={"alert_created": False, "automation_id": None}, ) args = tool_event.parsed_arguments() + automation_id = _extract_automation_id(tool_event) operator_correct = True threshold_correct = True @@ -126,5 +140,9 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation "filters_correct": filters_correct, "metric_correct": metric_correct, "recipients_correct": recipients_correct, + # Real server-assigned id of the automation this run created — + # lets a caller (e.g. a cleanup step) delete the exact object + # created instead of diffing the workspace catalog before/after. + "automation_id": automation_id, }, ) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py index 1204161bd..e43b5ebb5 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py @@ -28,7 +28,7 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation return ItemEvaluation( passed=False, rank_key=(False, False, False), - detail={"metric_created": False, "maql_correct": False, "format_correct": False}, + detail={"metric_created": False, "maql_correct": False, "format_correct": False, "metric_id": None}, ) result = tool_event.parsed_result() @@ -54,5 +54,10 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation "actual_maql": actual_maql, "expected_format": expected_format, "actual_format": actual_format, + # The real id of the metric this run created, straight from the + # create_metric tool result — lets a caller (e.g. a cleanup step) + # delete the exact object created instead of diffing the workspace + # catalog before/after and guessing by name. + "metric_id": payload.get("metric_id"), }, ) diff --git a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py index 6397220a8..937a005d2 100644 --- a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py +++ b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py @@ -15,11 +15,15 @@ def _item(expected_output: dict) -> DatasetItem: ) -def _chat_with_alert(args: dict) -> ChatResult: +def _chat_with_alert(args: dict, result: dict | None = None) -> ChatResult: return ChatResult.model_validate( { "toolCallEvents": [ - {"functionName": "create_metric_alert", "functionArguments": json.dumps(args), "result": "{}"} + { + "functionName": "create_metric_alert", + "functionArguments": json.dumps(args), + "result": json.dumps(result if result is not None else {}), + } ] } ) @@ -81,3 +85,29 @@ def test_alert_evaluator_fails_when_no_tool_call(): ) assert result.passed is False assert result.detail["alert_created"] is False + assert result.detail["automation_id"] is None + + +def test_alert_evaluator_extracts_automation_id_from_top_level_result(): + expected = {"Operator": "LESS_THAN", "Threshold": "20000"} + actual_args = {"operator": "LESS_THAN", "threshold": 20000} + result = get_evaluator("alert_skill").evaluate( + _item(expected), _chat_with_alert(actual_args, result={"id": "automation-abc-123"}) + ) + assert result.detail["automation_id"] == "automation-abc-123" + + +def test_alert_evaluator_extracts_automation_id_from_nested_data_result(): + expected = {"Operator": "LESS_THAN", "Threshold": "20000"} + actual_args = {"operator": "LESS_THAN", "threshold": 20000} + result = get_evaluator("alert_skill").evaluate( + _item(expected), _chat_with_alert(actual_args, result={"data": {"id": "automation-xyz-789"}}) + ) + assert result.detail["automation_id"] == "automation-xyz-789" + + +def test_alert_evaluator_automation_id_none_when_result_has_no_id(): + expected = {"Operator": "LESS_THAN", "Threshold": "20000"} + actual_args = {"operator": "LESS_THAN", "threshold": 20000} + result = get_evaluator("alert_skill").evaluate(_item(expected), _chat_with_alert(actual_args, result={})) + assert result.detail["automation_id"] is None diff --git a/packages/gooddata-eval/tests/test_metric_skill_evaluator.py b/packages/gooddata-eval/tests/test_metric_skill_evaluator.py index 9b07fd177..fb08a1a02 100644 --- a/packages/gooddata-eval/tests/test_metric_skill_evaluator.py +++ b/packages/gooddata-eval/tests/test_metric_skill_evaluator.py @@ -32,6 +32,7 @@ def test_metric_evaluator_passes_on_exact_match(): assert result.passed is True assert result.detail["maql_correct"] is True assert result.detail["format_correct"] is True + assert result.detail["metric_id"] == "avg_order_value" def test_metric_evaluator_fails_wrong_maql(): @@ -48,3 +49,4 @@ def test_metric_evaluator_fails_when_no_tool_call(): result = ev.evaluate(_item(), empty) assert result.passed is False assert result.detail["metric_created"] is False + assert result.detail["metric_id"] is None From 513f43029716aec5cb27c5e5c0f0d234f628896e Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Wed, 22 Jul 2026 16:40:06 +0200 Subject: [PATCH 2/2] fix: guard non-mapping nested result in _extract_automation_id result_data["data"] being present but not a dict (malformed/unexpected tool-result payload) crashed with AttributeError on .get("id") instead of reporting automation_id=None like the rest of the function already does for other malformed shapes. Addresses CodeRabbit review comment on #1694. --- .../src/gooddata_eval/core/evaluators/alert_skill.py | 3 ++- .../gooddata-eval/tests/test_alert_skill_evaluator.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) 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 f64a74854..4d19641de 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 @@ -35,7 +35,8 @@ def _extract_automation_id(tool_event) -> str | None: result_data = tool_event.parsed_result() if not isinstance(result_data, dict): return None - return result_data.get("id") or (result_data.get("data") or {}).get("id") + nested_data = result_data.get("data") + return result_data.get("id") or (nested_data.get("id") if isinstance(nested_data, dict) else None) def _check_threshold(expected: dict, actual_args: dict) -> bool: diff --git a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py index 937a005d2..b04240b6b 100644 --- a/packages/gooddata-eval/tests/test_alert_skill_evaluator.py +++ b/packages/gooddata-eval/tests/test_alert_skill_evaluator.py @@ -111,3 +111,14 @@ def test_alert_evaluator_automation_id_none_when_result_has_no_id(): actual_args = {"operator": "LESS_THAN", "threshold": 20000} result = get_evaluator("alert_skill").evaluate(_item(expected), _chat_with_alert(actual_args, result={})) assert result.detail["automation_id"] is None + + +def test_alert_evaluator_automation_id_none_when_nested_data_is_not_a_mapping(): + # Malformed/unexpected payload shape: "data" present but not a dict -> must + # not crash, just report automation_id=None (CodeRabbit review on PR #1694). + expected = {"Operator": "LESS_THAN", "Threshold": "20000"} + actual_args = {"operator": "LESS_THAN", "threshold": 20000} + result = get_evaluator("alert_skill").evaluate( + _item(expected), _chat_with_alert(actual_args, result={"data": "not-a-mapping"}) + ) + assert result.detail["automation_id"] is None