🔄️ Improve conversation list performance - #6498
Merged
Merged
Conversation
AndyScherzinger
added
enhancement
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 14, 2026 15:47
a137536 to
8c9e228
Compare
AndyScherzinger
force-pushed
the
feat/noid/convoListUpdate
branch
2 times, most recently
from
August 14, 2026 17:08
f71c91f to
afcaf62
Compare
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 14, 2026 17:22
8c9e228 to
4f75c5e
Compare
AndyScherzinger
force-pushed
the
feat/noid/convoListUpdate
branch
2 times, most recently
from
August 14, 2026 18:33
bfcd87c to
1374082
Compare
AndyScherzinger
changed the title
AndyScherzinger
added
3. to review
mahibi
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 18, 2026 10:14
4f75c5e to
4711f95
Compare
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
2 times, most recently
from
August 18, 2026 18:30
6ee450c to
20790c7
Compare
AndyScherzinger
force-pushed
the
feat/noid/convoListUpdate
branch
from
August 18, 2026 22:40
1374082 to
883b385
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32193753681/artifacts/9345340420 |
AndyScherzinger
force-pushed
the
feat/noid/convoListUpdate
branch
from
August 19, 2026 05:03
883b385 to
f0b7da9
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32218002583/artifacts/9353109650 |
The room list was fed by manual emissions: getRooms emitted a database snapshot and a post-sync snapshot, and nothing else. Any change to the conversations table in between stayed invisible until the next full fetch cycle. roomListFlow now observes the conversations table for the selected account, making the database the single source of truth: every write reaches the UI reactively, whoever made it. getRooms shrinks to selecting the account and triggering the background sync, which stays in place unchanged as the authority and self-healing safeguard (deletions, statuses, drift correction). The sync applies deletions and upserts in one transaction, so observers see a single consistent update per sync instead of intermediate states. Unlike the MutableSharedFlow it replaces, the database-backed flow can throw, and getRoomsStateFlow collects it eagerly in the viewModelScope - an uncaught exception there would crash the app, so it is caught and surfaced as GetRoomsErrorState. updateConversationLocallyAndEmit lost its manual emission and with it any difference to updateConversation, so it is removed and all call sites point at updateConversation. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…h-ups Background message catch-ups (room list prefetch, push notifications) cached the messages but never touched the conversation entry, so the conversation list kept showing the stale last message and unread count until the next full room list sync answered - even though the fresh data was already on the device. After a room-level catch-up that persisted messages, the room's conversation entry is now updated with the newest persisted message (skipping system messages that never become a conversation's preview), its activity timestamp and a locally derived unread count. The count mirrors the server's calculation (spreed's ChatManager.getUnreadCount counts the comment and object_shared verbs only), is only derived when the latest chat block reaches back to the last read message, and excludes the user's own messages since the server advances the author's read marker on every post - which the locally cached marker may lag behind. Concurrency with the room list sync is handled without locking: the write is a single guarded UPDATE that only applies while the derived state is newer than the stored one, so the sync - which remains the authority and self-healing safeguard - can never be overwritten with older data and no read-modify-write window exists. The derivation runs inside the existing per-room catch-up mutex. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Marking a chat as read (or unread) only sent the marker to the server in a fire-and-forget call: the local conversation entry was never updated, so the conversation list showed a stale unread badge until the next room list sync answered - and a failed call was only logged, silently losing the read state. The read state is now written into the local conversation entry immediately (with the unread count recounted from the cached messages the way the server counts), so the list reflects it the moment the user leaves the chat. Sending the marker moves to a ReadMarkerSyncWorker: network-constrained, exponential backoff, at most three attempts, and unique per room with REPLACE so the newest marker always wins and a retry can never ship a stale marker backwards. The server remains the authority, with one narrowly scoped exception: a server response computed before a concurrently sent marker reached the server would revert the entry to unread until the next sync. Markers are therefore tracked as pending until a sync confirms their delivery, and while the server's read state is provably behind a pending marker it is kept out of the merge - for the room list sync and the single-room refresh alike. Once the server has caught up (or moved past the marker, e.g. read further on another device) the server state applies unchanged, so marking as unread from another device keeps working. When sending ultimately fails the worker releases the pending marker and the next sync restores the server state, so the inconsistency window stays bounded by the retry backoff or one sync cycle. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Covers, against a real in-memory database, that a background catch-up updates the conversation entry from the fetched messages (preview, activity, derived unread count with system messages excluded the way the server excludes them), that the guarded update never regresses a newer stored entry, that the local read state write-through resets and recounts the unread badge (own messages excluded), that neither a room list sync nor a single-room refresh can revert the read state while its marker is pending - with the server's authority restored once it confirms the marker, including a lower read state from marking unread on another device - and that the reactive room list flow delivers plain database writes without any fetch call. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
ChatMessageSyncer had grown into owning message fetching, chat blocks, catch-up coalescing, HTTP sync anchors and - since the conversation list freshness work - conversation entry updates, the local read state and the pending read markers. Move the conversation-list concerns into a dedicated ConversationListUpdater: reflecting catch-ups in the conversation entries, the optimistic read state write-through, and the pending markers with their stale-server-response guard. Behavior is unchanged; the syncer delegates the catch-up reflection, and the chat repository, the read marker worker and the conversations repository now use the updater directly instead of going through the syncer. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Returning from a chat to the conversation list made the list visibly jump shortly after rendering. The saved scroll position was re-armed on every resume and restored on the next room list emission - which used to be an immediate local snapshot, making the restore an invisible no-op. With the list derived reactively from the database, the state current at resume never re-emits, so the first post-resume emission is the server sync response seconds later: scrollToItem then re-anchored an already rendered (and possibly slightly changed) list under the user's eyes. Restore the position only once per activity lifetime: after a recreation the LazyListState is genuinely lost and the first emission - which the reactive flow now delivers immediately on subscription - restores it, while a plain resume keeps the retained state's position naturally, without any late scrollToItem. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The conversation list actions - mark as read/unread (menu and swipe), add to or remove from favorites, and tag assignment - write the local conversation entry optimistically and revert on server failure, but had no protection against the same race the read marker had: a room list sync (or single-room refresh) whose response was computed before the action reached the server reverts the entry to the old state until the next sync, flipping badges and jumping favorite-sorted rows. Generalize the pending read marker mechanism in ConversationListUpdater to these actions: each optimistic write registers a pending change, and while the server response provably doesn't reflect it yet, the local state is kept in the merge. Once the server confirms the change it is released and the server state applies unchanged - so changes made on other devices keep flowing - and a failed action releases its pending change when reverting, restoring the server's authority. Mark-as-read reuses the pending read marker itself; mark-as-unread, favorites and tags get their own pending state with value-based confirmation. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…other list actions Archiving from the conversation list was the last list action without an optimistic write: it awaited the server call and a full room list sync before the row moved, and a concurrent sync whose response was computed before the archive reached the server could still flip the row back until the next refresh. Move the action into ConversationsListViewModel following the favorite pattern: the archived flag is written locally first (the reactive list moves the row instantly, no extra sync round-trip), the server call is retried once and reverted on failure, and the change is registered as a pending archived flag in ConversationListUpdater so stale server responses cannot revert it before the server confirms. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
AndyScherzinger
force-pushed
the
feat/noid/convoListUpdate
branch
from
August 19, 2026 08:50
f0b7da9 to
7701c50
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32234557614/artifacts/9358783407 |
Collaborator
|
Not for this PR, but an idea for a followup PR. Summarized by claude: Allow favorite/archive/mark-unread/tags while offlineProblemThe conversation list's quick actions (favorite, archive, mark as unread, tags) are hidden while offline — gated by isOnline in ConversationList.kt:392 (swipe) and networkMonitor.isOnline.value in ConversationsListActivity.handleConversationLongClick (long-press menu). This predates the optimistic-write + pending-guard mechanism added in feat/noid/convoListUpdate, which now makes it safe to write these changes locally and reconcile with the server later. Mark-as-read/unread already gets this treatment for the read marker specifically: ChatViewModel.setChatReadMessage writes the local state immediately and hands delivery to ReadMarkerSyncWorker — a NetworkType.CONNECTED-constrained, exponential-backoff, disk-persisted WorkManager job. Favorite, archive, mark-unread (from the list), and tags do not have this — they call withRetry(1) inline in the viewmodel coroutine (ConversationsListViewModel.addConversationToFavorites, toggleConversationArchive, markConversationAsUnread, ConversationTagsViewModel) and revert the optimistic write immediately on any failure, including "currently offline." Removing the UI gate today would just make those actions flash and then flip back with an error while offline — not the offline-first behavior we want. Proposal
Out of scope
Risks / open questions
|
A background catch-up (room-list-triggered or push-notification-driven) derives its unread count from conversation.lastReadMessage read fresh from the database. A read marker just registered as pending - e.g. from leaving a chat - may not have committed its own updateReadState write to that same row yet by the time a concurrently running catch-up reads it, so the count gets derived from a stale, not-yet-advanced read position. Unlike the room list sync and single-room refresh merge, this write path never consulted the pending marker at all, so nothing caught the spurious positive count before it landed in the conversations table: the conversation flashed as read and then flipped back to unread, staying that way until the next full sync self-healed it. Take the higher of the stored lastReadMessage and the pending marker as the derivation baseline - it's always at least as advanced as what's actually stored, so it closes the race without suppressing unread detection for a message that is genuinely newer than the marker. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Leaving a chat can race the conversation list's own resume-triggered sync: both fire at nearly the same moment, and the sync's response is only guarded correctly against a stale server state if the pending read marker already exists by the time that response gets merged. The marker was only registered inside updateLocalReadState, after two suspending database reads, itself launched fire-and-forget from onPause() - a real device trace showed the list's sync firing 230ms before the marker was actually registered, a gap that widens further under main-thread contention. During that window the sync sees no pending marker yet and applies the server's outdated read state unguarded, flipping the conversation back to unread until the next sync. Split registration out into its own synchronous, non-suspending call and run it before anything is launched, so the marker is armed the instant setChatReadMessage returns - no coroutine dispatch and no database read stand between deciding to mark as read and the guard being in place. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32256159443/artifacts/9366605775 |
mahibi
approved these changes
Aug 19, 2026
mahibi
left a comment
mahibi
left a comment
Collaborator
There was a problem hiding this comment.
Great improvements again! 🚀 👍
during debugging i was sometimes still able to reproduce this:
- got to chat with unread messages
- read all
- go back
- conversation is marked as read
- but a second later it's marked as unread
- only after pull it is marked as read again
With the two commits i added this seems to be fixed. I was not able to reproduce it anymore.
I might create more followup PR's but this one should be merged.
Member
Author
|
Nice improvements @mahibi 🎉 |
8 tasks
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.
For the user interface, rely on the database, make action where useful optimistic with self-healing in place based on the regular server-fetches. Sagefuard the changes via actions when competing with server fetches being in-flight, likely impacting the conversation list data.
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)