fix(unread): fix badge counts stuck at "1" for unvisited rooms in sliding sync#538
Draft
Just-Insane wants to merge 3 commits intoSableClient:devfrom
Draft
fix(unread): fix badge counts stuck at "1" for unvisited rooms in sliding sync#538Just-Insane wants to merge 3 commits intoSableClient:devfrom
Just-Insane wants to merge 3 commits intoSableClient:devfrom
Conversation
…e unread counts With sliding sync, room list subscriptions use timeline_limit=1 so unvisited rooms (no read receipt) only have 1 event in memory. fixupNotifications() re-derives the notification count from whatever events are in the live timeline; for these rooms it computes 1 and overwrites the server-supplied notification_count, causing every unvisited-room badge to show '1' instead of the true count. After the user opens a room the active subscription delivers the full timeline and a read receipt is sent, at which point fixup (applied only for visited rooms) works correctly. This explains why badges 'fix themselves' after visiting a room. Fix: add room.getEventReadUpTo(userId) guard so fixupNotifications is only called for rooms that have been visited this session (have a read receipt or fully-read marker). Unvisited rooms use the raw server-supplied notification_count directly. This also resolves the related symptom where unread counts are wrong after a cache clear or force-close: all rooms start without receipts, so fixup was corrupting every badge on the initial sync pass.
Contributor
|
hey it seems like you're missing a changeset. Can you please add a changeset, as described here https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md#documenting-a-change Thank you :3 |
The previous guard (`getEventReadUpTo(userId)` truthy) only blocked truly unvisited rooms. For visited rooms with a truncated timeline, fixup still ran against 1-event timelines and overwrote the server count with 1. The correct condition is: only run fixupNotifications when the receipt event itself is present in the live timeline -- i.e., the timeline is deep enough for the re-derivation to be accurate. With sliding sync list subscriptions (timeline_limit=1) the receipt for a room with 15 unreads is rarely the very last event, so the receipt won't be in the 1-event timeline and fixup is skipped, preserving the server-supplied count.
Contributor
Author
|
Updated this slightly so that it works better for sliding sync |
Contributor
Author
|
I can't get this reliably working with the current sliding sync implementation. Will leave this open (draft) and come back to it at some point maybe. |
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.
Description
With sliding sync, unvisited rooms (no read receipt) show a badge of 1 instead of the true unread count. After visiting a room, the badge corrects itself. After a cache clear or force-close, all badges are wrong on the initial sync pass.
Root Cause
Room list subscriptions use
timeline_limit=1, so unvisited rooms only have a single event in memory.room.fixupNotifications(userId)re-derives the notification count from whatever events are in the live timeline — for these rooms that's exactly 1 event, so it sets_notificationCounts.total = 1and overwrites the server-suppliednotification_count.Flow for a room with 15 true unreads:
notification_count: 15→ stored in SDKfixupNotifications()→ recalculates from 1-event timeline →total = 1roomHaveUnread()returnsfalse(no read receipt) → stale-clamp fires buthasUserReadEventis also false → total stays at 1!readUpToIdfallback seeshasActivity=true, total=1≠0→ returns{ total: 1 }← bugAfter the user opens a room, the active subscription delivers the full timeline and a read receipt is sent. At that point
fixupgets the full event set and works correctly.Fix
Guard
room.fixupNotifications(userId)withroom.getEventReadUpTo(userId)so fixup is only applied for rooms the user has visited (where a read receipt or fully-read marker is present). Unvisited rooms use the raw server-suppliednotification_countdirectly.This also resolves the related symptom where unread counts are wrong after a cache clear or force-close: all rooms start without receipts, so fixup was corrupting every badge on the initial sync pass.
Fixes #
Type of change
Checklist:
AI disclosure:
Adds
room.getEventReadUpTo(userId)guard around theroom.fixupNotifications(userId)call ingetUnreadInfoinsrc/app/utils/room.ts. When the guard fails (no read receipt present), the function skips the fixup and uses the server-provided notification count directly.