feat(llc): Implement local unread counts#2804
Conversation
renefloor
changed the title
renefloor
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds an opt-in local unread-count mode for channels without read events, including local read mutations, message-based count updates, remote-state preservation, tests, mocks, and changelog documentation. ChangesLocal unread count tracking
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MessageEvent
participant Channel
participant MessageRules
participant ChannelClientState
MessageEvent->>Channel: Receive message.new or message.deleted
Channel->>MessageRules: Check unread eligibility
MessageRules-->>Channel: Return eligibility
Channel->>ChannelClientState: Update local unread count
Channel->>ChannelClientState: Apply local read boundary
ChannelClientState-->>Channel: Return recomputed unread state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
renefloor
force-pushed
the
feat/local-unread-count
branch
from
82e91e4 to
a495169
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/stream_chat/test/src/client/channel_test.dart (1)
9810-10089: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive-path test for
Channel.markUnreadlocal recompute.The new group thoroughly covers
markReadandmarkUnreadByTimestamplocal recompute, plusmarkUnread's throw-when-unknown-message case, but there's no test exercising a successfulmarkUnread(messageId)call and asserting the resultingunreadCount— the anchor/microsecond-offset arithmetic inChannel.markUnread(subtracting 1 microsecond from the anchor'screatedAt) is currently only covered indirectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat/test/src/client/channel_test.dart` around lines 9810 - 10089, The local unread-count tests lack a successful-path case for Channel.markUnread. Add a test in the “Local unread count” group that creates locally known messages with distinct timestamps, calls markUnread with a valid message ID, asserts unreadCount reflects only messages after the anchor (including the one-microsecond offset behavior), and verifies no network request via client.markChannelUnread.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_chat/lib/src/client/channel.dart`:
- Around line 3615-3637: The markReadLocally method does not reconcile pending
delivery receipts after updating read state. Add
_client.channelDeliveryReporter.reconcileDelivery([_channel]) to markReadLocally
after updateRead, matching the delivery reconciliation behavior used by related
mark-read flows.
---
Nitpick comments:
In `@packages/stream_chat/test/src/client/channel_test.dart`:
- Around line 9810-10089: The local unread-count tests lack a successful-path
case for Channel.markUnread. Add a test in the “Local unread count” group that
creates locally known messages with distinct timestamps, calls markUnread with a
valid message ID, asserts unreadCount reflects only messages after the anchor
(including the one-microsecond offset behavior), and verifies no network request
via client.markChannelUnread.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 030f7148-3f52-4392-aa77-629db23c3680
📒 Files selected for processing (8)
docs/docs_screenshots/test/src/mocks.dartpackages/stream_chat/CHANGELOG.mdpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/lib/src/core/util/message_rules.dartpackages/stream_chat/test/src/client/channel_test.dartpackages/stream_chat/test/src/core/util/message_rules_test.dartpackages/stream_chat/test/src/mocks.dart
| /// Marks the channel as read locally, without making a network request. | ||
| /// | ||
| /// Used for channels that track unread counts locally (see | ||
| /// [Channel.usesLocalUnreadCount]), since the server rejects the mark-read | ||
| /// endpoint for channels that have read events disabled. | ||
| void markReadLocally({String? messageId}) { | ||
| final currentUser = _client.state.currentUser; | ||
| if (currentUser == null) return; | ||
|
|
||
| final now = DateTime.now(); | ||
| final lastReadMessageId = messageId ?? messages.lastOrNull?.id; | ||
|
|
||
| final existingUserRead = currentUserRead; | ||
| updateRead([ | ||
| Read( | ||
| user: currentUser, | ||
| lastRead: now, | ||
| lastReadMessageId: lastReadMessageId, | ||
| lastDeliveredAt: existingUserRead?.lastDeliveredAt, | ||
| lastDeliveredMessageId: existingUserRead?.lastDeliveredMessageId, | ||
| ), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant symbols and surrounding code
rg -n "_listenReadEvents|reconcileDelivery|markReadLocally|markRead\\(" packages/stream_chat/lib/src/client/channel.dart packages/stream_chat/lib/src -S
# Show the relevant slice around markReadLocally
sed -n '3570,3665p' packages/stream_chat/lib/src/client/channel.dart
# Show the read event listener slice
sed -n '3000,3095p' packages/stream_chat/lib/src/client/channel.dartRepository: GetStream/stream-chat-flutter
Length of output: 8499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "canUseDeliveryReceipts|canUseReadReceipts|delivery receipts|read receipts" packages/stream_chat/lib -SRepository: GetStream/stream-chat-flutter
Length of output: 3601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "void updateRead|updateRead\\(|currentUserRead|channelDeliveryReporter|reconcileDelivery" packages/stream_chat/lib/src -SRepository: GetStream/stream-chat-flutter
Length of output: 4513
🌐 Web query:
stream_chat ChannelDeliveryReporter reconcileDelivery markReadLocally read receipts delivery receipts
💡 Result:
In the Stream Chat SDK (specifically the Dart/Flutter implementation), ChannelDeliveryReporter is a utility class responsible for managing and throttling delivery receipt reporting to the server [1][2][3]. Core Components and Functionality: ChannelDeliveryReporter: This class collects channels requiring delivery acknowledgments and reports them to the server efficiently [1][2]. It includes a throttle mechanism to prevent excessive API calls [2][3]. - reconcileDelivery(Iterable channels): Reconciles the delivery reporting state for a set of channels with their current internal state [1][2]. - submitForDelivery(Iterable channels): Submits channels to the reporter queue for delivery receipt processing [1][2]. - cancelDelivery(Iterable channels): Cancels pending delivery reports for specific channels [1][2]. Delivery Receipts vs. Read Receipts: - Delivery Receipts track when a message has been successfully delivered to a recipient's device [4][5]. This is confirmed via the message.delivered WebSocket event [4]. It is disabled by default and may require specific configuration or support contact [4][5]. - Read Receipts track when a user has actively viewed a message or marked a channel as read [4][5]. This is confirmed via the message.read WebSocket event [4]. Implementation Notes: - The SDK handles delivery confirmation automatically, including throttling and duplicate prevention [4][5]. - Developers can access the full read and delivery state (e.g., lastDeliveredAt, lastDeliveredMessageId) via the channel.state.read object, which provides a list of members and their respective read/delivery statuses [6]. - While markReadLocally is often used in local state management to update UI immediately before the server confirms the action, the SDK provides explicit methods like markChannelRead, markAllRead, and markChannelsDelivered to sync state with the Stream server [7]. The MarkChannelsDelivered typedef is the callback function used to transmit these delivery acknowledgments to the backend [8].
Citations:
- 1: https://pub.dev/documentation/stream_chat/latest/stream_chat/ChannelDeliveryReporter-class.html
- 2: https://pub.dev/documentation/stream_chat_flutter_core/latest/stream_chat_flutter_core/ChannelDeliveryReporter-class.html
- 3: https://pub.dev/documentation/stream_chat_flutter/latest/stream_chat_flutter/ChannelDeliveryReporter-class.html
- 4: https://getstream.io/chat/docs/flutter-dart/message-delivery-and-read-status/
- 5: https://getstream.io/chat/docs/android/message-delivery-and-read-status/
- 6: https://getstream.io/chat/docs/sdk/flutter/stream-chat-flutter/message-list/read-indicators/
- 7: https://pub.dev/documentation/stream_chat_flutter_core/latest/stream_chat_flutter_core/StreamChatClient-class.html
- 8: https://pub.dev/documentation/stream_chat/latest/stream_chat/MarkChannelsDelivered.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1848,1875p' packages/stream_chat/lib/src/client/channel.dart
printf '\n----\n'
sed -n '3288,3335p' packages/stream_chat/lib/src/client/channel.dart
printf '\n----\n'
sed -n '3398,3492p' packages/stream_chat/lib/src/client/channel.dart
printf '\n----\n'
sed -n '4698,4768p' packages/stream_chat/lib/src/client/channel.dart
printf '\n----\n'
sed -n '96,145p' packages/stream_chat/lib/src/client/channel_delivery_reporter.dartRepository: GetStream/stream-chat-flutter
Length of output: 10239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "markReadLocally\\(" packages/stream_chat/lib -SRepository: GetStream/stream-chat-flutter
Length of output: 373
markReadLocally should reconcile delivery receipts too.
It only updates read state; for channels with delivery receipts enabled, skipping _client.channelDeliveryReporter.reconcileDelivery([_channel]) leaves pending delivery state stale after a local mark-read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/stream_chat/lib/src/client/channel.dart` around lines 3615 - 3637,
The markReadLocally method does not reconcile pending delivery receipts after
updating read state. Add
_client.channelDeliveryReporter.reconcileDelivery([_channel]) to markReadLocally
after updateRead, matching the delivery reconciliation behavior used by related
mark-read flows.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2804 +/- ##
==========================================
+ Coverage 71.00% 71.04% +0.04%
==========================================
Files 429 429
Lines 26891 26938 +47
==========================================
+ Hits 19093 19138 +45
- Misses 7798 7800 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // keep their slot. Only applies to channels that track unread counts | ||
| // locally (see [Channel.usesLocalUnreadCount]) — server-driven | ||
| // channels get corrected counts from server read events instead. | ||
| if (hardDelete && _channel.usesLocalUnreadCount && MessageRules.canCountAsUnread(message, _channel)) { |
There was a problem hiding this comment.
Just curious: Don't soft-deletes also decrement the unread count?
| _checkInitialized(); | ||
|
|
||
| if (usesLocalUnreadCount) { | ||
| state!.markUnreadLocally(lastRead: timestamp); |
There was a problem hiding this comment.
In the markUnread(String messageId) method, we call markUnreadLocally with a -1ms of the message timestamp -> but here we call the markUnreadLocally with the exact timestamp. I am curious whether we might be missing some edge-case here, because this method will usually be called with the exact createdAt timestamp of a specific message. So if we call:
final message = targetMessage;
channel.markUnread(message.id);
channel.markUnreadByTimestamp(message.createdAt);Would result in calling markUnreadLocally with a different timestamp. Probably not a big deal, but I wanted to flag it nonetheless!
| updateRead([ | ||
| Read( | ||
| user: currentUser, | ||
| lastRead: now, |
There was a problem hiding this comment.
Question: Does the lastRead mark the moment the channel was read, or the timestamp of the last read message? (I think we should align this with how the server populates this field)
|
|
||
| final localRead = currentUserRead; | ||
| final remoteReads = remoteState.read; | ||
| if (localRead == null || remoteReads == null) return remoteState; |
There was a problem hiding this comment.
Question: Will we always get an initial Read object in the response for a channel which has read/delivery receipts disabled? I am worried that if we do not get an initial object, we won't have a baseline to update on message.new/message.deleted events.
2 participants
Submit a pull request
Linear: FLU-587
CLA
Description of the pull request
This adds an option for local only unread counts, especially interesting for things like livestreams with many updates.
Summary by CodeRabbit
isLocalUnreadCountEnabled).markRead/markUnread/markUnreadByTimestampupdate local unread state without network requests.