Conversation
|
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 selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe alert skill now distinguishes unspecified and empty filters, preserves filter lists and threshold ranges, passes the original question to simulated responses, enforces ALWAYS and ONCE cadence, and validates complete alert proposals before confirmation. ChangesAlert semantics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EvaluationLoop
participant AlertSkill
participant SimulatedUser
EvaluationLoop->>AlertSkill: pass original request
AlertSkill->>SimulatedUser: generate response with question and conversation history
SimulatedUser->>AlertSkill: confirm alert fields and cadence
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 200-205: Update generate_simulated_alert_response and its call
from run_agentic_alert_skill to include the original question in the generated
system prompt, ensuring simulated users can derive required filters even when
normalized filter prose is None. Preserve the existing filter guidance and add
an interaction test where the agent requests a required filter without repeating
it.
- Around line 175-178: Use the normalized trigger key from the trigger-handling
flow when constructing the goal text, so omitted triggers are rendered as ALWAYS
rather than “not specified”; update the relevant goal-generation logic near the
trigger instructions and add a regression test using a fixture without Trigger
that rejects an ONCE proposal.
- Around line 318-331: Update _normalize_expected_filters so non-list,
non-marker Filters values return None instead of arbitrary text, matching its
documented contract and allowing _check_filters to treat them as unspecified.
Preserve any prose separately through the existing simulated-user state or
context mechanism if it is needed there, without changing marker handling or
exact list expectations.
🪄 Autofix
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: 5b3316b3-07d6-468c-8810-579a35579094
📒 Files selected for processing (2)
packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.pypackages/gooddata-eval/tests/test_agentic_alert_skill.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1721 +/- ##
==========================================
+ Coverage 78.59% 78.74% +0.15%
==========================================
Files 271 271
Lines 18772 18794 +22
==========================================
+ Hits 14754 14800 +46
+ Misses 4018 3994 -24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JIRA: QA-28623 risk: nonprod Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JIRA: QA-28623
risk: nonprod
Summary by CodeRabbit