fix(alerts): pass correct filters for aggregate trends (#26357)

posthog/tasks/alerts/test/test_alert_checks.py

@@ -100,7 +100,7 @@ def test_alert_is_triggered_for_values_above_higher_threshold(
         anomalies_descriptions = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
         assert len(anomalies_descriptions) == 1
         assert (
-            "The insight value ($pageview) for previous day (1) is more than upper threshold (0.0)"
+            "The insight value ($pageview) for previous interval (1) is more than upper threshold (0.0)"
             in anomalies_descriptions[0]
         )
 
@@ -130,7 +130,7 @@ def test_alert_is_triggered_for_value_below_lower_threshold(
 
         assert mock_send_notifications_for_breaches.call_count == 1
         anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
-        assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies
+        assert "The insight value ($pageview) for previous interval (0) is less than lower threshold (1.0)" in anomalies
 
     def test_alert_triggers_but_does_not_send_notification_during_firing(
         self, mock_send_notifications_for_breaches: MagicMock, mock_send_errors: MagicMock
@@ -318,7 +318,7 @@ def test_alert_with_insight_with_filter(
 
         assert mock_send_notifications_for_breaches.call_count == 1
         anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
-        assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies
+        assert "The insight value ($pageview) for previous interval (0) is less than lower threshold (1.0)" in anomalies
 
     @patch("posthog.tasks.alerts.utils.EmailMessage")
     def test_send_emails(

posthog/tasks/alerts/test/test_trends_absolute_alerts.py

@@ -88,6 +88,34 @@ def create_time_series_trend_insight(self, breakdown: Optional[BreakdownFilter]
 
         return insight
 
+    def create_aggregate_trend_insight(self, breakdown: Optional[BreakdownFilter] = None) -> dict[str, Any]:
+        query_dict = TrendsQuery(
+            series=[
+                EventsNode(
+                    event="signed_up",
+                    math=BaseMathType.TOTAL,
+                ),
+                EventsNode(
+                    event="$pageview",
+                    name="Pageview",
+                    math=BaseMathType.TOTAL,
+                ),
+            ],
+            breakdownFilter=breakdown,
+            trendsFilter=TrendsFilter(display=ChartDisplayType.ACTIONS_PIE),
+            interval=IntervalType.WEEK,
+            dateRange=InsightDateRange(date_from="-8w"),
+        ).model_dump()
+
+        insight = self.dashboard_api.create_insight(
+            data={
+                "name": "insight",
+                "query": query_dict,
+            }
+        )[1]
+
+        return insight
+
     def test_alert_lower_threshold_breached(self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock) -> None:
         insight = self.create_time_series_trend_insight()
         alert = self.create_alert(insight, series_index=0, lower=1)
@@ -295,3 +323,87 @@ def test_trend_breakdown_no_threshold_breached(
         assert alert_check.error is None
 
         mock_send_breaches.assert_not_called()
+
+    def test_aggregate_trend_high_threshold_breached(
+        self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock
+    ) -> None:
+        insight = self.create_aggregate_trend_insight()
+        alert = self.create_alert(insight, series_index=0, upper=1)
+
+        with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=4)):
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="3",
+                properties={"$browser": "Firefox"},
+            )
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="1",
+                properties={"$browser": "Chrome"},
+            )
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="2",
+                properties={"$browser": "Chrome"},
+            )
+            flush_persons_and_events()
+
+        check_alert(alert["id"])
+
+        updated_alert = AlertConfiguration.objects.get(pk=alert["id"])
+        assert updated_alert.state == AlertState.FIRING
+        assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1)
+
+        alert_check = AlertCheck.objects.filter(alert_configuration=alert["id"]).latest("created_at")
+        assert alert_check.calculated_value == 3
+        assert alert_check.state == AlertState.FIRING
+        assert alert_check.error is None
+
+        mock_send_breaches.assert_called_once_with(
+            ANY, ["The insight value (signed_up) for previous interval (3) is more than upper threshold (1.0)"]
+        )
+
+    def test_aggregate_trend_with_breakdown_high_threshold_breached(
+        self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock
+    ) -> None:
+        insight = self.create_aggregate_trend_insight(BreakdownFilter(breakdowns=[Breakdown(property="$browser")]))
+        alert = self.create_alert(insight, series_index=0, upper=1)
+
+        with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=4)):
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="3",
+                properties={"$browser": "Firefox"},
+            )
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="1",
+                properties={"$browser": "Chrome"},
+            )
+            _create_event(
+                team=self.team,
+                event="signed_up",
+                distinct_id="2",
+                properties={"$browser": "Chrome"},
+            )
+            flush_persons_and_events()
+
+        check_alert(alert["id"])
+
+        updated_alert = AlertConfiguration.objects.get(pk=alert["id"])
+        assert updated_alert.state == AlertState.FIRING
+        assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1)
+
+        alert_check = AlertCheck.objects.filter(alert_configuration=alert["id"]).latest("created_at")
+        assert alert_check.calculated_value == 2
+        assert alert_check.state == AlertState.FIRING
+        assert alert_check.error is None
+
+        mock_send_breaches.assert_called_once_with(
+            ANY, ["The insight value (signed_up - Chrome) for previous interval (2) is more than upper threshold (1.0)"]
+        )

