Skip to content

feat(gooddata-eval): report created metric/automation id from single-turn evaluators#1694

Merged
Tomkess merged 2 commits into
masterfrom
feat/report-created-object-ids
Jul 23, 2026
Merged

feat(gooddata-eval): report created metric/automation id from single-turn evaluators#1694
Tomkess merged 2 commits into
masterfrom
feat/report-created-object-ids

Conversation

@Tomkess

@Tomkess Tomkess commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Problem

MetricSkillEvaluator and AlertSkillEvaluator (packages/gooddata-eval/src/gooddata_eval/core/evaluators/{metric_skill,alert_skill}.py) already parse the create_metric / create_metric_alert tool call's result to grade a run (MAQL/format match, operator/threshold/trigger/filters/metric/recipients match), but discard the real server-assigned id of whatever object the agent created.

This matters for any consumer running these single-turn kinds against a persistent/shared workspace: there is currently no way to identify the exact object a run created from gd-eval's own --json report. The agentic counterparts (agentic_metric_skill/agentic_alert_skill, core/agentic/{metric_skill,alert_skill}.py) already extract this same id from the identical tool-call-result payload to self-clean via entities_api.delete_entity_metrics/delete_entity_automations in a finally block — the single-turn path has the same data in hand and simply never surfaces it.

Concretely: a caller orchestrating single-turn metric_skill/alert_skill runs against a shared eval workspace (e.g. a scheduled CI job) has no choice today but to diff the workspace's metric/automation catalog before and after each call and guess which new object belongs to which run — fragile under any concurrent activity, and for alert_skill there's no deterministic field to cross-check the guess against at all.

Change

  • MetricSkillEvaluator.evaluate(): adds detail["metric_id"], read from the already-unwrapped create_metric tool result (payload.get("metric_id")). null when no tool call fired.
  • AlertSkillEvaluator.evaluate(): adds detail["automation_id"], extracted from the create_metric_alert tool call's result (not its arguments — the id is server-assigned, the agent's requested arguments don't carry it) via a new _extract_automation_id() helper, mirroring core/agentic/alert_skill.py's _extract_alert_call's result_data.get("id") or (result_data.get("data") or {}).get("id") unwrap shape. null when no tool call fired.
  • No changes to control flow, scoring, passed/rank_key computation, or the JSON report's shape/schema beyond the two new detail keys — json_report.py's _build_run_dict already serializes item.best_detail verbatim, so both new fields flow through to --json output with no further changes needed there.
  • No cleanup/deletion behavior added to the single-turn path — this PR is reporting-only. (Whether the single-turn evaluators should also self-clean like their agentic counterparts is a separate question, deliberately out of scope here.)

Testing

  • tests/test_metric_skill_evaluator.py: asserts detail["metric_id"] on the existing exact-match fixture (already encoded "metric_id": "avg_order_value" in its tool-result JSON, previously unused by any assertion) and asserts None on the no-tool-call path.
  • tests/test_alert_skill_evaluator.py: extends _chat_with_alert() to accept an optional result payload (previously hardcoded to "{}"), adds three new cases — top-level {"id": ...}, nested {"data": {"id": ...}}, and no-id-present — plus a None assertion on the no-tool-call path.
  • Full package suite: pytest packages/gooddata-eval/tests — 242 passed (245 with the 3 new cases in this PR).
  • End-to-end sanity check: ran this branch's editable build against a live workspace via gd-eval run --dataset <single metric_skill item> --json report.json and confirmed report["runs"][...]["items"][...]["detail"]["metric_id"] is present in the actual CLI output (null in that particular run, since the agent didn't call create_metric that time — confirms the field wires through end-to-end regardless of outcome).

Compatibility

Purely additive — two new keys in each evaluator's detail dict. No existing key removed, renamed, or changed in meaning; no public function signatures changed.

Summary by CodeRabbit

  • New Features

    • Evaluation results now include the created metric’s ID.
    • Alert evaluation results now include the created automation’s ID when available.
    • Missing tool results clearly report these IDs as unavailable.
  • Tests

    • Added coverage for IDs returned in multiple alert result formats.
    • Expanded metric evaluation tests to verify ID reporting in success and failure scenarios.

…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.
@Tomkess
Tomkess requested review from hkad98, lupko and pcerny as code owners July 22, 2026 13:58
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Tomkess, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40254a55-1b3f-4b2e-b3f6-6f19ad4c1196

📥 Commits

Reviewing files that changed from the base of the PR and between 0bd8938 and 513f430.

📒 Files selected for processing (2)
  • packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py
  • packages/gooddata-eval/tests/test_alert_skill_evaluator.py
📝 Walkthrough

Walkthrough

Alert and metric evaluators now expose created object identifiers in evaluation details, including None for missing tool calls or unavailable identifiers. Tests cover top-level and nested alert result identifiers and metric identifier reporting.

Changes

Evaluator result identifiers

Layer / File(s) Summary
Alert automation identifier extraction
packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py, packages/gooddata-eval/tests/test_alert_skill_evaluator.py
Alert evaluation extracts automation IDs from top-level or nested tool results and reports None when unavailable.
Metric identifier reporting
packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py, packages/gooddata-eval/tests/test_metric_skill_evaluator.py
Metric evaluation includes the created metric ID and reports None when no creation tool call exists.

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

Suggested reviewers: lupko, pcerny, hkad98

Poem

A rabbit hops through results bright,
IDs emerge from payload night.
Alerts and metrics leave a trace,
Missing calls show empty space.
Tests nibble bugs with gentle grace.

🚥 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 summarizes the main change: single-turn evaluators now report created metric and automation IDs.
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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py`:
- Around line 28-38: Update _extract_automation_id to validate
result_data["data"] is a dict before calling .get("id"), returning None when the
nested value is malformed; add a regression case covering a non-mapping data
payload and asserting automation_id is None.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 32104030-f7c1-4215-a07c-bbbe300bd31e

📥 Commits

Reviewing files that changed from the base of the PR and between 56f00e1 and 0bd8938.

📒 Files selected for processing (4)
  • packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py
  • packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py
  • packages/gooddata-eval/tests/test_alert_skill_evaluator.py
  • packages/gooddata-eval/tests/test_metric_skill_evaluator.py

Comment thread packages/gooddata-eval/src/gooddata_eval/core/evaluators/alert_skill.py Outdated
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.
@Tomkess
Tomkess merged commit 5684211 into master Jul 23, 2026
12 checks passed
@Tomkess
Tomkess deleted the feat/report-created-object-ids branch July 23, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants