Skip to content

feat: Add personal rate notifications - #2271

Open
niemyjski wants to merge 2 commits into
mainfrom
niemyjski/add-personal-rate-notifications-spec
Open

niemyjski wants to merge 2 commits into
mainfrom
niemyjski/add-personal-rate-notifications-spec

Conversation

@niemyjski

@niemyjski niemyjski commented May 31, 2026

Copy link
Copy Markdown
Member

What

Add personal email alerts when a project or stack reaches a configured event count within a time window. Users can create, edit, disable, snooze, and delete rules from account notification settings.

The ingestion pipeline shares minute counters across matching rules. Scheduled evaluation applies thresholds and cooldowns; delivery rechecks the rule, membership, email preferences, and project notification budget. Rule changes invalidate counter plans across replicas. Evaluation allows one minute for in-flight writes to settle, adding one minute of alert latency. Emails use the current typed Razor renderer.

Why

Occurrence notifications can miss sustained bursts. Rate rules let users choose when a burst warrants an email and control repeat alerts.

APIs and behavior

  • Add CRUD and snooze/resume endpoints under /api/v2/users/{userId}/projects/{projectId}/rate-notifications.
  • Runtime processing and the Svelte UI require premium access and the rate-notifications organization feature. Rules remain stored after a downgrade or feature removal.
  • Remove rules when their user leaves the organization or their project/organization is deleted.
  • Preserve existing routes and payloads; add has_rate_notifications to project responses.

Verification

  • Rebased onto main at bb0315556.
  • Release build: zero warnings or errors.
  • 71 focused backend unit tests passed, including reproduced cache-invalidation and minute-boundary races, counter behavior, serialization, and typed email rendering.
  • Frontend validation, 780 unit tests, and production build passed.
  • Generated API models from the OpenAPI snapshot; added a regression for nullable update enums.
  • C# whitespace verification and strict OpenSpec validation passed. Helm lint/render passed. Project and stack email previews checked at desktop/mobile widths.
  • Hosted checks passed on ae336a09d: 3,053 backend tests passed (three intentional skips), all 64 browser E2E tests passed including rule management, frontend checks passed, and Docker builds passed. CI run.
  • Local service-backed runs were blocked by the shared Elasticsearch cluster's inactive primary shards; hosted CI completed API, OpenAPI snapshot, and browser verification in isolated infrastructure.

Breaking changes

None. The feature is additive and gated.

@niemyjski

Copy link
Copy Markdown
Member Author

@github-copilot review

Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Fixed
Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Fixed
Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@niemyjski niemyjski changed the title feat(spec): Add Personal Rate Notifications feat: Add personal rate notifications Jul 10, 2026
@niemyjski

Copy link
Copy Markdown
Member Author

Follow-up audit complete

Audited live head 862ad4ffe0f7406e541c36d944d08d39a80c5d13 against origin/main dc940dd15c8d222d9764080622fdf4583d4546b8, every GitHub feedback surface, and the complete effective diff using the thermo-nuclear code-quality review.

Feedback classification

  • Three github-code-quality inline findings are resolved, outdated, already fixed, and superseded by the architecture pivot. The criticized rule-filtering loop no longer exists; RateNotificationCounterPlan.GetCounterKeys(...) now compiles signal/stack eligibility centrally and ingestion iterates only matching counter keys.
  • Copilot's submitted review contains no finding; the reviewer reported an internal error. It is non-actionable.
  • No human reviewer submitted a finding. The top-level Copilot review request is administrative, not feedback.
  • The current coverage comment is informational and green: 77% line / 66% branch.
  • Live result: 0 unresolved review threads and 0 unresolved actionable findings.

Thermo-nuclear finding and RCA

The evaluator producer and delivery consumer independently formatted the queued SubjectKey. That duplicated a protocol invariant across process boundaries; a future one-sided edit could enqueue valid notifications that delivery silently rejected.

Commit 862ad4ffe centralizes project/stack subject-key construction in the existing rate-notification counter plan and uses it from both evaluator and delivery. Focused regression coverage proves both subject scopes. No other actionable correctness, authorization, serialization, concurrency, non-atomic update, file-size, or unnecessary orchestration issue remained after the complete diff audit.

Product/UI dogfood

Dogfooded the exact PR runtime on desktop (1280x720) and mobile (390x844): empty state, create, edit threshold, snooze, resume, disable, and delete all succeeded. The UI states the cost-saving purpose, provides a useful 10 errors / 5 minutes / 30-minute cooldown starting point, and keeps actions usable at both viewports. The committed Playwright scenario independently passed.

Verification

  • dotnet test ... --filter-class Exceptionless.Tests.Pipeline.UpdateRateCountersActionTests — 11 passed
  • dotnet test ... --filter-class Exceptionless.Tests.Jobs.RateNotificationEvaluatorJobTests — 10 passed
  • dotnet test ... --filter-class Exceptionless.Tests.Jobs.RateNotificationsJobTests — 19 passed
  • dotnet test Exceptionless.slnx --no-restore --no-build — 2,608 total; 2,606 passed; 2 intentional skips; 0 failed
  • dotnet build Exceptionless.slnx --no-restore — succeeded; 0 warnings; 0 errors
  • focused dotnet format --verify-no-changes — passed
  • rate-notification Vitest files — 2 files / 12 tests passed
  • local Playwright rate-notification E2E — 1 passed
  • git diff --check — passed
  • GitHub required checks on 862ad4ffe — version, test-client, test-e2e, test-api/coverage, docker-build, and CLA all passed

No external blocker remains.

@niemyjski

Copy link
Copy Markdown
Member Author

Final follow-up on head 1a9cbe377e3bb7ff70f20eb7b12578f545e13905:

  • Re-audited all review threads, reviews, and conversation: the three inline findings remain resolved/outdated and superseded by RateNotificationCounterPlan; no new actionable feedback appeared.
  • Commit 50d919437 fixes the failing test-client check by restoring deterministic class/module sort order in e2e/fixtures/api-client.ts without behavior changes.
  • Merged current main normally in 1a9cbe377 to satisfy strict up-to-date branch protection; no rebase or force-push.
  • Post-push local validation: npm run lint, npm run check (0 errors/warnings), npm run build, and git diff --check all pass.
  • Final GitHub run 31624111842: version, test-client, test-api/coverage, test-e2e (Aspire + Playwright), docker-build, and CLA all passed; publish/deploy jobs skipped as expected for a PR.

Final state: open, current with main, mergeable, CLEAN, zero unresolved review threads.

} while (await results.NextPageAsync());

var plan = RateNotificationCounterPlan.Compile(projectId, rules);
await _cache.SetAsync(cacheKey, plan, CacheTtl);

@niemyjski niemyjski Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[P2] Prevent invalidation from losing to an in-flight cache fill

The striped gates only coordinate this process. In a multi-replica deployment, worker A can load the pre-mutation rules, API B can save and invalidate the shared key, and then A can publish its stale plan after that removal. The old plan remains usable for the five-minute TTL, so new rules can miss countering and disabled/deleted rules can remain active. Please coordinate load/publish and invalidation with a distributed per-project generation/CAS or lock so an invalidation cannot be overwritten by an older fill; add a regression that pauses the fill across a mutation.

</div>

<RateNotificationRuleList
hasPremiumFeatures={selectedProject.has_premium_features}

@niemyjski niemyjski Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[P2] Remove the unreachable premium branch tree

has_rate_notifications is already the canonical premium-plus-rollout capability, and both rate-rule components are mounted only inside that guard. Therefore hasPremiumFeatures is always true here: the upgrade callbacks, alerts, disabled branches, and the enabled argument used to build requests can never execute in production. Please remove that redundant plumbing from the page, list, form, schema helper, and its dead test so the combined capability has one owner and the two entitlement models cannot drift.

@niemyjski

niemyjski commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Thermo-nuclear follow-up on head 002497e:

Commit 002497e applies the surgical review fixes: removes the resurrected migration cron resume, aligns the canonical email source with singular/plural rendering, reuses the E2E polling helper, restores PUT stack-scope validation parity, centralizes supported-window policy across API/runtime, makes active-key discovery precede increments, memoizes duplicate bucket reads, and aligns generated nullable-enum schemas with the API contract.

Static evidence: git diff --check passed; OpenAPI and endpoint-manifest JSON parse; source/compiled template placeholders and generated TypeScript/Zod nullability agree. The contract generator could not run because this isolated worktree has no installed swagger-typescript-api. Per the explicit review constraint, no local tests, builds, Aspire, E2E, browser, runtime services, or dogfood were run, so these fixes are not locally test-verified. Two deeper non-surgical findings remain as inline review threads.

@niemyjski
niemyjski force-pushed the niemyjski/add-personal-rate-notifications-spec branch from 58e3c30 to ae336a0 Compare September 16, 2026 00:40
@github-actions

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.Insulation 37% 35% 286
Exceptionless.Web 85% 70% 8281
Exceptionless.Core 77% 69% 10900
Exceptionless.AppHost 38% 41% 147
Summary 79% (27240 / 34301) 68% (12688 / 18543) 19614

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.

2 participants