Skip to content

fix(backend): background-job UTC date skew + retire redundant Discord notifier - #211

Merged
thomasluizon merged 2 commits into
mainfrom
fix/background-job-utc-date-skew
Jun 20, 2026
Merged

fix(backend): background-job UTC date skew + retire redundant Discord notifier#211
thomasluizon merged 2 commits into
mainfrom
fix/background-job-utc-date-skew

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Jun 20, 2026

Copy link
Copy Markdown
Owner

What & why

Production was throwing a recurring 500 (surfaced via the new Sentry pipeline): the reminder background job's batched INSERT into Notifications + SentReminders kept hitting the SentReminders unique dedup index.

Root cause: ReminderSchedulerService.ProcessRelativeReminders deduped its bulk pre-check against Date == utcToday, but HabitLog.Date and SentReminder.Date store the user's local date (set via UserDateService.GetUserTodayAsync). For any user whose local calendar date differs from UTC's (e.g. Brazil, UTC-3, after UTC midnight), the pre-check looked in the wrong day bucket, missed the already-sent row, and re-attempted the insert on every 1-minute tick → unique-violation → EF logs it at Error → Sentry alert. The collision is caught gracefully (no duplicate push), so no user-facing harm — but a steady stream of error noise. The sibling ProcessScheduledReminders already handled this with a ±1-day window.

Changes

  • ReminderSchedulerService.ProcessRelativeReminders — query the utcToday-1 .. utcToday+1 window and key both the dedup set and the logged-habit set on the user's local date (mirrors ProcessScheduledReminders). Fixes the SentReminder.Date skew (the error) and the parallel HabitLog.Date skew (was sending reminders for already-logged habits to off-UTC users).
  • GoalDeadlineNotificationService / StreakGoalSyncService — found by a full audit of the same bug class. Both load streak logs with a UTC-derived lower bound against the user-local HabitLog.Date. Widened the bound by one day so a far-west user's oldest in-window logs aren't clipped. Pure widening — can't overcount (the streak walk still uses userToday).

Inspected and intentionally NOT changed

  • HabitDueDateAdvancementService — its bulk filter uses a UTC ConservativeCutoffUtc(), but there is a separate authoritative per-user check ShouldAdvanceForUserToday (DueDate < userToday). The UTC pre-filter only ever delays a deliberately-conservative process by ≤1 day; it never advances wrongly. Making it more aggressive risks the "re-hides overdue" regression. Left as-is by design.

Also: observability cleanup (retire code-level Discord notifier)

With orbit-api added to the Sentry→Discord alert rule, the bespoke DiscordAlertNotifier is redundant — UnhandledExceptionHandler already SentrySdk.CaptureExceptions every 500, and the notifier double-pinged Discord on 500s. Removed so all three projects alert through one mechanism (Sentry → Discord), with messages linking to full issue context.

Deletes DiscordAlertNotifier, IAlertNotifier, DiscordAlertSettings, the "Discord" HttpClient + DiscordAlerts config section, and the now-unused notifier arg in UnhandledExceptionHandler. The DiscordAlerts__WebhookUrl env var on Render is now unused (safe to delete). ⚠️ After this deploys, the API reaches Discord only via Sentry — so orbit-api must be in the Sentry→Discord rule for API errors to land in #alerts.

Tests

3 new tests in ReminderSchedulerServiceTests reproduce the cross-timezone scenario deterministically (timezone chosen from the current UTC hour so userToday ≠ utcToday every run; a guard assertion confirms it). They fail on the old code, pass on the new. Full solution: Domain 386 / Application 1995 / Infrastructure 1114 — all passing.

🤖 Generated with Claude Code

…ound jobs

The reminder scheduler's relative-reminder path deduped against `Date == utcToday`,
but HabitLog.Date and SentReminder.Date store the user's LOCAL date. For users whose
local date differs from UTC (e.g. Brazil after UTC midnight), the bulk pre-check missed
already-sent reminders and already-logged habits, so every tick re-attempted the insert
and hit the SentReminders unique index — a recurring production 500 surfaced via Sentry.

- ReminderSchedulerService.ProcessRelativeReminders: query the +/-1-day window and key the
  dedup and logged sets on the user's local date (mirrors ProcessScheduledReminders).
- GoalDeadlineNotificationService / StreakGoalSyncService: widen the streak log lookback
  by one day so a far-west user's oldest in-window logs aren't clipped by the UTC bound.
- HabitDueDateAdvancementService inspected and intentionally left: its UTC cutoff is a
  deliberate conservative buffer; the per-user check (DueDate < userToday) is authoritative.

Adds regression tests that reproduce the cross-timezone scenario deterministically.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tight, well-reasoned fix. The relative-reminder path now mirrors the already-correct scheduled-reminder path: ±1-day window on the bulk query, dedup keyed on (HabitId, userToday) and (HabitId, userToday, MinutesBefore), and the SentReminder row written with the user-local date. The streak service lookback widening in GoalDeadlineNotificationService and StreakGoalSyncService is a pure safe broadening — the authoritative streak walk still uses userToday so it can't overcount. The three regression tests are deterministic: selecting UTC±12 based on current UTC hour guarantees userToday ≠ UtcToday regardless of when CI runs, and the guard assertion will catch any broken assumption loudly. Inspected-and-intentionally-not-changed rationale for HabitDueDateAdvancementService is solid.

