feat: always notify on DM messages like Slack/Discord#405
Conversation
DM messages previously only triggered desktop notifications when they contained explicit @mentions, because the notification system is driven by the home feed's mentions query which joins event_mentions — a table only populated for events with p-tags. DMs without @mentions were invisible to the notification pipeline. This adds a UNION ALL to query_mentions that also returns all messages from DM channels (channel_type = 'dm'), excluding the user's own. The feed response now includes channel_type on each item so the frontend can distinguish DMs. eligibleFeedNotificationItems always includes DM items regardless of the mentions toggle, and notificationTitle shows "Direct message" instead of "@Mention" for DM items. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add channel_type field to FeedItemInfo in Tauri serde bridge so the relay value actually reaches the frontend (was silently dropped). - Wrap each UNION ALL half in parenthesized subqueries with independent ORDER BY … LIMIT so active DMs cannot push @mentions out of results. - Add NOT EXISTS guard on event_mentions in the DM branch to prevent duplicate notifications when a DM also contains an explicit @mention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract EVENT_COLS / EVENT_COLS_UNALIASED constants, push_channel_id_filter(), and collect_stored_events() to eliminate duplication across the three feed query functions. No behavioral change — generated SQL is identical. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9e57f7747
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let channel_type_map: HashMap<uuid::Uuid, String> = all_channels | ||
| .into_iter() | ||
| .map(|c| (c.id, c.channel_type)) |
There was a problem hiding this comment.
This map is derived from list_channels(None), which only returns the 1000 most recent channels (sprout-db/src/channel.rs), so older-but-accessible channels are missing. When feed events come from those channels, channel_type later falls back to an empty string and the desktop logic no longer classifies them as DMs, so DM notifications can be skipped when mentions are disabled. This only manifests in larger deployments, but it directly breaks the new “DMs always notify” behavior.
Useful? React with 👍 / 👎.
…DM notifications The original approach added a UNION ALL subquery in query_mentions to include DM messages in the feed. After review feedback, this is replaced with a client-side WebSocket hook that fires instant DM desktop notifications. Changes: - Revert UNION ALL in query_mentions (back to single indexed query) - Add onDmMessage callback in useLiveChannelUpdates with warmup guard, dedup, self-message filtering, and active-channel awareness - Wire handleDmNotification in AppShell through sendDesktopNotification - Fix first-load suppression in useFeedDesktopNotifications: initialize seenItemIdsRef from localStorage instead of empty set, persist after each notification cycle, remove hasInitializedFeedRef early-return - Exclude DMs from eligibleFeedNotificationItems to prevent double toasts - Re-arm warmup guard on WebSocket reconnect to suppress replay backlog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses three review blockers: 1. 500ms warmup window permanently dropped real DMs — replaced with deterministic `event.created_at < subscriptionStartedAt` check so only backlog events are suppressed, never live messages. 2. Reconnect race condition from overlapping setTimeout timers — reconnect handler now simply updates subscriptionStartedAt, which naturally suppresses replayed historical events without any timers. 3. Empty localStorage causes notification flood on first feed — added a guard that seeds seenItemIdsRef without notifying when the seen set is empty and the feed has items (first load or cleared storage). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…constant The helper is used for both mention and DM dedup — the old name was misleading. Also replaced inline MAX_SEEN_FEED_ITEMS with the existing module-level HOME_FEED_SEEN_MAX_ITEMS constant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tlongwell-block
left a comment
There was a problem hiding this comment.
Independent Review (Pip / @tlongwell-block)
Score: 7/10 — Mergeable. Architecture is correct, three original blockers are fixed. Two minor follow-up items remain.
What's solid
- WS hook for instant DM notifications — correct, follows existing patterns
- Timestamp-based backlog suppression — deterministic, no timing races
- Feed catch-up with localStorage persistence — offline mentions now notify
- DM exclusion from feed path prevents double-toasting
- Backend refactor (shared helpers) is clean and behavior-preserving
channel_typeenrichment flows correctly through the full stack
Follow-up items (non-blocking, for a separate PR)
1. DMs during brief disconnect are lost.
On reconnect, dmSubscriptionStartedAtRef is set to Date.now()/1000. Replay events from the disconnect window have created_at < now, so they're suppressed. The feed path also excludes DMs (eligibleFeedNotificationItems filters channelType !== "dm"). Result: DMs arriving during a 5-60s disconnect are permanently missed by both paths.
Fix: On reconnect, set dmSubscriptionStartedAtRef to lastSeenCreatedAt - RECONNECT_REPLAY_SKEW_SECS (matching the relay's replay since filter) instead of Date.now(). This lets genuinely-missed DMs through while the dedup set prevents double-notification of already-seen events.
2. Empty-seen-set guard can suppress first real feed alert.
If the first feed poll returns 0 items (no mentions yet), seenItemIdsRef stays empty. When the second poll returns a real mention, the guard (size === 0 && items.length > 0) triggers and seeds without notifying. The first real alert is dropped.
Fix: Track initialization separately — use a simple hasSeededRef boolean that's set after the first seed, regardless of whether it came from localStorage or the guard. Only seed once.
Verified non-issues
- Reconnect race: JS event loop guarantees
emitReconnectIfNeeded()fires (updating the timestamp) before any replay events can be dispatched to handlers. The ordering inrelayClientSession.ts:311-312is safe. - Boundary second: An event with
created_at === subscriptionStartedAtpasses through. This is at most one false-positive notification, never a loss. Acceptable.
* origin/main: Add multi-workspace support to desktop app (#409) feat(mobile): add #channel autocomplete to compose bar (#411) fix: close race window that dropped active channel messages (#410) feat(pulse): rich text editor with @mentions, media uploads, and formatting (#407) feat(mobile): multi-workspace support (#408) feat: always notify on DM messages like Slack/Discord (#405)
Summary
DM notifications now fire for every incoming DM, matching Slack/Discord behavior. Previously, only DMs containing explicit @mentions triggered desktop notifications.
Approach
After review feedback from Tyler and Pip, the original server-side UNION ALL approach was replaced with a client-side WebSocket hook for instant DM notifications, plus a first-load suppression fix for offline catch-up.
Three layers of notification delivery:
Real-time WS hook (
useLiveChannelUpdates.ts) — instant DM notification viaonDmMessagecallback, with:seenDmEventIdsRef(capped at 200)Feed poll catch-up (
hooks.ts) — offline DMs caught on next feed poll:seenItemIdsRefinitialized from localStorage instead of empty sethasInitializedFeedRefearly-return that was swallowing first-load itemsServer-side
channel_typeenrichment (relay API) — Home feed items now carrychannelTypefor display and filteringChanges
crates/sprout-db/src/feed.rs— Reverted UNION ALL, kept shared helperscrates/sprout-relay/src/api/feed.rs—channel_typefield in feed responsedesktop/src-tauri/src/models.rs—FeedItemInfo.channel_typein Tauri bridgedesktop/src/features/channels/useLiveChannelUpdates.ts— DM WS hook with warmup, dedup, reconnect handlingdesktop/src/app/AppShell.tsx—handleDmNotificationwired tosendDesktopNotificationdesktop/src/features/notifications/hooks.ts— First-load suppression fix, localStorage persistencedesktop/src/features/notifications/lib/feed.ts— DM title, excluded DMs from feed notification pathdesktop/src/shared/api/types.ts—channelTypeonFeedItemdesktop/src/shared/api/tauri.ts—channel_typetransformerTest plan
🤖 Generated with Claude Code