fix(gooddata-eval): treat alert proposal part as confirmation signal - #1699
Conversation
JIRA: QA-28774 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAlert proposals now flow through SSE parsing, ChangesAlert proposal flow
uv installation configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SSEClient
participant ChatResult
participant ConversationLoop
participant render_alert_proposal
participant SimulatedUser
participant AlertCreation
SSEClient->>ChatResult: Provide alertProposals
ChatResult->>ConversationLoop: Provide proposal-only response
ConversationLoop->>render_alert_proposal: Render latest proposal
render_alert_proposal->>SimulatedUser: Send proposal text and CTA
SimulatedUser->>ConversationLoop: Return confirmation
ConversationLoop->>AlertCreation: Create metric alert
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
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/agentic/alert_skill.py`:
- Around line 329-330: Update the alert handling around the proposal processing
so summary["alert"] is assigned whenever the proposal contains an alert,
including after execution is removed and the alert becomes empty; do not gate
the assignment on alert truthiness. Add a regression case covering an alert
containing only {"execution": {...}} and verify the resulting summary omits the
execution payload.
🪄 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: 42c63366-50ac-425e-8c43-d993aa8026fb
📒 Files selected for processing (7)
packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.pypackages/gooddata-eval/src/gooddata_eval/core/models.pypackages/gooddata-eval/tests/test_agentic_alert_skill.pypackages/gooddata-eval/tests/test_agentic_conversation.pypackages/gooddata-eval/tests/test_sse_client.py
… field JIRA: QA-28774 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Unrelated to the rest of this PR; unblocks `unit-tests`, which every Python-changing PR now hits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1699 +/- ##
==========================================
+ Coverage 78.05% 78.30% +0.24%
==========================================
Files 271 271
Lines 18676 18689 +13
==========================================
+ Hits 14578 14634 +56
+ Misses 4098 4055 -43 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…parison create_metric_alert addresses a notification one of two ways: `recipients`/ `external_recipients` (raw email addresses) when the channel can send externally, or `internal_recipients` (internal GoodData user ids, never emails) when the channel is restricted to workspace-registered users. _check_recipients only ever read recipients/external_recipients, so any alert delivered the internal way always failed this check regardless of what the fixture expected -- confirmed live: a real, correctly-delivered alert with internal_recipients=['user.<uuid>'] still scored recipients_correct=False, because the code was comparing against a key that's never populated for that delivery path. Resolves the expected email to its internal user id via the Users entities API (GET /entities/users?filter=email==...), lazily -- only when the plain comparison already failed and internal_recipients is actually present, so no unconditional network call is added to the hot path (existing run_agentic_alert_skill tests never mock GoodDataSdk, only ChatClient). Same shape of gap as #1699 (alert_proposals as a confirmation signal): gooddata-eval's evaluator hadn't been taught to read a real tool-response shape yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JIRA: QA-28774
Summary by CodeRabbit
New Features
Bug Fixes
Tests