perf(llc, core): guard channel list event handlers against unnecessary re-sorts#2824
Open
VelikovPetar wants to merge 3 commits into
Open
perf(llc, core): guard channel list event handlers against unnecessary re-sorts#2824VelikovPetar wants to merge 3 commits into
VelikovPetar wants to merge 3 commits into
Conversation
…y re-sorts Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a presence event constant, uses it in channel-list event routing, guards updates for unlisted channels or users, and expands routing and handler tests. ChangesChannel list event handling
Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
VelikovPetar
marked this pull request as ready for review
July 17, 2026 14:28
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_flutter_core/lib/src/stream_channel_list_event_handler.dart`:
- Around line 191-197: Update the user presence/update handler to treat a
channel as matching when the event user appears in either state.members or
state.membership, so membership-only channels still receive the update and list
re-sort. Add a regression test covering an event user present only in
membership.
🪄 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: 3e268090-bafc-4071-b7c8-d55df1a0caf3
📒 Files selected for processing (8)
packages/stream_chat/CHANGELOG.mdpackages/stream_chat/lib/src/event_type.dartpackages/stream_chat_flutter_core/CHANGELOG.mdpackages/stream_chat_flutter_core/lib/src/stream_channel_list_controller.dartpackages/stream_chat_flutter_core/lib/src/stream_channel_list_event_handler.dartpackages/stream_chat_flutter_core/test/mocks.dartpackages/stream_chat_flutter_core/test/stream_channel_list_controller_test.dartpackages/stream_chat_flutter_core/test/stream_channel_list_event_handler_test.dart
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2824 +/- ##
==========================================
+ Coverage 71.28% 71.33% +0.05%
==========================================
Files 430 430
Lines 26934 26940 +6
==========================================
+ Hits 19199 19218 +19
+ Misses 7735 7722 -13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xsahil03x
approved these changes
Jul 17, 2026
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
2 participants
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.
Submit a pull request
Linear: FLU-599
Github Issue: #
CLA
Description of the pull request
Three event handlers in
StreamChannelListControllerunconditionally replaced the entire channel list on every matching event, forcing a full list copy +O(n log n)re-sort even when the event didn't affect any listed channel. This adds a guard to each (matching the one already present inonNotificationRemovedFromChannel):onChannelUpdated/onMemberUpdated— early-return unless the event's channel (event.cid ?? event.channel?.cid) is in the current list.onUserPresenceChanged— member-state updates are preserved exactly; the list is only reassigned (re-sorted) when at least one listed channel actually contained the event user.Also extracts the previously hard-coded
'user.presence.changed'dispatch string into a newEventType.userPresenceChangedconstant.Breaking change
None — not source-breaking. No public signatures change; this is a behavior-preserving micro-optimization. The old "reassign an unchanged list" path was already a no-op at the UI level (suppressed by
PagedValue'sDeepCollectionEquality-based==short-circuit inValueNotifier); the guards only skip the wasted sort + allocation.Performance
Throwaway micro-benchmark (not committed; the "old" numbers were produced by
git stash-ing the real handler, not a copied class), 20k iterations over 30 channels:channel.updated(out-of-list)user.presence.changed(non-member)The presence win is smaller by design: presence events carry no cid, so the handler still scans every channel's members; the guard only saves the copy + re-sort on top of that scan.
Tests
stream_channel_list_event_handler_test.dart— unit-tests the handler in isolation against a mock controller, covering every handler method (and the three guards' listed/unlisted/non-member branches).stream_channel_list_controller_test.dart— theEvent handlinggroup was rewritten from behavior-assertions into a routing test (mock handler injected) that verifies each event type dispatches to the correct method.MockStreamChannelListControllerandMockStreamChannelListEventHandler.Screenshots / Videos
No UI changes.
Summary by CodeRabbit
New Features
user.presence.changedevent type and integrated it into channel list presence handling.Bug Fixes
channel.updated,member.updated, and user/presence updates only refresh/re-sort when they affect channels already in the list.Tests