fix(llc): keep reaction group while its count stays positive - #2858
Conversation
Message.deleteMyReaction dropped the entire ReactionGroup whenever the updated sumScores reached 0, even while the count was still greater than zero, so count-based reaction UIs lost other users' reactions during the optimistic delete until the next server event. Gate the group's survival on count alone, mirroring the backend which derives groups from the reaction count (sum of scores is an independent aggregate that may legitimately be zero). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesReaction group retention
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2858 +/- ##
=======================================
Coverage 72.86% 72.86%
=======================================
Files 429 429
Lines 27698 27698
=======================================
Hits 20183 20183
Misses 7515 7515 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The _reactionGroupsReadValue fallback that synthesizes reaction groups from the old reaction_counts/reaction_scores maps dropped a group when its sum_scores was 0, discarding a still-populated group at parse time. Gate on count only, matching the delete-path fix. Also trims the two reaction-group comments to one terse line each. Addresses review feedback on #2858. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The _maybeGetReactionGroups fallback that derives reaction groups from the old reaction_counts/reaction_scores maps dropped a group when its sum_scores was 0, discarding a still-populated group. Gate on count only, matching the delete-path fix; the score sum may legitimately be zero or negative. Also trims the reaction-group comments to one terse line each. Port of the review follow-up on #2858. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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/lib/src/core/models/message.dart`:
- Around line 175-176: Update the count guard in the reaction-group construction
logic to skip all non-positive legacy counts by changing the zero-only check to
a less-than-or-equal check. Retain groups only when count is positive, while
preserving the existing handling of score values.
🪄 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 Plus
Run ID: 3e01010c-fcbd-44f3-8a28-f6759592791c
📒 Files selected for processing (3)
packages/stream_chat/CHANGELOG.mdpackages/stream_chat/lib/src/core/models/message.dartpackages/stream_chat/test/src/core/models/message_test.dart
- Guard the legacy synthesizer on count <= 0 so a malformed negative count can't build an invalid ReactionGroup (CodeRabbit). - Use a realistic delete-path fixture (own score 1, other score -1) instead of unreachable score 0 reactions (renefloor). - Extend the legacy-payload test to cover a negative score total. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Guard the legacy synthesizer on count <= 0 so a malformed negative count can't build an invalid ReactionGroup. - Use a realistic delete-path fixture (own score 1, other score -1) instead of unreachable score 0 reactions. - Derive the legacy-payload test from the deprecated reactionCounts/ reactionScores params (a real Message) covering a zero/negative total. Port of the review follow-up on #2858. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Messagedropped an entireReactionGroupin two places when a group's summed scores were non-positive, even though itscountwas still positive — so count-based reaction UIs lost other users' reactions:deleteMyReaction(optimistic delete) kept a group only whencount > 0 && sumScores > 0. During an optimistic delete, a group whose scores net to zero/negative vanished until the next server event re-hydrated it._reactionGroupsReadValue(legacy-payload parse fallback that synthesizes groups from the oldreaction_counts/reaction_scoresmaps) discarded a group whensumScores == 0, dropping a still-populated group at parse time. (Flagged in review by @renefloor.)Root cause / backend alignment
The backend derives reaction groups from the
reactionstable viacount(type)/sum(score)grouped by(message_id, type). A group exists iff at least one reaction row remains (count >= 1);sum(score)is an independent aggregate that may legitimately be0or negative and never gates whether the group exists.Both call sites now gate purely on the count.
Changes
deleteMyReaction: retain the group whileupdatedCount > 0._reactionGroupsReadValue: retain the group whilecount > 0(guardcount <= 0so a malformed negative count can't build an invalid group).score: 1, otherscore: -1→ net zero, then negative after delete) and a legacy-payload test covering zero and negative score totals.stream_chatCHANGELOG entries.Testing
flutter test test/src/core/models/message_test.dart test/src/core/models/message_reaction_helper_test.dart— all pass.Fixes FLU-663
🤖 Generated with Claude Code