Repository navigation
fix: mark a thread read only once its conversation loads on screen - #603
Merged
Merged
Conversation
A subscription reaching the host marked the thread read, so a remote client whose subscription arrived late, or a phone whose conversation loaded behind the thread list, made a thread read that never reached the screen. The client now sends MarkSessionRead with the updated_at it showed, once the conversation has loaded and is on screen.
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
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.
Summary
A thread was marked read when the host received a subscription to it. On a bad network a remote client's subscription can arrive long after the user opened a thread, saw a blank loading screen and went back. A phone also keeps the thread selected on the thread list, so its conversation still loads there. Either way the thread became read although nothing reached the screen.
Now the client says when a thread is read, and only once its conversation has loaded and is on screen.
throughis theupdated_atthe client actually showed, so a delayed acknowledgement never covers updates that came after it and never rewinds a newer one.updated_at. That replaces the host's own advance for subscribed threads.MarkSessionReadis a new command. It is noted under "Unreleased" abovePROTOCOL_VERSIONand the number is not bumped (principle 9).Evidence
Each new test fails with its part of the fix removed and passes with it.
only_a_loaded_conversation_marks_a_thread_read. It delivers a subscribe and an unsubscribe the way a reconnecting client replays them.Before: fails with
a late subscription is not a read.After: passes. It also checks that a delayed older acknowledgement does not rewind, and that a later update makes the thread unread again.
a_thread_is_reported_read_once_its_conversation_loads.Before: without the acknowledgement, the expected
MarkSessionRead("shown", 100)never appears.After: passes. Nothing is sent for a view left before it loaded, or while it loaded off screen. One acknowledgement is sent when it is shown, and another when its
updated_atadvances.a_conversation_that_loads_behind_the_thread_list_is_not_read. It restores a paired phone on a thread page, goes back before the baseline arrives, then delivers it.Before: with on-screen tracking forced on, it sends
["thread-a"]while the list is showing.After: it sends nothing until the thread page is shown again.
Changed tests:
updates_on_the_viewed_thread_do_not_mark_it_unreadis replaced by the host test above. Its contract that a viewed thread stays read through updates now belongs to the client store test, since the client acknowledges them.index_and_visit_changes_cross_the_wire_one_thread_at_a_timenow changes a visit withMarkSessionReadinstead of the removedmark_visited. Its contract, that only the changed visit crosses the wire, is unchanged.Checks run locally on macOS:
cargo fmt --all --checkcargo clippy --workspace --all-targets --locked -- -D warningscargo nextest run --workspace --lockedcargo check -p tcode-web --target wasm32-unknown-unknownandcargo check -p tcode-ios --target aarch64-apple-ios-sim, both with-D warningsAndroid, Windows, Linux and
cargo macheteare left to CI. Nothing was tried on a real phone over a degraded network.Merge Danger
Door: two-way
The stored
last_visitedformat is unchanged, and reverting restores the old trigger.Blast Radius: unread-dots
Unread dots on every client may now lag until a conversation has actually loaded on screen. A client built before this change never sends
MarkSessionRead, so against a new host its threads never become read. Clients and hosts come from one tree between releases.