feat(spec): Add Personal Rate Notifications#2271
Open
niemyjski wants to merge 14 commits into
Open
Conversation
Create cut-down MVP spec for personal rate-based notifications informed by issue #177. Includes proposal, design, tasks, and spec with Given/When/Then scenarios covering: - Personal per-user rules (project and stack scoped) - Cache-backed 1-minute UTC bucket counters - Async evaluator job with distributed lock - Cooldown and snooze noise controls - Email delivery with full validation - Svelte UI for rule management Intentionally excludes digests, webhooks, Slack, anomaly detection, quiet hours, and other advanced alerting features. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bugs identified via staff-engineer review comparing against existing notification pipeline (070_QueueNotificationAction, EventNotificationsJob, Stack.AllowNotifications): 1. Missing premium feature gate: The spec introduced a new free notification channel but the existing system requires HasPremiumFeatures. Added to proposal risks table, design gating section, evaluator, and Svelte UI. 2. Muted-stack / discarded-event suppression: Counters would have counted events on Ignored/Snoozed/Discarded stacks and known-bot requests. Added AllowNotifications and IsBot checks to UpdateRateCountersAction, mirroring existing 070_QueueNotificationAction logic. 3. Snooze back-alert bug (shared-counter race): Rate counters are keyed by project/stack + signal, not per-rule. After snooze expires, activity gathered during the muted window would immediately fire because the counter reflects global traffic. Fix: evaluator uses max(windowStartUtc, rule.SnoozedUntilUtc) as the lower bucket boundary so only post-snooze traffic counts toward the threshold. 4. Orphaned rule lifecycle: No cleanup on membership removal or project/org deletion. Added CleanupRateNotificationRulesAsync pattern mirroring OrganizationService.CleanupProjectNotificationSettingsAsync. 5. Stack-scoped email missing context: Delivery job never loaded the stack, so stack-scoped email copy had no title/link. Added stack lookup requirement to delivery design and spec. Also expanded test matrix to cover all five findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@github-copilot review |
|
Introduces the core domain model for personal rate notification rules. Each rule belongs to an org/project/user and configures a threshold- based alert for a specific signal (errors, critical errors, etc.) with snooze/cooldown semantics to prevent alert fatigue.
RateCounterService uses ICacheClient 1-minute bucket counters to track event rates per signal/project/stack. 075_UpdateRateCountersAction [Priority(75)] increments counters during event ingestion for all matching signals.
EvaluatorJob runs minutely (cron) to check rules against counters and enqueue notifications. Implements the snooze back-alert fix: uses max(windowStart, snoozedUntilUtc) as the effective window start to prevent false alerts from traffic counted during a snooze period. RateNotificationsJob dequeues and sends notifications via IMailer.
…email template Adds Handlebars HTML email template and mailer implementation. Template includes rule name, signal, threshold, observed count, project info, and snooze/manage links.
REST CRUD + snooze/unsnooze endpoints under:
/api/v2/users/{userId}/projects/{projectId}/rate-notifications
Unsnooze sets SnoozedUntilUtc = now (not null) to prevent back-alerts
after snooze expiry. Premium gate: non-premium forces IsEnabled=false.
- IRateNotificationRuleRepository, RateCounterService, RateNotificationRuleCache - RateNotificationsJob (queue delivery job) - RateNotificationEvaluatorJob (minutely distributed cron job) - RateNotification queue for in-process, Azure, Redis, SQS providers
RateCounterServiceTests: 12 unit tests (no ES), including the critical regression: SumBucketsAsync_WithSnoozeFix_IgnoresTrafficDuringSnooze proves that events counted during a snooze period are excluded. RateNotificationEvaluatorJobTests: integration-level job tests including the back-alert scenario at job level (1 notification for Rule B only). RateNotificationRuleControllerTests: CRUD + snooze endpoint tests.
- types.ts: ViewRateNotificationRule, NewRateNotificationRule, UpdateRateNotificationRule, SnoozeRateNotificationRuleRequest - api.svelte.ts: TanStack Query wrappers for list, create, update, delete, snooze, unsnooze - components/rate-notification-rule-list.svelte: list with enable toggle, snooze badge, delete confirm, premium gate - components/rate-notification-rule-form.svelte: create/edit form with validation, premium gate, noise warning - index.ts: barrel export Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s and test false positives - Controller: AddAsync(rule, o => o.Cache().ImmediateConsistency()) ensures the 20-rule-per-project limit is enforced even under rapid concurrent creation. Without this, ES doesn't refresh before CountByProjectIdAndUserIdAsync, returning stale count=0 and allowing unlimited rules. - Tests: add Foundatio.Repositories using + ImmediateConsistency() on all _ruleRepository.AddAsync calls in RateNotificationEvaluatorJobTests so that GetEnabledByProjectIdAsync sees freshly indexed rules. Previously the 3 passing tests were false positives (rules invisible → no enqueue = expected) and the 2 failing tests were false negatives (rules invisible → no enqueue ≠ expected). All 29 rate-notification tests now pass: 12/12 RateCounterServiceTests 12/12 RateNotificationRuleControllerTests 5/5 RateNotificationEvaluatorJobTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ate notifications
Two bugs fixed:
1. Enum serialization mismatch: RateNotificationSignal/Subject were bare int enums;
API rejected frontend's string values ('Errors', 'Project') with 400.
Fix: add JsonStringEnumConverter (STJ) + StringEnumConverter (Newtonsoft) to both
enums, matching the StackStatus pattern used elsewhere in the codebase.
API now returns and accepts string enum names.
2. Mutation created inside event handler: createRateNotificationRule/
updateRateNotificationRule were called inside handleSubmit(), which runs
outside Svelte's synchronous component init context — TanStack Query
context not available there. Fix: create both mutations at init with
reactive getter props, call mutateAsync only inside the handler.
Also: integrated RateNotificationRuleList + RateNotificationRuleForm into
account/notifications page, fixed type errors in both components
(Select.Root type='single', Alert variant, parseSeconds array safety).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ipeline action Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+84
to
+93
| foreach (var rule in rules.Where(r => matchedSignals.Contains(r.Signal))) | ||
| { | ||
| // For Stack subject, only match if this event belongs to the rule's stack | ||
| if (rule.Subject == RateNotificationSubject.Stack && | ||
| (String.IsNullOrEmpty(rule.StackId) || !String.Equals(ctx.Event.StackId, rule.StackId, StringComparison.Ordinal))) | ||
| continue; | ||
|
|
||
| string counterKey = BuildCounterKey(rule); | ||
| await _counterService.IncrementAsync(counterKey); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
OpenSpec proposal and design for personal rate-based notifications. Users configure rules like:
This closes issue #177 at the design level. The implementation tasks live in
tasks.md.Why
Current per-event occurrence notifications miss sustained high-volume error bursts. Rate notifications fill this gap without overwhelming users—mandatory cooldowns, snooze support, and fresh-baseline semantics keep alert fatigue low.
New APIs / Behaviors
GET/POST/PUT/DELETE /api/v2/projects/{projectId}/notification-rules— CRUD forRateNotificationRulePOST /api/v2/projects/{projectId}/notification-rules/{ruleId}/snooze— snooze a ruleUpdateRateCountersActionat priority 71 (after existing notification gate at 70)RateNotificationEvaluatorJoband delivery jobBugs Found and Fixed in This PR
During staff-engineer review five spec-level regressions were identified and corrected:
HasPremiumFeaturesmax(windowStartUtc, rule.SnoozedUntilUtc)as bucket lower boundBreaking Changes
None — this is an additive spec-only PR.
Implementation Status