posthog/tasks/alerts/trends.py

@@ -58,19 +58,21 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
         (query.breakdownFilter.breakdown and query.breakdownFilter.breakdown_type) or query.breakdownFilter.breakdowns
     )
 
+    is_non_time_series = _is_non_time_series_trend(query)
+
     match condition.type:
         case AlertConditionType.ABSOLUTE_VALUE:
             if threshold.type != InsightThresholdType.ABSOLUTE:
                 raise ValueError(f"Absolute threshold not configured for alert condition ABSOLUTE_VALUE")
 
-            # want value for current interval (last hour, last day, last week, last month)
-            # depending on the alert calculation interval
-            if _is_non_time_series_trend(query):
-                filters_override = _date_range_override_for_intervals(query, last_x_intervals=2)
-            else:
+            if is_non_time_series:
                 # for non time series, it's an aggregated value for full interval
                 # so we need to compute full insight
                 filters_override = None
+            else:
+                # want values back till previous interval (last hour, last day, last week, last month)
+                # depending on the alert calculation interval
+                filters_override = _date_range_override_for_intervals(query, last_x_intervals=2)
 
             calculation_result = calculate_for_query_based_insight(
                 insight,
@@ -83,18 +85,21 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
             if not calculation_result.result:
                 raise RuntimeError(f"No results found for insight with alert id = {alert.id}")
 
+            interval = query.interval if not is_non_time_series else None
+
             if has_breakdown:
                 # for breakdowns, we need to check all values in calculation_result.result
                 breakdown_results = calculation_result.result
 
                 for breakdown_result in breakdown_results:
+                    # pick previous interval value
                     prev_interval_value = _pick_interval_value_from_trend_result(query, breakdown_result, -1)
                     breaches = _validate_bounds(
                         threshold.bounds,
                         prev_interval_value,
                         threshold.type,
                         condition.type,
-                        query.interval,
+                        interval,
                         breakdown_result["label"],
                     )
 
@@ -114,13 +119,13 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
                     prev_interval_value,
                     threshold.type,
                     condition.type,
-                    query.interval,
+                    interval,
                     selected_series_result["label"],
                 )
 
                 return AlertEvaluationResult(value=prev_interval_value, breaches=breaches)
         case AlertConditionType.RELATIVE_INCREASE:
-            if _is_non_time_series_trend(query):
+            if is_non_time_series:
                 raise ValueError(f"Relative alerts not supported for non time series trends")
 
             # to measure relative increase, we can't alert until current interval has completed
@@ -188,7 +193,7 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
             return AlertEvaluationResult(value=(increase if not has_breakdown else None), breaches=breaches)
 
         case AlertConditionType.RELATIVE_DECREASE:
-            if _is_non_time_series_trend(query):
+            if is_non_time_series:
                 raise ValueError(f"Relative alerts not supported for non time series trends")
 
             # to measure relative decrease, we can't alert until current interval has completed