From 41ef018ea6bd52389d1bb97b581a743e7a30ded0 Mon Sep 17 00:00:00 2001 From: Sandy Spicer Date: Fri, 25 Sep 2026 11:08:33 -0700 Subject: [PATCH 1/4] fix(alerts): spread alert checks over a window after each interval Each alert now checks at a stable offset after its interval boundary, derived from the alert id: 2 to 13 minutes past the hour for hourly alerts, 1 to 3 minutes into each quarter for 15-minute alerts, and minutes 2 to 59 of the anchor hour for daily, weekly and monthly alerts. An explicit schedule_start_time keeps the time the user chose. Real-time alerts are unchanged. Existing alerts pick up their offset at their next check, so no migration is needed. The form previews the check window. The six-hourly query upgrade schedule also moves off minute zero. Co-Authored-By: Claude Opus 5.5 --- .../api/test/test_alert_15_minute_interval.py | 7 +- posthog/tasks/alerts/schedule_restriction.py | 10 ++- .../alerts/test/test_schedule_restriction.py | 34 +++++-- posthog/tasks/alerts/utils.py | 15 ++-- posthog/temporal/alerts/activities.py | 7 +- posthog/temporal/schedule.py | 5 +- products/alerts/backend/facade/scheduling.py | 88 +++++++++++++++---- .../alerts/backend/test/test_scheduling.py | 85 ++++++++++++++---- .../alerts/backend/tests/api/test_alert.py | 5 +- .../frontend/components/AlertIntervalRow.tsx | 19 +++- .../logic/alertSchedulingStale.test.ts | 44 +++++++--- .../frontend/logic/alertSchedulingStale.ts | 36 +++++--- 12 files changed, 274 insertions(+), 81 deletions(-) diff --git a/posthog/api/test/test_alert_15_minute_interval.py b/posthog/api/test/test_alert_15_minute_interval.py index a2675404ea57..9e917a9b9ae7 100644 --- a/posthog/api/test/test_alert_15_minute_interval.py +++ b/posthog/api/test/test_alert_15_minute_interval.py @@ -1,5 +1,6 @@ from datetime import UTC, datetime from typing import Any, cast +from uuid import UUID import pytest import time_machine @@ -13,6 +14,7 @@ from posthog.constants import AvailableFeature from posthog.tasks.alerts.utils import calculation_interval_to_order, next_check_time +from products.alerts.backend.facade.scheduling import CalendarInterval, alert_check_offset from products.alerts.backend.models.alert import AlertConfiguration @@ -101,6 +103,7 @@ def test_calculation_interval_to_order_ranks_every_15_minutes_before_hourly(self def test_next_check_time_advances_by_15_minutes(self) -> None: alert = MagicMock(spec=AlertConfiguration) + alert.id = UUID("0193f3c6-2a4b-7d2e-8f00-3c1b5d7e9a10") alert.calculation_interval = AlertCalculationInterval.EVERY_15_MINUTES alert.next_check_at = datetime(2026, 4, 6, 14, 0, 0, tzinfo=UTC) alert.team = MagicMock() @@ -110,7 +113,9 @@ def test_next_check_time_advances_by_15_minutes(self) -> None: alert.skip_weekend = False with time_machine.travel("2026-04-06T14:00:00Z", tick=False): - assert next_check_time(alert) == datetime(2026, 4, 6, 14, 15, 0, tzinfo=UTC) + assert next_check_time(alert) == datetime(2026, 4, 6, 14, 15, 0, tzinfo=UTC) + alert_check_offset( + CalendarInterval.EVERY_15_MINUTES, alert.id + ) def test_calculation_interval_to_order_raises_for_none(self) -> None: with pytest.raises(ValueError, match="Invalid alert calculation interval: None"): diff --git a/posthog/tasks/alerts/schedule_restriction.py b/posthog/tasks/alerts/schedule_restriction.py index b4cd6d228566..42321d2dadf7 100644 --- a/posthog/tasks/alerts/schedule_restriction.py +++ b/posthog/tasks/alerts/schedule_restriction.py @@ -16,12 +16,14 @@ MAX_UNBLOCK_STEPS, MIN_BLOCKED_WINDOW_MINUTES, MINUTES_PER_DAY, + alert_check_offset, is_local_minute_blocked, is_utc_datetime_blocked as _is_utc_datetime_blocked_pure, merged_intervals_cover_full_day, normalize_schedule_restriction_value, parse_blocked_windows_tuples, scan_next_unblocked_utc, + to_calendar_interval, validate_and_normalize_schedule_restriction, ) from products.alerts.backend.models.alert import AlertConfiguration @@ -87,4 +89,10 @@ def snap_candidate_utc_to_schedule_restriction(alert: AlertConfiguration, candid if not alert.schedule_restriction: return candidate_utc normalized = candidate_utc.astimezone(UTC).replace(second=0, microsecond=0) - return next_unblocked_utc(alert, normalized) + snapped = next_unblocked_utc(alert, normalized) + if snapped == normalized or alert.schedule_start_time is not None: + return snapped + # A snap lands on the end of a quiet window, which is usually on the hour, so every alert + # behind the same window would check at once. The alert's offset spreads them again. + offset_snap = snapped + alert_check_offset(to_calendar_interval(alert.calculation_interval), alert.id) + return snapped if is_utc_datetime_blocked(alert, offset_snap) else offset_snap diff --git a/posthog/tasks/alerts/test/test_schedule_restriction.py b/posthog/tasks/alerts/test/test_schedule_restriction.py index 77b339b9d39f..69ab3d4129a8 100644 --- a/posthog/tasks/alerts/test/test_schedule_restriction.py +++ b/posthog/tasks/alerts/test/test_schedule_restriction.py @@ -1,16 +1,23 @@ -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any +from uuid import UUID import pytest import time_machine from unittest.mock import MagicMock +from parameterized import parameterized + from posthog.tasks.alerts import schedule_restriction as schedule_restriction_module from posthog.tasks.alerts.schedule_restriction import is_utc_datetime_blocked, next_unblocked_utc from posthog.tasks.alerts.utils import next_check_at_after_schedule_restriction_change +from products.alerts.backend.facade.scheduling import CalendarInterval, alert_check_offset from products.alerts.backend.models.alert import AlertConfiguration +ALERT_ID = UUID("0193f3c6-2a4b-7d2e-8f00-3c1b5d7e9a10") +HOURLY_OFFSET = alert_check_offset(CalendarInterval.HOURLY, ALERT_ID) + class TestIsUtcDatetimeBlockedAndNextUnblocked: def _alert(self, tz: str, restriction: dict[str, Any] | None) -> MagicMock: @@ -65,6 +72,7 @@ def test_next_unblocked_retries_and_logs_when_scan_hits_cap(self, monkeypatch: p class TestNextCheckAtAfterScheduleRestrictionChange: def _hourly_alert(self, **kwargs: Any) -> MagicMock: alert = MagicMock(spec=AlertConfiguration) + alert.id = ALERT_ID alert.team = MagicMock() alert.team.timezone = "UTC" alert.calculation_interval = "hourly" @@ -78,17 +86,31 @@ def test_cleared_restriction_schedules_from_now_and_restores_existing_value(self existing = datetime(2026, 4, 7, 18, 30, tzinfo=UTC) alert = self._hourly_alert(schedule_restriction=None, next_check_at=existing) out = next_check_at_after_schedule_restriction_change(alert) - assert out == datetime(2026, 4, 6, 15, 0, 0, tzinfo=UTC) + assert out == datetime(2026, 4, 6, 15, 0, 0, tzinfo=UTC) + HOURLY_OFFSET assert alert.next_check_at == existing - def test_future_next_check_inside_blocked_window_snaps_to_first_unblocked_minute(self) -> None: + @parameterized.expand( + [ + # The first allowed minute is a window end on the hour, so the alert's offset moves the check past it. + ("offset_after_window_end", [{"start": "11:00", "end": "16:00"}], HOURLY_OFFSET), + # The offset would land inside the next quiet window, so the check stays on the first allowed minute. + ( + "offset_inside_next_window", + [{"start": "11:00", "end": "16:00"}, {"start": "16:01", "end": "17:00"}], + timedelta(0), + ), + ] + ) + def test_future_next_check_inside_blocked_window_snaps_to_first_unblocked_minute( + self, _name: str, blocked_windows: list[dict[str, str]], offset: timedelta + ) -> None: with time_machine.travel("2026-04-06T14:00:00Z", tick=False): alert = self._hourly_alert( - schedule_restriction={"blocked_windows": [{"start": "11:00", "end": "16:00"}]}, + schedule_restriction={"blocked_windows": blocked_windows}, next_check_at=datetime(2026, 4, 6, 15, 30, tzinfo=UTC), ) out = next_check_at_after_schedule_restriction_change(alert) - assert out == datetime(2026, 4, 6, 16, 0, 0, tzinfo=UTC) + assert out == datetime(2026, 4, 6, 16, 0, 0, tzinfo=UTC) + offset def test_custom_schedule_start_time_inside_blocked_window_snaps_to_first_unblocked_minute(self) -> None: with time_machine.travel("2026-04-06T20:00:00Z", tick=False): @@ -108,4 +130,4 @@ def test_does_not_keep_stale_snap_when_earlier_runs_are_allowed(self) -> None: next_check_at=datetime(2026, 4, 6, 20, 0, 0, tzinfo=UTC), ) out = next_check_at_after_schedule_restriction_change(alert) - assert out == datetime(2026, 4, 6, 17, 44, 0, tzinfo=UTC) + assert out == datetime(2026, 4, 6, 17, 0, 0, tzinfo=UTC) + HOURLY_OFFSET diff --git a/posthog/tasks/alerts/utils.py b/posthog/tasks/alerts/utils.py index 152c5f43519b..718848d3431d 100644 --- a/posthog/tasks/alerts/utils.py +++ b/posthog/tasks/alerts/utils.py @@ -123,18 +123,21 @@ def _next_check_time_core(alert: AlertConfiguration) -> datetime: now=datetime.now(pytz.UTC), tz_name=alert.team.timezone, next_check_at=alert.next_check_at, + alert_id=alert.id, schedule_start_time=alert.schedule_start_time, ) def next_check_time(alert: AlertConfiguration) -> datetime: """ - Rule by calculation interval - - hourly alerts -> want them to run at the same min every hour (same min comes from creation time so that they're spread out and don't all run at the start of the hour) - daily alerts -> want them to run at the start of the day (around 1am) by the timezone of the team - weekly alerts -> want them to run at the start of the week (Mon around 3am) by the timezone of the team - monthly alerts -> want them to run at the start of the month (first day of the month around 4am) by the timezone of the team + Rule by calculation interval. Each alert keeps a stable offset after the interval boundary + (alert_check_offset), so alerts that share an interval do not all run at its start. + + every 15 minutes alerts -> 1 to 3 minutes after each quarter hour + hourly alerts -> 2 to 13 minutes after each hour + daily alerts -> in the 1am hour of the team's timezone + weekly alerts -> in the 3am hour on Monday, in the team's timezone + monthly alerts -> in the 4am hour on the first day of the month, in the team's timezone """ candidate = _next_check_time_core(alert) return snap_candidate_utc_to_schedule_restriction(alert, candidate) diff --git a/posthog/temporal/alerts/activities.py b/posthog/temporal/alerts/activities.py index 38b5b4510b12..9657428d710d 100644 --- a/posthog/temporal/alerts/activities.py +++ b/posthog/temporal/alerts/activities.py @@ -37,7 +37,10 @@ from posthog.sync import database_sync_to_async from posthog.tasks.alerts.investigation_notifications import run_investigation_notification_safety_net from posthog.tasks.alerts.metrics_investigation import run_metrics_alert_investigation, should_investigate_metrics_alert -from posthog.tasks.alerts.schedule_restriction import is_utc_datetime_blocked, next_unblocked_utc +from posthog.tasks.alerts.schedule_restriction import ( + is_utc_datetime_blocked, + snap_candidate_utc_to_schedule_restriction, +) from posthog.tasks.alerts.utils import ( CALCULATION_INTERVAL_ORDER, add_alert_check, @@ -538,7 +541,7 @@ def _prepare() -> PrepareAlertResult: "Skipping alert check because of schedule restriction (quiet hours)", alert_id=alert.id, ) - alert.next_check_at = next_unblocked_utc(alert, now) + alert.next_check_at = snap_candidate_utc_to_schedule_restriction(alert, now) alert.save(update_fields=["next_check_at"]) return PrepareAlertResult(action=PrepareAction.SKIP, reason=SkipReason.QUIET_HOURS) diff --git a/posthog/temporal/schedule.py b/posthog/temporal/schedule.py index 927c1adcdcfd..cdc0938429b4 100644 --- a/posthog/temporal/schedule.py +++ b/posthog/temporal/schedule.py @@ -222,7 +222,10 @@ async def create_upgrade_queries_schedule(client: Client): id="upgrade-queries-schedule", task_queue=settings.GENERAL_PURPOSE_TASK_QUEUE, ), - spec=ScheduleSpec(intervals=[ScheduleIntervalSpec(every=timedelta(hours=6))]), + spec=ScheduleSpec( + intervals=[ScheduleIntervalSpec(every=timedelta(hours=6), offset=timedelta(minutes=2))], + jitter=timedelta(minutes=30), + ), ) if await a_schedule_exists(client, "upgrade-queries-schedule"): diff --git a/products/alerts/backend/facade/scheduling.py b/products/alerts/backend/facade/scheduling.py index 4b59b0ad59e6..61f2bdf93611 100644 --- a/products/alerts/backend/facade/scheduling.py +++ b/products/alerts/backend/facade/scheduling.py @@ -1,9 +1,10 @@ """Scheduling math for alert checks. -Sub-daily checks preserve their existing cadence and skip missed intervals. +Sub-daily checks skip missed intervals. Daily, weekly, and monthly checks anchor to calendar instants in the team's -local timezone. Quiet hours and weekend skipping layer local-time restrictions -on top of those schedules. +local timezone. Automatic schedules except real time run each alert at a stable +offset after the interval boundary (see `alert_check_offset`). Quiet hours and +weekend skipping layer local-time restrictions on top of those schedules. Pure Python with no Django or model imports. Timezones are passed as IANA names, quiet-hours windows as parsed tuples. @@ -12,7 +13,7 @@ from __future__ import annotations from collections.abc import Callable -from datetime import UTC, date, datetime, timedelta +from datetime import UTC, date, datetime, time, timedelta from enum import StrEnum from math import ceil from typing import Any, cast @@ -23,6 +24,8 @@ from pytz.exceptions import AmbiguousTimeError, NonExistentTimeError from pytz.tzinfo import BaseTzInfo +from posthog.scheduling.jitter import deterministic_offset + DEFAULT_SCHEDULE_INTERVAL_SECONDS = 60 @@ -111,6 +114,32 @@ class CalendarInterval(StrEnum): REAL_TIME_CADENCE_MINUTES = 2 EVERY_15_MINUTES_CADENCE_MINUTES = 15 +# (earliest offset, width) of the window after each interval boundary in which an alert's check runs. +# The earliest offset gives ingestion time to deliver the interval that just closed. +_CHECK_WINDOWS: dict[CalendarInterval, tuple[timedelta, timedelta]] = { + CalendarInterval.EVERY_15_MINUTES: (timedelta(minutes=1), timedelta(minutes=3)), + CalendarInterval.HOURLY: (timedelta(minutes=2), timedelta(minutes=12)), + CalendarInterval.DAILY: (timedelta(minutes=2), timedelta(minutes=58)), + CalendarInterval.WEEKLY: (timedelta(minutes=2), timedelta(minutes=58)), + CalendarInterval.MONTHLY: (timedelta(minutes=2), timedelta(minutes=58)), +} + + +def alert_check_offset(interval: CalendarInterval, alert_id: UUID | str) -> timedelta: + """How long after each interval boundary this alert's check runs. + + The offset is the same for an alert on every check, and alerts that share an interval + spread evenly over the window, so the fleet does not query ClickHouse at the boundary. + Real-time checks run every two minutes, so they have no window. + """ + window = _CHECK_WINDOWS.get(interval) + if window is None: + return timedelta(0) + earliest, width = window + offset = deterministic_offset(str(alert_id), width, floor=earliest) + # Whole minutes, because the scheduler collects due checks once a minute. + return timedelta(minutes=offset // timedelta(minutes=1)) + def to_calendar_interval(value: str | None) -> CalendarInterval: if value is None: @@ -138,16 +167,27 @@ def _localize_wall_time(team_timezone: BaseTzInfo, naive_local: datetime) -> dat def _calendar_anchor_utc( - local_now: datetime, team_timezone: BaseTzInfo, *, target_date: date, hour: int, + offset: timedelta, ) -> datetime: - naive_local = datetime.combine(target_date, local_now.timetz().replace(tzinfo=None)).replace(hour=hour) + naive_local = datetime.combine(target_date, time(hour=hour)) + offset return _localize_wall_time(team_timezone, naive_local).astimezone(UTC) +def _floor_to_local_period(timestamp: datetime, team_timezone: BaseTzInfo, period_minutes: int) -> datetime: + """Start of the local-time period of `period_minutes` that contains `timestamp`. + + The subtraction runs on the absolute instant, so a local hour that a DST change repeats + still resolves to the start of the hour that `timestamp` is in. + """ + local = timestamp.astimezone(team_timezone) + minutes_into_period = (local.hour * 60 + local.minute) % period_minutes + return timestamp - timedelta(minutes=minutes_into_period, seconds=local.second, microseconds=local.microsecond) + + def _next_check_at_for_schedule_start_time( interval: CalendarInterval, *, @@ -236,18 +276,21 @@ def next_calendar_check_time( now: datetime, tz_name: str, next_check_at: datetime | None, + alert_id: UUID | str, schedule_start_time: str | None = None, ) -> datetime: """Nominal next check instant, before quiet-hours snapping. - Sub-daily intervals keep their cadence from the previous next_check_at. If + Real-time checks keep their cadence from the previous next_check_at. If a check is late, the next check skips missed intervals and is after now. - Daily/weekly/monthly anchor to fixed local instants: 1am tomorrow, 3am next - Monday, 4am on the 1st of next month. Hour-only replacement keeps the - minute/second spread. + Daily/weekly/monthly anchor to fixed local hours: 1am tomorrow, 3am next + Monday, 4am on the 1st of next month. Except for real time, each check then + runs at the alert's offset after its local interval boundary. Explicit + schedule_start_time values keep the time the user selected. """ team_timezone = pytz.timezone(tz_name) local_now = now.astimezone(team_timezone) + offset = alert_check_offset(interval, alert_id) if schedule_start_time is not None: return _next_check_at_for_schedule_start_time( @@ -260,36 +303,43 @@ def next_calendar_check_time( ) match interval: - case CalendarInterval.REAL_TIME | CalendarInterval.EVERY_15_MINUTES | CalendarInterval.HOURLY: - interval_delta = { - CalendarInterval.REAL_TIME: timedelta(minutes=REAL_TIME_CADENCE_MINUTES), - CalendarInterval.EVERY_15_MINUTES: timedelta(minutes=EVERY_15_MINUTES_CADENCE_MINUTES), - CalendarInterval.HOURLY: timedelta(hours=1), - }[interval] + case CalendarInterval.REAL_TIME: + interval_delta = timedelta(minutes=REAL_TIME_CADENCE_MINUTES) candidate = (next_check_at or now) + interval_delta if candidate <= now: candidate += interval_delta * (int((now - candidate) // interval_delta) + 1) return candidate + case CalendarInterval.EVERY_15_MINUTES | CalendarInterval.HOURLY: + cadence_minutes = EVERY_15_MINUTES_CADENCE_MINUTES if interval == CalendarInterval.EVERY_15_MINUTES else 60 + interval_delta = timedelta(minutes=cadence_minutes) + # One cadence after the previous check lands in the next interval. The check runs at this + # alert's offset into that interval, which also moves an alert off the minute it was created on. + candidate = ( + _floor_to_local_period((next_check_at or now) + interval_delta, team_timezone, cadence_minutes) + offset + ) + if candidate <= now: + candidate += interval_delta * (int((now - candidate) // interval_delta) + 1) + return candidate case CalendarInterval.DAILY: return _calendar_anchor_utc( - local_now, team_timezone, target_date=local_now.date() + timedelta(days=1), hour=1, + offset=offset, ) case CalendarInterval.WEEKLY: return _calendar_anchor_utc( - local_now, team_timezone, target_date=local_now.date() + timedelta(days=7 - local_now.weekday()), hour=3, + offset=offset, ) case CalendarInterval.MONTHLY: if local_now.month == 12: target_date = date(local_now.year + 1, 1, 1) else: target_date = date(local_now.year, local_now.month + 1, 1) - return _calendar_anchor_utc(local_now, team_timezone, target_date=target_date, hour=4) + return _calendar_anchor_utc(team_timezone, target_date=target_date, hour=4, offset=offset) case _ as unreachable: raise ValueError(f"Unhandled alert calculation interval: {unreachable!r}") diff --git a/products/alerts/backend/test/test_scheduling.py b/products/alerts/backend/test/test_scheduling.py index 362ad6948af5..f65376540cc8 100644 --- a/products/alerts/backend/test/test_scheduling.py +++ b/products/alerts/backend/test/test_scheduling.py @@ -1,5 +1,6 @@ -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any +from uuid import UUID import pytest @@ -8,6 +9,7 @@ from products.alerts.backend.facade.scheduling import ( BlockedWindow, CalendarInterval, + alert_check_offset, is_weekend, next_calendar_check_time, parse_blocked_windows_tuples, @@ -19,6 +21,7 @@ # Wednesday 2026-03-18 12:00 UTC NOW = datetime(2026, 3, 18, 12, 0, tzinfo=UTC) PREV_CHECK = datetime(2026, 3, 18, 11, 47, tzinfo=UTC) +ALERT_ID = UUID("0193f3c6-2a4b-7d2e-8f00-3c1b5d7e9a10") class TestValidateAndNormalizeScheduleRestriction: @@ -134,6 +137,7 @@ def test_hourly_check_returns_to_the_custom_minute_after_a_quiet_hours_delay(sel now=datetime(2026, 4, 7, 7, 0, tzinfo=UTC), tz_name="UTC", next_check_at=datetime(2026, 4, 7, 7, 0, tzinfo=UTC), + alert_id=ALERT_ID, schedule_start_time="22:30", ) == datetime(2026, 4, 7, 8, 30, tzinfo=UTC) @@ -143,6 +147,7 @@ def test_hourly_alert_created_after_its_start_time_uses_the_first_future_check(s now=datetime(2026, 4, 6, 23, 50, tzinfo=UTC), tz_name="UTC", next_check_at=None, + alert_id=ALERT_ID, schedule_start_time="09:35", ) == datetime(2026, 4, 7, 0, 35, tzinfo=UTC) @@ -169,6 +174,7 @@ def test_next_check_uses_schedule_start_time_on_create( now=datetime(2026, 3, 18, 9, 30, tzinfo=UTC), tz_name="UTC", next_check_at=None, + alert_id=ALERT_ID, schedule_start_time=schedule_start_time, ) == expected @@ -226,6 +232,7 @@ def test_next_check_respects_the_cadence_after_an_anchor_edit( now=now, tz_name="UTC", next_check_at=datetime(2026, 3, 18, 9, 30, tzinfo=UTC), + alert_id=ALERT_ID, schedule_start_time="09:35", ) assert result == expected @@ -234,46 +241,84 @@ def test_next_check_respects_the_cadence_after_an_anchor_edit( class TestNextCalendarCheckTime: @parameterized.expand( [ - # Sub-daily intervals preserve their schedule phase and skip missed evaluations. + # Sub-daily intervals keep one check per interval and skip missed evaluations. Real time keeps + # its phase; the others run at the alert's offset into the interval after the previous check. ("real_time_from_prev", CalendarInterval.REAL_TIME, PREV_CHECK, datetime(2026, 3, 18, 12, 1, tzinfo=UTC)), ("real_time_first_check", CalendarInterval.REAL_TIME, None, datetime(2026, 3, 18, 12, 2, tzinfo=UTC)), ( "15min_from_prev", CalendarInterval.EVERY_15_MINUTES, PREV_CHECK, - datetime(2026, 3, 18, 12, 2, tzinfo=UTC), + datetime(2026, 3, 18, 12, 0, tzinfo=UTC), ), - ("hourly_from_prev", CalendarInterval.HOURLY, PREV_CHECK, datetime(2026, 3, 18, 12, 47, tzinfo=UTC)), + ("hourly_from_prev", CalendarInterval.HOURLY, PREV_CHECK, datetime(2026, 3, 18, 12, 0, tzinfo=UTC)), ( "hourly_skips_backlog", CalendarInterval.HOURLY, datetime(2026, 3, 18, 5, 47, tzinfo=UTC), - datetime(2026, 3, 18, 12, 47, tzinfo=UTC), + datetime(2026, 3, 18, 12, 0, tzinfo=UTC), ), ] ) def test_sub_daily_advances_from_previous( self, _name: str, interval: CalendarInterval, next_check_at: datetime | None, expected: datetime ) -> None: - result = next_calendar_check_time(interval, now=NOW, tz_name="UTC", next_check_at=next_check_at) - assert result == expected + result = next_calendar_check_time( + interval, now=NOW, tz_name="UTC", next_check_at=next_check_at, alert_id=ALERT_ID + ) + assert result == expected + alert_check_offset(interval, ALERT_ID) + + @parameterized.expand( + [ + # name, interval, cadence, first minute of the window, first minute after it + ("every_15_minutes", CalendarInterval.EVERY_15_MINUTES, timedelta(minutes=15), 1, 4), + ("hourly", CalendarInterval.HOURLY, timedelta(hours=1), 2, 14), + ("daily", CalendarInterval.DAILY, timedelta(days=1), 2, 60), + ("weekly", CalendarInterval.WEEKLY, timedelta(weeks=1), 2, 60), + ] + ) + def test_each_alert_checks_at_its_own_minute_inside_the_window( + self, _name: str, interval: CalendarInterval, cadence: timedelta, first_minute: int, end_minute: int + ) -> None: + minute_period = int(min(cadence, timedelta(hours=1)).total_seconds() // 60) + minutes_used: set[int] = set() + for index in range(600): + alert_id = UUID(int=index) + check = next_calendar_check_time(interval, now=NOW, tz_name="UTC", next_check_at=None, alert_id=alert_id) + following = next_calendar_check_time( + interval, now=check, tz_name="UTC", next_check_at=check, alert_id=alert_id + ) + late_now = check + cadence * 2.5 + late = next_calendar_check_time( + interval, now=late_now, tz_name="UTC", next_check_at=check, alert_id=alert_id + ) + + minute = check.minute % minute_period + assert first_minute <= minute < end_minute + assert following - check == cadence + assert late > late_now + assert (late.minute % minute_period, late.second) == (minute, check.second) + minutes_used.add(minute) + + assert minutes_used == set(range(first_minute, end_minute)) @parameterized.expand( [ - # Daily anchors to ~1am local tomorrow; minute preserved for spread. US/Pacific is UTC-7 on this date. - ("daily_pacific", CalendarInterval.DAILY, "US/Pacific", (2026, 3, 19, 8, 0)), - # Weekly anchors to ~3am next Monday local (Mon 2026-03-23), 3am PDT = 10:00 UTC - ("weekly_pacific", CalendarInterval.WEEKLY, "US/Pacific", (2026, 3, 23, 10, 0)), - # Monthly anchors to ~4am on the 1st of next month, 4am PDT = 11:00 UTC - ("monthly_pacific", CalendarInterval.MONTHLY, "US/Pacific", (2026, 4, 1, 11, 0)), + # Daily anchors to the 1am hour local tomorrow. US/Pacific is UTC-7 on this date. + ("daily_pacific", CalendarInterval.DAILY, "US/Pacific", datetime(2026, 3, 19, 8, 0, tzinfo=UTC)), + # Weekly anchors to the 3am hour next Monday local (Mon 2026-03-23), 3am PDT = 10:00 UTC + ("weekly_pacific", CalendarInterval.WEEKLY, "US/Pacific", datetime(2026, 3, 23, 10, 0, tzinfo=UTC)), + # Monthly anchors to the 4am hour on the 1st of next month, 4am PDT = 11:00 UTC + ("monthly_pacific", CalendarInterval.MONTHLY, "US/Pacific", datetime(2026, 4, 1, 11, 0, tzinfo=UTC)), ] ) def test_calendar_anchors_in_team_timezone( - self, _name: str, interval: CalendarInterval, tz_name: str, expected_utc: tuple + self, _name: str, interval: CalendarInterval, tz_name: str, anchor: datetime ) -> None: - result = next_calendar_check_time(interval, now=NOW, tz_name=tz_name, next_check_at=PREV_CHECK) - assert (result.year, result.month, result.day, result.hour, result.minute) == expected_utc - assert result.tzinfo is not None + result = next_calendar_check_time( + interval, now=NOW, tz_name=tz_name, next_check_at=PREV_CHECK, alert_id=ALERT_ID + ) + assert result == anchor + alert_check_offset(interval, ALERT_ID) def test_daily_across_dst_spring_forward(self) -> None: # US spring-forward was 2026-03-08: local 1am tomorrow maps PST(-8) -> PDT(-7), @@ -283,12 +328,14 @@ def test_daily_across_dst_spring_forward(self) -> None: now=datetime(2026, 3, 7, 12, 0, tzinfo=UTC), tz_name="US/Pacific", next_check_at=None, + alert_id=ALERT_ID, ) after = next_calendar_check_time( CalendarInterval.DAILY, now=datetime(2026, 3, 8, 12, 0, tzinfo=UTC), tz_name="US/Pacific", next_check_at=None, + alert_id=ALERT_ID, ) assert before.hour == 9 assert after.hour == 8 @@ -312,7 +359,9 @@ def test_daily_across_dst_spring_forward(self) -> None: def test_calendar_anchors_keep_local_wall_time_across_dst( self, _name: str, interval: CalendarInterval, now: datetime, expected: datetime ) -> None: - assert next_calendar_check_time(interval, now=now, tz_name="America/New_York", next_check_at=None) == expected + assert next_calendar_check_time( + interval, now=now, tz_name="America/New_York", next_check_at=None, alert_id=ALERT_ID + ) == expected + alert_check_offset(interval, ALERT_ID) class TestIsWeekend: diff --git a/products/alerts/backend/tests/api/test_alert.py b/products/alerts/backend/tests/api/test_alert.py index 751360bb7deb..8a82372b29f7 100644 --- a/products/alerts/backend/tests/api/test_alert.py +++ b/products/alerts/backend/tests/api/test_alert.py @@ -27,6 +27,7 @@ from products.alerts.backend.facade.api import INSIGHT_ALERT_EVENT_IDS, LLMDetectorUnavailableError from products.alerts.backend.facade.contracts import AlertDelivery from products.alerts.backend.facade.destinations import MAX_DESTINATIONS_PER_ALERT, count_active_alert_destinations +from products.alerts.backend.facade.scheduling import CalendarInterval, alert_check_offset from products.alerts.backend.judge.verdict import LLMDetectionVerdict from products.alerts.backend.logic.insight_alert_destinations import SLACK_TEMPLATE_ID from products.alerts.backend.models.alert import AlertCheck, AlertConfiguration, AlertSubscription, Threshold @@ -1445,7 +1446,9 @@ def test_patch_schedule_restriction_snaps_next_check_to_first_minute_outside_qui ) assert response.status_code == status.HTTP_200_OK, response.content nxt = response.json()["next_check_at"] - assert datetime.fromisoformat(nxt.replace("Z", "+00:00")) == datetime(2026, 4, 6, 16, 0, 0, tzinfo=UTC) + assert datetime.fromisoformat(nxt.replace("Z", "+00:00")) == datetime( + 2026, 4, 6, 16, 0, 0, tzinfo=UTC + ) + alert_check_offset(CalendarInterval.HOURLY, alert["id"]) def test_patch_schedule_restriction_empty_normalizes_to_null(self) -> None: creation_request = { diff --git a/products/alerts/frontend/components/AlertIntervalRow.tsx b/products/alerts/frontend/components/AlertIntervalRow.tsx index 9701a0407c54..e6623ec32d9f 100644 --- a/products/alerts/frontend/components/AlertIntervalRow.tsx +++ b/products/alerts/frontend/components/AlertIntervalRow.tsx @@ -100,15 +100,22 @@ export function AlertIntervalRow({ if (alertForm.calculation_interval === AlertCalculationInterval.REAL_TIME) { nextEvaluation = null } else if (creatingNewAlert || nextPlannedEvaluationStale) { - const approximateTime = approximateNextAlertRun( + const { earliest, latest } = approximateNextAlertRun( alertForm.calculation_interval, currentTeam?.timezone ?? 'UTC', alertForm.schedule_start_time ) nextEvaluation = ( - - Approximately + + {earliest.isSame(latest) ? 'Approximately' : 'Approximately between'} + + {!earliest.isSame(latest) && ( + <> + and + + + )} ) @@ -199,6 +206,12 @@ export function AlertIntervalRow({ {evaluatedWindow} {nextEvaluation} + {alertForm.calculation_interval !== AlertCalculationInterval.REAL_TIME && + !alertForm.schedule_start_time && ( +

+ Automatic checks use a consistent minute for each alert so evaluations are spread out. +

+ )} ) } diff --git a/products/alerts/frontend/logic/alertSchedulingStale.test.ts b/products/alerts/frontend/logic/alertSchedulingStale.test.ts index fb1711553190..efc5244f7ca3 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.test.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.test.ts @@ -12,16 +12,18 @@ import { describe('alertSchedulingStale', () => { describe('approximateNextAlertRun', () => { it.each([ - [AlertCalculationInterval.REAL_TIME, '2026-07-24T16:02:00.000Z'], - [AlertCalculationInterval.EVERY_15_MINUTES, '2026-07-24T16:15:00.000Z'], - [AlertCalculationInterval.HOURLY, '2026-07-24T17:00:00.000Z'], - [AlertCalculationInterval.DAILY, '2026-07-25T05:00:00.000Z'], - [AlertCalculationInterval.WEEKLY, '2026-07-27T07:00:00.000Z'], - [AlertCalculationInterval.MONTHLY, '2026-08-01T08:00:00.000Z'], - ])('matches the backend anchor for %s', (interval, expected) => { - const now = dayjs.utc('2026-07-24T16:00:00.000Z') + [AlertCalculationInterval.REAL_TIME, '2026-07-24T16:09:00.000Z', '2026-07-24T16:09:00.000Z'], + [AlertCalculationInterval.EVERY_15_MINUTES, '2026-07-24T16:16:00.000Z', '2026-07-24T16:18:00.000Z'], + [AlertCalculationInterval.HOURLY, '2026-07-24T17:02:00.000Z', '2026-07-24T17:13:00.000Z'], + [AlertCalculationInterval.DAILY, '2026-07-25T05:02:00.000Z', '2026-07-25T05:59:00.000Z'], + [AlertCalculationInterval.WEEKLY, '2026-07-27T07:02:00.000Z', '2026-07-27T07:59:00.000Z'], + [AlertCalculationInterval.MONTHLY, '2026-08-01T08:02:00.000Z', '2026-08-01T08:59:00.000Z'], + ])('matches the backend window for %s', (interval, earliest, latest) => { + const now = dayjs.utc('2026-07-24T16:07:00.000Z') - expect(approximateNextAlertRun(interval, 'America/Toronto', null, now).toISOString()).toBe(expected) + const result = approximateNextAlertRun(interval, 'America/Toronto', null, now) + expect(result.earliest.toISOString()).toBe(earliest) + expect(result.latest.toISOString()).toBe(latest) }) it.each([ @@ -30,19 +32,35 @@ describe('alertSchedulingStale', () => { '00:55', '2026-07-24T16:30:00.000Z', '2026-07-24T16:55:00.000Z', + '2026-07-24T16:55:00.000Z', ], [ AlertCalculationInterval.EVERY_15_MINUTES, '00:55', '2026-07-24T16:55:00.000Z', '2026-07-24T17:10:00.000Z', + '2026-07-24T17:10:00.000Z', + ], + [ + AlertCalculationInterval.HOURLY, + '00:55', + '2026-07-24T16:30:00.000Z', + '2026-07-24T16:55:00.000Z', + '2026-07-24T16:55:00.000Z', + ], + [ + AlertCalculationInterval.HOURLY, + '00:NaN', + '2026-07-24T16:30:00.000Z', + '2026-07-24T17:02:00.000Z', + '2026-07-24T17:13:00.000Z', ], - [AlertCalculationInterval.HOURLY, '00:55', '2026-07-24T16:30:00.000Z', '2026-07-24T16:55:00.000Z'], - [AlertCalculationInterval.HOURLY, '00:NaN', '2026-07-24T16:30:00.000Z', '2026-07-24T17:30:00.000Z'], - ])('uses %s schedule start time %s', (interval, scheduleStartTime, nowValue, expected) => { + ])('uses %s schedule start time %s', (interval, scheduleStartTime, nowValue, expected, expectedLatest) => { const now = dayjs.utc(nowValue) - expect(approximateNextAlertRun(interval, 'UTC', scheduleStartTime, now).toISOString()).toBe(expected) + const result = approximateNextAlertRun(interval, 'UTC', scheduleStartTime, now) + expect(result.earliest.toISOString()).toBe(expected) + expect(result.latest.toISOString()).toBe(expectedLatest) }) }) diff --git a/products/alerts/frontend/logic/alertSchedulingStale.ts b/products/alerts/frontend/logic/alertSchedulingStale.ts index 66e83de03397..3635bd96b10b 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.ts @@ -15,7 +15,7 @@ export function approximateNextAlertRun( timezone: string, scheduleStartTime: string | null | undefined = null, now: Dayjs = dayjs() -): Dayjs { +): { earliest: Dayjs; latest: Dayjs } { let localNow: Dayjs try { localNow = now.tz(timezone) @@ -42,22 +42,38 @@ export function approximateNextAlertRun( return candidate } + if (interval === AlertCalculationInterval.REAL_TIME) { + const nextRun = localNow.add(2, 'minutes') + return { earliest: nextRun, latest: nextRun } + } + if (interval === AlertCalculationInterval.EVERY_15_MINUTES || interval === AlertCalculationInterval.HOURLY) { + const cadence = interval === AlertCalculationInterval.EVERY_15_MINUTES ? 15 : 60 + const customRun = nextRunFromScheduleStartMinute(cadence) + if (customRun) { + return { earliest: customRun, latest: customRun } + } + const anchor = localNow.startOf('minute').add(cadence - (localNow.minute() % cadence), 'minutes') + return { + earliest: anchor.add(cadence === 15 ? 1 : 2, 'minutes'), + latest: anchor.add(cadence === 15 ? 3 : 13, 'minutes'), + } + } + + let anchor: Dayjs switch (interval) { - case AlertCalculationInterval.REAL_TIME: - return localNow.add(2, 'minutes') - case AlertCalculationInterval.EVERY_15_MINUTES: - return nextRunFromScheduleStartMinute(15) ?? localNow.add(15, 'minutes') - case AlertCalculationInterval.HOURLY: - return nextRunFromScheduleStartMinute(60) ?? localNow.add(1, 'hour') case AlertCalculationInterval.DAILY: - return calendarAnchor(localNow.add(1, 'day'), 1, timezone) + anchor = calendarAnchor(localNow.add(1, 'day'), 1, timezone) + break case AlertCalculationInterval.WEEKLY: { const daysUntilMonday = localNow.day() === 0 ? 1 : 8 - localNow.day() - return calendarAnchor(localNow.add(daysUntilMonday, 'days'), 3, timezone) + anchor = calendarAnchor(localNow.add(daysUntilMonday, 'days'), 3, timezone) + break } case AlertCalculationInterval.MONTHLY: - return calendarAnchor(localNow.add(1, 'month').startOf('month'), 4, timezone) + anchor = calendarAnchor(localNow.add(1, 'month').startOf('month'), 4, timezone) + break } + return { earliest: anchor.add(2, 'minutes'), latest: anchor.add(59, 'minutes') } } export function normalizeScheduleRestrictionForCompare( From 03457d813b91ff62574bb199f364e3ae7d602ed0 Mon Sep 17 00:00:00 2001 From: Sandy Spicer Date: Sun, 27 Sep 2026 23:13:09 -0700 Subject: [PATCH 2/4] fix(alerts): find local hour starts on the wall clock across DST Hourly and 15-minute checks found the interval start by subtracting the local minutes from the absolute time, and caught up by adding whole intervals. With Lord Howe's 30-minute DST change that ran a second check in the same local hour and skipped the next one. The start now comes from the local wall clock, and the next check is the first local interval start after the previous check. Co-Authored-By: Claude Opus 5.5 --- products/alerts/backend/facade/scheduling.py | 45 +++++++++++----- .../alerts/backend/test/test_scheduling.py | 53 +++++++++++++++++-- 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/products/alerts/backend/facade/scheduling.py b/products/alerts/backend/facade/scheduling.py index 61f2bdf93611..73bf2d2cf3e0 100644 --- a/products/alerts/backend/facade/scheduling.py +++ b/products/alerts/backend/facade/scheduling.py @@ -156,12 +156,12 @@ def is_weekend(now: datetime, tz_name: str) -> bool: return now_local.isoweekday() in [6, 7] -def _localize_wall_time(team_timezone: BaseTzInfo, naive_local: datetime) -> datetime: +def _localize_wall_time(team_timezone: BaseTzInfo, naive_local: datetime, *, repeated_is_dst: bool = True) -> datetime: localize = cast(Callable[[datetime, bool | None], datetime], team_timezone.localize) try: return localize(naive_local, None) except AmbiguousTimeError: - return localize(naive_local, True) + return localize(naive_local, repeated_is_dst) except NonExistentTimeError: return team_timezone.normalize(localize(naive_local, False)) @@ -177,15 +177,35 @@ def _calendar_anchor_utc( return _localize_wall_time(team_timezone, naive_local).astimezone(UTC) -def _floor_to_local_period(timestamp: datetime, team_timezone: BaseTzInfo, period_minutes: int) -> datetime: +def _local_period_start(timestamp: datetime, team_timezone: BaseTzInfo, period_minutes: int) -> datetime: """Start of the local-time period of `period_minutes` that contains `timestamp`. - The subtraction runs on the absolute instant, so a local hour that a DST change repeats - still resolves to the start of the hour that `timestamp` is in. + The start is found on the local wall clock. A 30-minute DST change (Australia/Lord_Howe) moves the + wall clock to the half hour, so subtracting the local minutes from the absolute instant would find + a start in the previous hour. A start that a DST change repeats resolves to the occurrence that + `timestamp` is in. A start that a DST change skips moves forward by the size of the change, which + is the instant of the change when the change happens on a period boundary. """ local = timestamp.astimezone(team_timezone) - minutes_into_period = (local.hour * 60 + local.minute) % period_minutes - return timestamp - timedelta(minutes=minutes_into_period, seconds=local.second, microseconds=local.microsecond) + wall_start = local.replace( + tzinfo=None, minute=local.minute - local.minute % period_minutes, second=0, microsecond=0 + ) + return _localize_wall_time(team_timezone, wall_start, repeated_is_dst=bool(local.dst())).astimezone(UTC) + + +def _next_local_period_start(after: datetime, team_timezone: BaseTzInfo, period_minutes: int) -> datetime: + """First start of a local-time period of `period_minutes` that is later than `after`. + + A DST change can make a local period longer than `period_minutes`, so one step can land in the + same period again. The loop steps until it reaches a later period. + """ + step = timedelta(minutes=period_minutes) + probe = after + step + start = _local_period_start(probe, team_timezone, period_minutes) + while start <= after: + probe += step + start = _local_period_start(probe, team_timezone, period_minutes) + return start def _next_check_at_for_schedule_start_time( @@ -311,14 +331,11 @@ def next_calendar_check_time( return candidate case CalendarInterval.EVERY_15_MINUTES | CalendarInterval.HOURLY: cadence_minutes = EVERY_15_MINUTES_CADENCE_MINUTES if interval == CalendarInterval.EVERY_15_MINUTES else 60 - interval_delta = timedelta(minutes=cadence_minutes) - # One cadence after the previous check lands in the next interval. The check runs at this - # alert's offset into that interval, which also moves an alert off the minute it was created on. - candidate = ( - _floor_to_local_period((next_check_at or now) + interval_delta, team_timezone, cadence_minutes) + offset - ) + # The check runs at this alert's offset into the first local interval after the previous check, + # which also moves an alert off the minute it was created on. + candidate = _next_local_period_start(next_check_at or now, team_timezone, cadence_minutes) + offset if candidate <= now: - candidate += interval_delta * (int((now - candidate) // interval_delta) + 1) + candidate = _next_local_period_start(now - offset, team_timezone, cadence_minutes) + offset return candidate case CalendarInterval.DAILY: return _calendar_anchor_utc( diff --git a/products/alerts/backend/test/test_scheduling.py b/products/alerts/backend/test/test_scheduling.py index f65376540cc8..b4df2a22ae24 100644 --- a/products/alerts/backend/test/test_scheduling.py +++ b/products/alerts/backend/test/test_scheduling.py @@ -345,23 +345,68 @@ def test_daily_across_dst_spring_forward(self) -> None: ( "weekly_spring_forward", CalendarInterval.WEEKLY, + "America/New_York", datetime(2026, 3, 6, 12, 0, tzinfo=UTC), + None, datetime(2026, 3, 9, 7, 0, tzinfo=UTC), ), ( "monthly_fall_back", CalendarInterval.MONTHLY, + "America/New_York", datetime(2026, 10, 31, 12, 0, tzinfo=UTC), + None, datetime(2026, 11, 1, 9, 0, tzinfo=UTC), ), + ( + "hourly_repeated_hour", + CalendarInterval.HOURLY, + "America/New_York", + datetime(2026, 11, 1, 5, 3, tzinfo=UTC), + datetime(2026, 11, 1, 5, 3, tzinfo=UTC), + datetime(2026, 11, 1, 6, 0, tzinfo=UTC), + ), + # Lord Howe moves its clocks by 30 minutes at 02:00. In October the 02:00 hour starts at 02:30 + # (15:30 UTC), one hour after the 01:00 hour starts. + ( + "hourly_half_hour_spring_forward", + CalendarInterval.HOURLY, + "Australia/Lord_Howe", + datetime(2026, 10, 3, 14, 33, tzinfo=UTC), + datetime(2026, 10, 3, 14, 33, tzinfo=UTC), + datetime(2026, 10, 3, 15, 30, tzinfo=UTC), + ), + # In April the 01:00 hour lasts 90 minutes, so the 02:00 hour starts at 15:30 UTC. + ( + "hourly_half_hour_fall_back", + CalendarInterval.HOURLY, + "Australia/Lord_Howe", + datetime(2026, 4, 4, 14, 3, tzinfo=UTC), + datetime(2026, 4, 4, 14, 3, tzinfo=UTC), + datetime(2026, 4, 4, 15, 30, tzinfo=UTC), + ), + ( + "hourly_half_hour_fall_back_late", + CalendarInterval.HOURLY, + "Australia/Lord_Howe", + datetime(2026, 4, 4, 14, 40, tzinfo=UTC), + datetime(2026, 4, 4, 13, 3, tzinfo=UTC), + datetime(2026, 4, 4, 15, 30, tzinfo=UTC), + ), ] ) - def test_calendar_anchors_keep_local_wall_time_across_dst( - self, _name: str, interval: CalendarInterval, now: datetime, expected: datetime + def test_checks_keep_local_wall_time_across_dst( + self, + _name: str, + interval: CalendarInterval, + tz_name: str, + now: datetime, + next_check_at: datetime | None, + interval_start: datetime, ) -> None: assert next_calendar_check_time( - interval, now=now, tz_name="America/New_York", next_check_at=None, alert_id=ALERT_ID - ) == expected + alert_check_offset(interval, ALERT_ID) + interval, now=now, tz_name=tz_name, next_check_at=next_check_at, alert_id=ALERT_ID + ) == interval_start + alert_check_offset(interval, ALERT_ID) class TestIsWeekend: From 4803cf8cf7012bd4cbd9e907313085ec8c1e6d8b Mon Sep 17 00:00:00 2001 From: Sandy Spicer Date: Sun, 27 Sep 2026 23:13:26 -0700 Subject: [PATCH 3/4] fix(alerts): show the explicit start time in the calendar alert preview A daily, weekly, or monthly alert with a schedule start time runs at exactly that local time, but the edit form preview showed the automatic window instead. Co-Authored-By: Claude Opus 5.5 --- .../logic/alertSchedulingStale.test.ts | 21 +++++++ .../frontend/logic/alertSchedulingStale.ts | 61 +++++++++++++++---- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/products/alerts/frontend/logic/alertSchedulingStale.test.ts b/products/alerts/frontend/logic/alertSchedulingStale.test.ts index efc5244f7ca3..e4224fe400d5 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.test.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.test.ts @@ -55,6 +55,27 @@ describe('alertSchedulingStale', () => { '2026-07-24T17:02:00.000Z', '2026-07-24T17:13:00.000Z', ], + [ + AlertCalculationInterval.DAILY, + '09:35', + '2026-07-24T08:00:00.000Z', + '2026-07-24T09:35:00.000Z', + '2026-07-24T09:35:00.000Z', + ], + [ + AlertCalculationInterval.WEEKLY, + '09:35', + '2026-07-24T16:30:00.000Z', + '2026-07-27T09:35:00.000Z', + '2026-07-27T09:35:00.000Z', + ], + [ + AlertCalculationInterval.MONTHLY, + '09:35', + '2026-07-24T16:30:00.000Z', + '2026-08-01T09:35:00.000Z', + '2026-08-01T09:35:00.000Z', + ], ])('uses %s schedule start time %s', (interval, scheduleStartTime, nowValue, expected, expectedLatest) => { const now = dayjs.utc(nowValue) diff --git a/products/alerts/frontend/logic/alertSchedulingStale.ts b/products/alerts/frontend/logic/alertSchedulingStale.ts index 3635bd96b10b..ac22dc7746a0 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.ts @@ -6,8 +6,20 @@ import { AlertCalculationInterval } from '~/queries/schema/schema-general' import type { ScheduleRestriction } from '../types' -function calendarAnchor(localDate: Dayjs, hour: number, timezone: string): Dayjs { - return dayjs.tz(`${localDate.format('YYYY-MM-DD')} ${hour}:00`, 'YYYY-MM-DD H:mm', timezone) +function calendarTime(localDate: Dayjs, hour: number, minute: number, timezone: string): Dayjs { + return dayjs.tz( + `${localDate.format('YYYY-MM-DD')} ${hour}:${String(minute).padStart(2, '0')}`, + 'YYYY-MM-DD H:mm', + timezone + ) +} + +function parseScheduleStartTime(scheduleStartTime: string | null | undefined): { hour: number; minute: number } | null { + const [hour, minute] = (scheduleStartTime ?? '').split(':').map(Number) + if (!Number.isInteger(hour) || !Number.isInteger(minute) || hour < 0 || hour > 23 || minute < 0 || minute > 59) { + return null + } + return { hour, minute } } export function approximateNextAlertRun( @@ -24,18 +36,13 @@ export function approximateNextAlertRun( localNow = now.utc() } - const scheduleStartMinute = scheduleStartTime ? Number(scheduleStartTime.split(':')[1]) : undefined + const scheduleStart = parseScheduleStartTime(scheduleStartTime) const nextRunFromScheduleStartMinute = (cadenceMinutes: number): Dayjs | null => { - if ( - scheduleStartMinute === undefined || - !Number.isInteger(scheduleStartMinute) || - scheduleStartMinute < 0 || - scheduleStartMinute > 59 - ) { + if (!scheduleStart) { return null } - let candidate = localNow.startOf('hour').minute(scheduleStartMinute).second(0).millisecond(0) + let candidate = localNow.startOf('hour').minute(scheduleStart.minute).second(0).millisecond(0) while (!candidate.isAfter(localNow)) { candidate = candidate.add(cadenceMinutes, 'minutes') } @@ -59,18 +66,46 @@ export function approximateNextAlertRun( } } + if (scheduleStart) { + // An explicit start time runs at exactly that local time: today, this Monday, or the 1st of this + // month, moved on by whole days, weeks, or months until it is in the future. + let firstDate: Dayjs + let unit: 'day' | 'week' | 'month' + switch (interval) { + case AlertCalculationInterval.DAILY: + firstDate = localNow + unit = 'day' + break + case AlertCalculationInterval.WEEKLY: + firstDate = localNow.add((8 - localNow.day()) % 7, 'days') + unit = 'week' + break + case AlertCalculationInterval.MONTHLY: + firstDate = localNow.startOf('month') + unit = 'month' + break + } + let steps = 0 + let run = calendarTime(firstDate, scheduleStart.hour, scheduleStart.minute, timezone) + while (!run.isAfter(localNow)) { + steps += 1 + run = calendarTime(firstDate.add(steps, unit), scheduleStart.hour, scheduleStart.minute, timezone) + } + return { earliest: run, latest: run } + } + let anchor: Dayjs switch (interval) { case AlertCalculationInterval.DAILY: - anchor = calendarAnchor(localNow.add(1, 'day'), 1, timezone) + anchor = calendarTime(localNow.add(1, 'day'), 1, 0, timezone) break case AlertCalculationInterval.WEEKLY: { const daysUntilMonday = localNow.day() === 0 ? 1 : 8 - localNow.day() - anchor = calendarAnchor(localNow.add(daysUntilMonday, 'days'), 3, timezone) + anchor = calendarTime(localNow.add(daysUntilMonday, 'days'), 3, 0, timezone) break } case AlertCalculationInterval.MONTHLY: - anchor = calendarAnchor(localNow.add(1, 'month').startOf('month'), 4, timezone) + anchor = calendarTime(localNow.add(1, 'month').startOf('month'), 4, 0, timezone) break } return { earliest: anchor.add(2, 'minutes'), latest: anchor.add(59, 'minutes') } From dabdd6899e3bebfb93b8ac8a4555b508dc6feb16 Mon Sep 17 00:00:00 2001 From: Sandy Spicer Date: Sun, 27 Sep 2026 23:50:38 -0700 Subject: [PATCH 4/4] fix(alerts): preview repeated local start times at their first occurrence When a DST change repeats an alert's start time, the backend runs at the first occurrence. dayjs picked either occurrence depending on the browser's timezone, and its isAfter re-reads the wall time, so the preview could show a run that had already passed. Co-Authored-By: Claude Opus 5.5 --- .../logic/alertSchedulingStale.test.ts | 32 +++++++++++++++---- .../frontend/logic/alertSchedulingStale.ts | 22 +++++++++---- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/products/alerts/frontend/logic/alertSchedulingStale.test.ts b/products/alerts/frontend/logic/alertSchedulingStale.test.ts index e4224fe400d5..7465f88dc639 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.test.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.test.ts @@ -33,6 +33,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:30:00.000Z', '2026-07-24T16:55:00.000Z', '2026-07-24T16:55:00.000Z', + 'UTC', ], [ AlertCalculationInterval.EVERY_15_MINUTES, @@ -40,6 +41,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:55:00.000Z', '2026-07-24T17:10:00.000Z', '2026-07-24T17:10:00.000Z', + 'UTC', ], [ AlertCalculationInterval.HOURLY, @@ -47,6 +49,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:30:00.000Z', '2026-07-24T16:55:00.000Z', '2026-07-24T16:55:00.000Z', + 'UTC', ], [ AlertCalculationInterval.HOURLY, @@ -54,6 +57,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:30:00.000Z', '2026-07-24T17:02:00.000Z', '2026-07-24T17:13:00.000Z', + 'UTC', ], [ AlertCalculationInterval.DAILY, @@ -61,6 +65,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T08:00:00.000Z', '2026-07-24T09:35:00.000Z', '2026-07-24T09:35:00.000Z', + 'UTC', ], [ AlertCalculationInterval.WEEKLY, @@ -68,6 +73,7 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:30:00.000Z', '2026-07-27T09:35:00.000Z', '2026-07-27T09:35:00.000Z', + 'UTC', ], [ AlertCalculationInterval.MONTHLY, @@ -75,14 +81,28 @@ describe('alertSchedulingStale', () => { '2026-07-24T16:30:00.000Z', '2026-08-01T09:35:00.000Z', '2026-08-01T09:35:00.000Z', + 'UTC', ], - ])('uses %s schedule start time %s', (interval, scheduleStartTime, nowValue, expected, expectedLatest) => { - const now = dayjs.utc(nowValue) + // Lord Howe repeats 01:30 to 02:00 on 5 April. At 15:05 UTC the first 01:45 (14:45 UTC) has passed, + // so the backend runs the next day. + [ + AlertCalculationInterval.DAILY, + '01:45', + '2026-04-04T15:05:00.000Z', + '2026-04-05T15:15:00.000Z', + '2026-04-05T15:15:00.000Z', + 'Australia/Lord_Howe', + ], + ])( + 'uses %s schedule start time %s', + (interval, scheduleStartTime, nowValue, expected, expectedLatest, timezone) => { + const now = dayjs.utc(nowValue) - const result = approximateNextAlertRun(interval, 'UTC', scheduleStartTime, now) - expect(result.earliest.toISOString()).toBe(expected) - expect(result.latest.toISOString()).toBe(expectedLatest) - }) + const result = approximateNextAlertRun(interval, timezone, scheduleStartTime, now) + expect(result.earliest.toISOString()).toBe(expected) + expect(result.latest.toISOString()).toBe(expectedLatest) + } + ) }) describe('normalizeScheduleRestrictionForCompare', () => { diff --git a/products/alerts/frontend/logic/alertSchedulingStale.ts b/products/alerts/frontend/logic/alertSchedulingStale.ts index ac22dc7746a0..a57bde0d8604 100644 --- a/products/alerts/frontend/logic/alertSchedulingStale.ts +++ b/products/alerts/frontend/logic/alertSchedulingStale.ts @@ -7,11 +7,19 @@ import { AlertCalculationInterval } from '~/queries/schema/schema-general' import type { ScheduleRestriction } from '../types' function calendarTime(localDate: Dayjs, hour: number, minute: number, timezone: string): Dayjs { - return dayjs.tz( - `${localDate.format('YYYY-MM-DD')} ${hour}:${String(minute).padStart(2, '0')}`, - 'YYYY-MM-DD H:mm', - timezone - ) + const wallTime = `${localDate.format('YYYY-MM-DD')} ${hour}:${String(minute).padStart(2, '0')}` + const run = dayjs.tz(wallTime, 'YYYY-MM-DD H:mm', timezone) + // When a DST change repeats this wall time, dayjs picks either occurrence depending on the browser's + // timezone. The backend uses the first occurrence, which has the larger UTC offset. + const shiftMinutes = run.subtract(3, 'hours').tz(timezone).utcOffset() - run.utcOffset() + const firstOccurrence = run.subtract(shiftMinutes, 'minutes').tz(timezone) + return shiftMinutes > 0 && firstOccurrence.format('YYYY-MM-DD H:mm') === wallTime ? firstOccurrence : run +} + +// Compare instants, because dayjs re-reads the wall time inside `isAfter` and can pick the other occurrence of a +// repeated local time. +function isLater(time: Dayjs, than: Dayjs): boolean { + return time.valueOf() > than.valueOf() } function parseScheduleStartTime(scheduleStartTime: string | null | undefined): { hour: number; minute: number } | null { @@ -43,7 +51,7 @@ export function approximateNextAlertRun( } let candidate = localNow.startOf('hour').minute(scheduleStart.minute).second(0).millisecond(0) - while (!candidate.isAfter(localNow)) { + while (!isLater(candidate, localNow)) { candidate = candidate.add(cadenceMinutes, 'minutes') } return candidate @@ -87,7 +95,7 @@ export function approximateNextAlertRun( } let steps = 0 let run = calendarTime(firstDate, scheduleStart.hour, scheduleStart.minute, timezone) - while (!run.isAfter(localNow)) { + while (!isLater(run, localNow)) { steps += 1 run = calendarTime(firstDate.add(steps, unit), scheduleStart.hour, scheduleStart.minute, timezone) }