…s the single alert source

UnhandledExceptionHandler already calls SentrySdk.CaptureException for every HTTP 500, so with
orbit-api added to the Sentry->Discord alert rule the bespoke DiscordAlertNotifier is redundant —
and it double-pinged Discord on 500s. Removing it unifies alerting across web, mobile, and API on
one mechanism: consistent formatting, one place to tune rules, and Sentry messages link to full
issue context.

Removes DiscordAlertNotifier + IAlertNotifier + DiscordAlertSettings + the "Discord" HttpClient
registration + the DiscordAlerts config section, and the now-unused IAlertNotifier arg in
UnhandledExceptionHandler. The DiscordAlerts__WebhookUrl env var on Render is now unused.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thomasluizon thomasluizon changed the title fix(reminders,streaks): correct UTC-vs-user-local date skew in background jobs fix(backend): background-job UTC date skew + retire redundant Discord notifier Jun 20, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second commit (Discord notifier retirement) is a clean, complete removal: IAlertNotifier + DiscordAlertNotifier + DiscordAlertSettings are deleted together with their DI registration, appsettings.json stanza, and tests, and UnhandledExceptionHandler's constructor is simplified. SentrySdk.CaptureException is still in place so exception capture is unaffected — the old fire-and-forget Discord call was actually an unobserved task anyway, so this is a net improvement in reliability. Combined with the already-approved reminder/streak timezone fix, both halves of this PR are solid.

@thomasluizon
thomasluizon merged commit cdf6c3e into main Jun 20, 2026
5 checks passed
@thomasluizon
thomasluizon deleted the fix/background-job-utc-date-skew branch June 20, 2026 01:58
@sonarqubecloud

Copy link
Copy Markdown

thomasluizon added a commit that referenced this pull request Jul 4, 2026
- .claude/agents/security-reviewer.md: StripeConfiguration.ApiKey is set at
  startup in src/Orbit.Api/Extensions/ServiceCollectionExtensions.Infrastructure.cs,
  not Program.cs — fix the path.
- .claude/agents/contract-aligner.md: there is no Common/DTOs/ (nor any DTOs/
  folder); DTOs are feature-local records/classes under Orbit.Application/
  <Feature>/ and <Feature>/Models/, plus request/response records alongside
  commands/queries — reword the surface-area description and the read step.
- load-tests/README.md + tests/CLAUDE.md: drop "staging" / "QA runs" mentions —
  there is no QA/staging env, targets are prod + local only (#211).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 4, 2026
…paths (#284)

* chore: stale docs cleanup — README reality, .planning removal, agent paths

- README.md: rewrite to current reality — AI is OpenAI (gpt-4.1-mini primary,
  gpt-5.4-nano sub-tasks via AI:{ApiKey,Model,BaseUrl}, api.openai.com/v1), not
  Gemini/Ollama; config section reflects actual appsettings keys; features +
  layer breakdown cover shipped domains (Goals, Gamification, Calendar,
  Accountability, Challenges, Referrals, Agent/MCP, Sync, Waitlist, ...);
  controller list = actual Controllers/ contents (26); payments = Stripe +
  Google Play Billing; related repo = orbit-ui-mobile (Turborepo: Next.js 16
  web + Expo Android), not "orbit-ui Nuxt 4".
- src/Orbit.Infrastructure/CLAUDE.md: OpenAI .NET SDK 2.8.0 -> 2.12.0 (per .csproj).
- .claude/agents/security-reviewer.md: fix JWT service path
  Services/TokenService.cs -> Services/JwtTokenService.cs (both references).
- Delete the frozen .planning/ snapshot (contradicts current architecture) and
  its now-dead **/.planning entry in .dockerignore.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: stale docs cleanup — agent paths, DTO layout, no-QA-env wording

- .claude/agents/security-reviewer.md: StripeConfiguration.ApiKey is set at
  startup in src/Orbit.Api/Extensions/ServiceCollectionExtensions.Infrastructure.cs,
  not Program.cs — fix the path.
- .claude/agents/contract-aligner.md: there is no Common/DTOs/ (nor any DTOs/
  folder); DTOs are feature-local records/classes under Orbit.Application/
  <Feature>/ and <Feature>/Models/, plus request/response records alongside
  commands/queries — reword the surface-area description and the read step.
- load-tests/README.md + tests/CLAUDE.md: drop "staging" / "QA runs" mentions —
  there is no QA/staging env, targets are prod + local only (#211).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: drop dangling add-api-endpoint skill reference in contract-aligner

The add-api-endpoint scaffolding skill was deleted in orbit-ui-mobile
(thomasluizon/orbit-ui-mobile#391), so remove its stale "when the skill
completes" invocation trigger from the contract-aligner agent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant