feat: major headlines push notifications by nimrodkra · Pull Request #3818 · dailydotdev/daily-api

nimrodkra · 2026-04-26T14:06:57Z

Summary

New MajorHeadlineAdded notification type with default settings + Zod schema entry.
New majorHeadlineAdded TypedNotificationWorker subscribing to api.v1.post-highlighted. Filters significance to Breaking/Major, skips non-public/deleted/vordr posts, fans out only to opted-in users.
Push gating: only Breaking triggers OneSignal push (Major stays in-app). Per-user 2h Redis dedup for Breaking pushes via mh:push:<userId>.
Adds UserActionType.DismissedMajorHeadlinesAlertsBanner for the frontend banner persistence.
Worker tests cover significance filtering, post visibility, vordr, mute state, and empty audience.

Test plan

Insert a Breaking highlight: subscribed user gets in-app + push; second highlight within 2h gets only in-app (deduped)
Insert a Major highlight: subscribed user gets in-app only
Insert a Notable/Routine/Unspecified highlight: no notification
Insert highlight referencing private/deleted/vordr post: skipped
User with MajorHeadlineAdded.inApp = muted: excluded from notification audience
pnpm run lint and worker spec pass

Subscribe to api.v1.post-highlighted and dispatch in-app notifications for Breaking/Major significance highlights, with push limited to Breaking and per-user 2h Redis dedup. Adds MajorHeadlineAdded notification type, default settings, schema entry, worker, and infra subscription. Made-with: Cursor

pulumi · 2026-04-26T14:07:54Z

🍹 The Update (preview) for dailydotdev/api/prod (at 4aca7b5) was successful.

✨ Neo Explanation

This is a routine deployment adding a new "major headline" breaking-news notification feature, including a new GCP Pub/Sub subscription and updated container images across the cluster. Risk is low, with the only thing worth validating being the scalability of the full-user-table fan-out query when the first Breaking post is published. ✅ Low Risk

This PR introduces a new MajorHeadlineAdded notification type that fires when a post is highlighted with Breaking significance. The new worker (majorHeadlineAdded) subscribes to the existing api.v1.post-highlighted Pub/Sub topic via a new subscription, processes the event, and fans out in-app notifications to all users who haven't muted the notification type.

The broad deployment updates (all Deployments and CronJobs) are purely a routine image tag bump to the new commit SHA. The migration Jobs are replaced as expected for this deployment pattern (name includes the commit SHA).

🔵 Info — The MajorHeadlineAdded notification defaults to inApp: Subscribed / email: Muted for all users. The worker streams the entire user table to build the recipient list, which could be a large fan-out on first Breaking highlight — worth confirming the streaming/concurrency approach (streamConcurrency = 10) is adequate for the user base size and that the Pub/Sub subscription acknowledgement deadline is long enough for the query to complete.

🔵 Info — There is no email template wired up (major_headline_added: ''), which is intentional per the code, but confirm this is expected at launch rather than an oversight.

Resource Changes

    Name                                                       Type                                  Operation
~   vpc-native-update-views-cron                               kubernetes:batch/v1:CronJob           update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob           update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob           update
~   vpc-native-update-tags-str-cron                            kubernetes:batch/v1:CronJob           update
-   vpc-native-api-db-migration-a018e700                       kubernetes:batch/v1:Job               delete
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob           update
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob           update
~   vpc-native-deployment                                      kubernetes:apps/v1:Deployment         update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob           update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment         update
~   vpc-native-materialize-yearly-best-post-archives-cron      kubernetes:batch/v1:CronJob           update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob           update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob           update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob           update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob           update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment         update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-channel-highlights-cron                   kubernetes:batch/v1:CronJob           update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob           update
+   vpc-native-api-clickhouse-migration-d60b2736               kubernetes:batch/v1:Job               create
+   vpc-native-api-db-migration-d60b2736                       kubernetes:batch/v1:Job               create
~   vpc-native-channel-digests-cron                            kubernetes:batch/v1:CronJob           update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment         update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment         update
-   vpc-native-api-clickhouse-migration-a018e700               kubernetes:batch/v1:Job               delete
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob           update
~   vpc-native-materialize-monthly-best-post-archives-cron     kubernetes:batch/v1:CronJob           update
+   api-sub-api.major-headline-added-notification              gcp:pubsub/subscription:Subscription  create
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment         update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob           update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob           update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob           update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob           update
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob           update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob           update
~   vpc-native-rotate-daily-quests-cron                        kubernetes:batch/v1:CronJob           update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob           update
... and 12 other changes

Made-with: Cursor

nimrodkra · 2026-04-26T14:31:36Z

@claude please review this pr

claude · 2026-04-26T14:31:52Z

Claude finished @nimrodkra's task in 1m 50s —— View job

PR Review: feat/major-headlines-push-notifications

Read all changed files
Review worker logic and filtering
Review push notification gating and Redis dedup
Review tests
Review infrastructure changes
Post final review feedback

Overall this is a well-structured feature addition. The code follows existing patterns, is concise, and the test coverage is solid. A few issues worth addressing before merge:

Issues

1. Scalability problem: full-table user query on every highlight event (`src/workers/notifications/majorHeadlineAdded.ts:51-58`)

The worker queries all subscribed users in a single unbounded query:

const users = await con
  .createQueryBuilder()
  .select('u.id', 'userId')
  .from(User, 'u')
  .where(
    `COALESCE(u."notificationFlags"->'major_headline_added'->>'inApp', 'subscribed') = 'subscribed'`,
  )
  .getRawMany<{ userId: string }>();

With a large user base (millions of users), this returns an enormous result set in memory before returning it to the notification generator. Other global-audience workers (e.g. generateAndStoreNotificationsV2) stream or paginate users. Suggest using a stream-based approach (stream().getMany() / cursor-based pagination) similar to how streamNotificationUsers works in the push worker, or deferring fan-out to the notification storage layer rather than pre-fetching all IDs upfront. Fix this →

2. Significance parsing is fragile in the push worker (`src/workers/newNotificationV2Push.ts:38-44`)

const [significanceStr] = (data.notification.uniqueKey || '').split(':');
const significance = parseInt(significanceStr, 10);
if (significance !== PostHighlightSignificance.Breaking) {
  return;
}

The uniqueKey for major_headline_added is set to ${ctx.significance}:${ctx.channel} in generate.ts:771. This is an implicit coupling — the push worker depends on a specific string format of uniqueKey that is defined elsewhere. If the key format ever changes (e.g. field ordering swapped), this silently breaks — all major headlines start getting push notifications. Consider encoding significance more explicitly, e.g. as a separate field on the notification, or at minimum add a defensive check (isNaN(significance)). Fix this →

3. `uniqueKey` does not prevent duplicate in-app notifications for the same post+channel (`src/notifications/generate.ts:771`)

.uniqueKey(`${ctx.significance}:${ctx.channel}`)

This key does not include postId. If the same post is highlighted twice on the same channel with the same significance level, the second highlight will be treated as a duplicate and deduplicated at the notification level — but this may also mean a new highlight on a different post could collide. Conversely, a post highlighted twice (corrected) would only show one in-app notification. It's worth confirming this is the intended behavior; including postId in the uniqueKey would be safer: `${ctx.post.id}:${ctx.significance}:${ctx.channel}`. Fix this →

4. `MajorHeadlineAdded` email is opted-in by default but has no email template

In DEFAULT_NOTIFICATION_SETTINGS (line 319-322 of notifications/common.ts), email is set to Subscribed. But notificationToTemplateId['major_headline_added'] is '' and notificationToTemplateData['major_headline_added'] returns null. This means the email channel is shown as subscribed in user preferences, but no email is ever sent — potentially confusing users who see they're "subscribed" to email for this type. Either set email: NotificationPreferenceStatus.Muted in the default, or document this is intentional (email template coming later). Fix this →

Minor observations

blockedPostTypes (majorHeadlineAdded.ts:15-19) blocks Welcome, Brief, Digest but not Collection. This seems intentional (Breaking news could be a collection) but worth confirming.
Redis dedup is per-user, not per-post (newNotificationV2Push.ts:80-95). The key is notification:major_headline_dedup:<userId> with a 2h TTL. This means a user who receives a Breaking push for post A won't receive one for post B within 2 hours — even if they're entirely different stories. This is likely intentional rate-limiting behavior, but should be documented or considered.
No test for blockedPostTypes — there's no test case for PostType.Welcome/Brief/Digest being skipped. Given these are explicitly listed in the worker, a test would be valuable. Fix this →
No test for the push dedup logic (newNotificationV2Push.ts:78-101) — the Redis-based Breaking push dedup is the most novel mechanism here and has no test coverage.
type vs interface — NotificationMajorHeadlineContext in types.ts:226 uses type (correct per CLAUDE.md), but NotificationStreakRestoreContext at line 83 uses interface. Not introduced by this PR, but for consistency the new type follows the right convention.

Looks good

Significance filtering using a Set is clean and readable.
Post visibility/vordr/deletion guards are correctly applied before the expensive user query.
majorHeadlineAdded is correctly registered in index.ts and workers.
generateStorageKey + StorageKey.MajorHeadlineDedup follows existing config conventions.
DismissedMajorHeadlinesAlertsBanner action type is a straightforward, appropriate addition.
Test coverage hits the main significance/visibility/mute paths well.

- Default email channel to Muted for major_headline_added since no email template is wired up. - Include postId in major_headline_added uniqueKey to prevent cross-post collisions. - Add NaN guard when parsing significance from uniqueKey in the push worker. - Add tests for blockedPostTypes in the worker and dedup/significance paths in the push worker. Made-with: Cursor

nimrodkra · 2026-04-26T14:53:11Z

Addressed @claude review feedback:

uniqueKey collisions: now includes postId ({postId}:{significance}:{channel}) so different posts can never collide.
Fragile significance parsing: added Number.isNaN guard in the push worker; updated the parser to read parts[1] to match the new uniqueKey layout.
Email default: switched MajorHeadlineAdded email default from Subscribed to Muted since no email template exists.
Tests: added coverage for blockedPostTypes (Welcome/Brief/Digest) in the worker, plus three new push-worker tests covering non-Breaking skip, malformed uniqueKey skip, and per-user dedup within the TTL window.

Skipped (worth a follow-up if scale becomes a concern):

Streaming subscribed users: kept the single getRawMany query for v1. The downstream notification storage layer already streams to push delivery, so memory pressure is mainly the userIds list. Happy to swap to streamNotificationUsers-style cursor in a follow-up if needed.

claude · 2026-04-26T14:53:24Z

Claude finished @nimrodkra's task in 1m 3s —— View job

Follow-up review: addressed feedback verified ✓

Read updated files to verify fixes
Verify uniqueKey now includes postId
Verify isNaN guard in push worker + parts[1] index update
Verify email default changed to Muted
Verify new tests for blockedPostTypes and push dedup
Post final review

All four issues from the initial review are correctly resolved:

uniqueKey collisions fixed (generate.ts:771) — key is now ${ctx.post.id}:${ctx.significance}:${ctx.channel}, eliminating cross-post collisions. ✓

Significance parsing hardened (newNotificationV2Push.ts:38-46) — parser now reads parts[1] (matching the new postId:significance:channel layout) and returns early on Number.isNaN(significance). The enum confirms Breaking = 1, Major = 2, so the test comments // Breaking significance (1) / // Major significance (2) are accurate. ✓

Email default corrected (notifications/common.ts:319-322) — MajorHeadlineAdded email is now NotificationPreferenceStatus.Muted, consistent with no template existing yet. ✓

New tests — the worker test file adds it.each([PostType.Welcome, PostType.Brief, PostType.Digest]) covering blockedPostTypes. The push worker now has three additional tests: non-Breaking skip (Major), malformed uniqueKey skip, and per-user dedup across two distinct Breaking notifications. All test logic aligns correctly with the implementation. ✓

Streaming follow-up — the getRawMany approach is a reasonable v1 choice and the trade-off is well understood. A follow-up ticket to switch to cursor-based streaming when scale demands it would be the right call.

LGTM — the PR is in good shape. Ready to merge.

…push-notifications

Made-with: Cursor

capJavert · 2026-04-27T14:12:28Z

+              if (isMajorHeadline) {
+                const dedupResults = await Promise.all(
+                  users.map(async (user) => {
+                    const acquired = await setRedisObjectIfNotExistsWithExpiry(


why are we dedupping here?

capJavert · 2026-04-27T14:13:18Z

+      if (isMajorHeadline) {
+        const parts = (data.notification.uniqueKey || '').split(':');
+        const significanceStr = parts[1];
+        const significance = parseInt(significanceStr, 10);
+        if (
+          Number.isNaN(significance) ||
+          significance !== PostHighlightSignificance.Breaking
+        ) {
+          return;
+        }
+      }


this seems fragile, we ideally skip this already on worker side

capJavert · 2026-04-27T14:15:48Z

+  {
+    topic: 'api.v1.post-highlighted',
+    subscription: 'api.major-headline-added-notification',
+    args: { ackDeadlineSeconds: 60 },


why are you setting ackDeadlineSeconds?

It means subscription will error if query takes longer, better to leave it without and use defaults if not specific reason

capJavert · 2026-04-27T14:18:32Z

+      .description(ctx.headline)
+      .avatarSource(ctx.source)
+      .objectPost(ctx.post, ctx.source, ctx.sharedPost)
+      .uniqueKey(`${ctx.post.id}:${ctx.significance}:${ctx.channel}`),


do we expect there will be same post.id and different significance, feels like just post id would do the trick eg. one headline per post per channel.

Not sure what channels are, maybe also just per post.

capJavert · 2026-04-27T14:20:42Z

+        .where(
+          `COALESCE(u."notificationFlags"->'major_headline_added'->>'inApp', 'subscribed') = 'subscribed'`,
+        )
+        .getRawMany<{ userId: string }>();


ideally we stream like in postAddedUserNotification worker

- Drop Major significance from notification generation; only Breaking generates notifications. Eliminates fragile uniqueKey parsing in the push worker. - Stream subscribed users via processStream instead of getRawMany. - Remove the per-user 2h Redis push dedup; Breaking is rare and uniqueKey already prevents duplicates per post. - Simplify uniqueKey to ${ctx.post.id} (one headline per post). - Drop ackDeadlineSeconds: 60 from the subscription, use defaults. - Remove unused MajorHeadlineDedup StorageKey and related dedup logic. Made-with: Cursor

nimrodkra · 2026-04-28T13:57:47Z

Thanks @capJavert! Addressed all five comments in 234787e by simplifying the architecture:

Push worker dedup + fragile uniqueKey parsing: Removed entirely. Instead of generating notifications for both Breaking + Major and gating push at the consumer, the worker now only generates notifications for Breaking significance. Major is dropped at the source. The push worker is back to its original state with no major-headline-specific code. We can re-introduce Major later as a separate notification type if there's demand.
uniqueKey: Simplified to just ctx.post.id — one headline per post.
ackDeadlineSeconds: Removed, using defaults now.
User streaming: Switched to con.createQueryBuilder().stream() + processStream, matching the postAddedUserNotification pattern.

Net diff: +20 / -168.

…push-notifications

…hub.com/dailydotdev/daily-api into feat/major-headlines-push-notifications

nimrodkra mentioned this pull request Apr 26, 2026

feat: major headlines push notifications dailydotdev/apps#5922

Open

5 tasks

fix: add MajorHeadlineAdded email mappings and lint

1ce7a61

Made-with: Cursor

nimrodkra force-pushed the feat/major-headlines-push-notifications branch from ed28c97 to 354fc04 Compare April 27, 2026 06:50

nimrodkra added 2 commits April 27, 2026 09:50

Merge remote-tracking branch 'origin/main' into feat/major-headlines-…

9850fb2

…push-notifications

chore: retrigger CI

3c91c25

Made-with: Cursor

nimrodkra requested a review from capJavert April 27, 2026 13:01

capJavert reviewed Apr 27, 2026

View reviewed changes

Merge branch 'main' into feat/major-headlines-push-notifications

a3e1744

nimrodkra requested a review from capJavert April 28, 2026 14:21

nimrodkra added 2 commits April 28, 2026 22:56

Merge remote-tracking branch 'origin/main' into feat/major-headlines-…

50ad852

…push-notifications

Merge branch 'feat/major-headlines-push-notifications' of https://git…

4aca7b5

…hub.com/dailydotdev/daily-api into feat/major-headlines-push-notifications

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: major headlines push notifications#3818

feat: major headlines push notifications#3818
nimrodkra wants to merge 9 commits intomainfrom
feat/major-headlines-push-notifications

nimrodkra commented Apr 26, 2026 •

edited

Loading

Uh oh!

pulumi Bot commented Apr 26, 2026 •

edited

Loading

Uh oh!

nimrodkra commented Apr 26, 2026

Uh oh!

claude Bot commented Apr 26, 2026 •

edited

Loading

Uh oh!

nimrodkra commented Apr 26, 2026

Uh oh!

claude Bot commented Apr 26, 2026 •

edited

Loading

Uh oh!

capJavert Apr 27, 2026

Uh oh!

capJavert Apr 27, 2026

Uh oh!

capJavert Apr 27, 2026

Uh oh!

capJavert Apr 27, 2026

Uh oh!

capJavert Apr 27, 2026

Uh oh!

capJavert Apr 27, 2026

Uh oh!

nimrodkra commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nimrodkra commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

pulumi Bot commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Neo Explanation

Resource Changes

Uh oh!

nimrodkra commented Apr 26, 2026

Uh oh!

claude Bot commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: feat/major-headlines-push-notifications

Issues

1. Scalability problem: full-table user query on every highlight event (src/workers/notifications/majorHeadlineAdded.ts:51-58)

2. Significance parsing is fragile in the push worker (src/workers/newNotificationV2Push.ts:38-44)

3. uniqueKey does not prevent duplicate in-app notifications for the same post+channel (src/notifications/generate.ts:771)

4. MajorHeadlineAdded email is opted-in by default but has no email template

Minor observations

Looks good

Uh oh!

nimrodkra commented Apr 26, 2026

Uh oh!

claude Bot commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Follow-up review: addressed feedback verified ✓

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

nimrodkra commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nimrodkra commented Apr 26, 2026 •

edited

Loading

pulumi Bot commented Apr 26, 2026 •

edited

Loading

claude Bot commented Apr 26, 2026 •

edited

Loading

1. Scalability problem: full-table user query on every highlight event (`src/workers/notifications/majorHeadlineAdded.ts:51-58`)

2. Significance parsing is fragile in the push worker (`src/workers/newNotificationV2Push.ts:38-44`)

3. `uniqueKey` does not prevent duplicate in-app notifications for the same post+channel (`src/notifications/generate.ts:771`)

4. `MajorHeadlineAdded` email is opted-in by default but has no email template

claude Bot commented Apr 26, 2026 •

edited

Loading