feat(seer-activity): Better communicate incompatible alerts#119007
feat(seer-activity): Better communicate incompatible alerts#119007leeandher wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4c97a6a. Configure here.
| return new Set<string>(); | ||
| } | ||
|
|
||
| return new Set<string>(incompatibleConditions.map(c => c.id)); |
There was a problem hiding this comment.
NONE logic false Seer conflicts
Low Severity
findSeerActivityIncompatibleConditions marks event-based filters as incompatible for Seer triggers even when the action filter uses none logic. Those handlers evaluate to non-triggering on activity payloads, which is what none requires, so the filter can still pass and actions can run despite the red conflict state.
Reviewed by Cursor Bugbot for commit 4c97a6a. Configure here.
| const slowConditions = new Set<DataConditionType>([ | ||
| DataConditionType.EVENT_FREQUENCY_COUNT, | ||
| DataConditionType.EVENT_FREQUENCY_PERCENT, | ||
| DataConditionType.EVENT_UNIQUE_USER_FREQUENCY_COUNT, | ||
| DataConditionType.EVENT_UNIQUE_USER_FREQUENCY_PERCENT, | ||
| DataConditionType.PERCENT_SESSIONS_COUNT, | ||
| DataConditionType.PERCENT_SESSIONS_PERCENT, | ||
| ]); |
There was a problem hiding this comment.
Bug: The slowConditions set is missing two frequency-based conditions, causing the UI to not warn users about silently non-functional automation rules.
Severity: MEDIUM
Suggested Fix
Add DataConditionType.EVENT_UNIQUE_USER_FREQUENCY_WITH_CONDITIONS_COUNT and DataConditionType.EVENT_UNIQUE_USER_FREQUENCY_WITH_CONDITIONS_PERCENT to the slowConditions set in utils.tsx to ensure they are correctly flagged as incompatible with Seer activity triggers.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: static/app/views/automations/hooks/utils.tsx#L205-L212
Potential issue: The `slowConditions` set is intended to identify automation conditions
that are incompatible with Seer activity triggers because they require Snuba queries.
However, it omits `EVENT_UNIQUE_USER_FREQUENCY_WITH_CONDITIONS_COUNT` and
`EVENT_UNIQUE_USER_FREQUENCY_WITH_CONDITIONS_PERCENT`. As a result, when a user
configures an automation rule with a Seer activity trigger and one of these conditions,
the UI will not display a warning. The backend will silently ignore the condition,
leading to a rule that does not function as expected, with no feedback provided to the
user.
Did we get this right? 👍 / 👎 to inform future reviews.
| if ( | ||
| anyLogicTypes.has(conditionGroup.logicType) && | ||
| incompatibleConditions.length !== conditionGroup.conditions.length | ||
| ) { | ||
| return new Set<string>(); |
There was a problem hiding this comment.
Bug: The function findSeerActivityIncompatibleConditions incorrectly flags compatible conditions as incompatible when using NONE logic, showing a false positive warning.
Severity: MEDIUM
Suggested Fix
Update findSeerActivityIncompatibleConditions to correctly handle the DataConditionGroupLogicType.NONE case. When the logic is NONE, event-required conditions should not be considered incompatible, as their falsy evaluation is the desired behavior.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: static/app/views/automations/hooks/utils.tsx#L324-L328
Potential issue: The `findSeerActivityIncompatibleConditions` function incorrectly flags
event-required conditions as incompatible when the filter logic is set to `NONE`. With
`NONE` logic, all conditions must evaluate to false for the filter to pass. Since
event-required conditions always evaluate to false when given a Seer activity object,
they are actually compatible and satisfy the `NONE` constraint. The current
implementation does not handle this case, resulting in a false positive warning in the
UI, preventing users from creating a valid automation rule.
Did we get this right? 👍 / 👎 to inform future reviews.


Some attributes and filters will never fire when connected to a Seer activity alert, so we should warn about those.
