fix(chat): back chats up by their applied event cursor, not by message presence - #1308
Merged
Conversation
…e presence A feed sync never backfilled a transcript. It asked `hasMessages` to decide whether a chat had ever been fetched, but the same sync writes each chat's last-message preview as a message row moments earlier — so the answer was always yes, and the newest page was never pulled. Chats opened to a single preview message until the user scrolled. Underneath that sat a worse problem: `latest_event_sequence` was written by two parties meaning two different things. The event stream advanced it to the sequence the client had actually applied; the feed sync overwrote it with the server's reported head via `@Insert(REPLACE)`. Once a sync landed, the next `GetDelta(after:)` resumed from head and silently skipped every event in between. The same whole-row replace also zeroed `analytics_counted_through`, re-counting received messages already counted. The column now has one meaning — the cursor the client has applied: - `ChatMetadataDao.upsert` inserts, or updates only the server-owned columns in place. The two client-owned watermarks are never touched by a sync. - `ChatEntityMapper` stops round-tripping the server head. A row starts at 0, meaning "this transcript has never been fetched"; a chat rebuilt from the database reports 0 for the head — unknown, not "no events". - `MessagingDelegate.loadMessages` seats the cursor at the newest page's frontier, and only ever forward. - The sync's decision reads the cursor: unseated means load the newest page (never a delta, which would re-pull the whole history from sequence 0), behind the server head means delta-sync the missed window. The pre-sync snapshot #1331 added comes out with it. That workaround read the cursor and `hasMessages` *before* the sync wrote, and carried the old cursor on `DeltaSyncNeeded`, because the write clobbered the row it was about to read. Nothing clobbers it now: the sync reads the cursor after its own write, and `performDeltaSync` reads it for itself again. This matches the iOS fix in code-ios-app#628.
bmc08gt
force-pushed
the
fix/chat-backfill-cursor
branch
from
August 26, 2026 16:22
e3dfbaa to
3d0d096
Compare
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
1 participant
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.
A feed sync never backfilled a transcript. It asked
hasMessagesto decide whether a chat had ever been fetched, but the same sync writes each chat's last-message preview as a message row moments earlier — so the answer was always yes, and the newest page was never pulled. Chats opened to a single preview message until the user scrolled.Underneath that sat a worse problem.
latest_event_sequencewas written by two parties meaning two different things: the event stream advanced it to the sequence the client had actually applied, while the feed sync overwrote it with the server's reported head via@Insert(REPLACE). Once a sync landed, the nextGetDelta(after:)resumed from head and silently skipped every event in between — lost messages, not just a stale transcript. The same whole-row replace also zeroedanalytics_counted_through, re-counting received messages that had already been counted.The fix
The column now has one meaning — the cursor the client has applied.
ChatMetadataDao.upsertinserts, or updates only the server-owned columns in place.latest_event_sequenceandanalytics_counted_throughare client-owned watermarks that no server payload carries, so a sync never touches them.ChatEntityMapperstops round-tripping the server head in either direction. A row starts at 0, meaning "this transcript has never been fetched"; a chat rebuilt from the database reports 0 for the head — unknown, not "no events".MessagingDelegate.loadMessagesseats the cursor at the newest page's frontier, and only ever forward — a page older than the cursor must not rewind it.FeedSyncDelegate's decision reads the cursor: unseated means load the newest page (never a delta, which would re-pull the whole history from sequence 0); behind the server head means delta-sync the missed window.Tests
15 new tests across three modules:
FeedSyncBackfillTest— the five backfill decisions. The harness stubshasMessagesto true, which is what reality looks like once the preview row is written, so the never-fetched cases genuinely discriminate against the old condition.MessagingLoadCursorTest— the cursor is seated from the page's highest event sequence, is not rewound by an older page, and stays unseated for an empty page.ChatMetadataDaoTest— an upsert preserves both client-owned watermarks, refreshes the server-owned columns, and inserts an unseen chat.ChatEntityMapperTest— the server head is never written as the applied cursor, and a chat rebuilt from the database reports an unknown head.Parity
Mirrors the iOS fix in code-payments/code-ios-app#628. One shared, deliberate behaviour: a genuinely empty chat leaves its cursor at 0 and is re-queried on each feed sync — iOS does the same (
if head > 0), so this is parity rather than drift.ChatMessageDataSource.hasMessagesnow has no production caller. Left in place as a query helper rather than widen the diff.