Skip to content

Conversation

@Hywan
Copy link
Member

@Hywan Hywan commented Dec 15, 2025

Based on top of #5947. Must be merged before this one.

The problem we are trying to solve is the following:

  • a local event is being sent,
  • the LatestEventValue is LocalIsSending,
  • the local event is finally sent,
  • the LatestEventValue is still LocalIsSending purposely, with the hope that an update from the Event Cache will replace it, thus becoming a Remote.

But sometimes, this update from the Event Cache comes before the update from the Send Queue because the whole mechanism is async with no ordering guarantee. Why is it problem? Because updates from the Event Cache are ignored until the buffer of local LatestEventValues aren't empty (so that local latest event cannot be overwritten and has the priority). It means that if the update from the Event Cache is received before RoomSendQueueUpdate::SentEvent, it is ignored, and the LatestEventValue stays in the LocalIsSending state. That's annoying.

The idea is to introduce a new state: LocalHasBeenSent which mimics Remote, but for a local event. It clarifies the state of a sent event, without relying on the Event Cache.

The patch is twofold:

  1. It introduces the new LatestEventValue::LocalHasBeenSent variant,
  2. It updates the latest_event Room List sorter to handle this new case.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 92.34450% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.51%. Comparing base (1480ede) to head (db6684a).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/timeline/latest_event.rs 78.57% 1 Missing and 8 partials ⚠️
...rates/matrix-sdk/src/latest_events/latest_event.rs 92.10% 6 Missing ⚠️
...k-ui/src/room_list_service/sorters/latest_event.rs 98.55% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #5968    +/-   ##
========================================
  Coverage   88.51%   88.51%            
========================================
  Files         363      363            
  Lines      103490   103590   +100     
  Branches   103490   103590   +100     
========================================
+ Hits        91599    91696    +97     
  Misses       7532     7532            
- Partials     4359     4362     +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 15, 2025

CodSpeed Performance Report

Merging #5968 will not alter performance

Comparing Hywan:fix-sdk-latest-event-sticky-locals (db6684a) with main (1480ede)

Summary

✅ 50 untouched

@Hywan Hywan force-pushed the fix-sdk-latest-event-sticky-locals branch 3 times, most recently from 11e07d3 to 5256dba Compare December 16, 2025 08:13
The problem we are trying to solve is the following:

- a local event is being sent,
- the `LatestEventValue` is `LocalIsSending`,
- the local event is finally sent,
- the `LatestEventValue` is still `LocalIsSending` purposely, with the
  hope that an update from the Event Cache will replace it.

But sometimes, this update from the Event Cache comes **before** the
update from the Send Queue. Why is it problem? Because updates from the
Event Cache are ignored until the buffer of local `LatestEventValue`s
aren't empty, which means that if an update from the Event Cache is
received before `RoomSendQueueUpdate::SentEvent`, it is ignored, and the
`LatestEventValue` stays in the `LocalIsSending` state. That's annoying.

The idea is to introduce a new state: `LocalHasBeenSent` which mimics
`Remote`, but for a local event. It clarifies the state of a sent event,
without relying on the Event Cache.
@Hywan Hywan force-pushed the fix-sdk-latest-event-sticky-locals branch from 5256dba to 1c7d1b6 Compare December 16, 2025 11:33
@Hywan Hywan marked this pull request as ready for review December 16, 2025 11:34
@Hywan Hywan requested a review from a team as a code owner December 16, 2025 11:34
@Hywan Hywan requested review from jmartinesp and poljar and removed request for a team and poljar December 16, 2025 11:34
Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

…enSent`.

This patch changes the semantics of the Room List `latest_event`
sorter by changing “is local” to “is remote like”, to include the new
`LatestEventValue::LocalHasBeenSent` variant.
@Hywan Hywan force-pushed the fix-sdk-latest-event-sticky-locals branch from 2d6de7b to db6684a Compare December 16, 2025 12:18
@Hywan Hywan merged commit 4e90cea into matrix-org:main Dec 16, 2025
55 of 56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants