Threads might skip processing some replies from the sync response

[stas-demydiuk]

TLDR
There is a scenario where SDK may skip processing some events related to the thread from the sync response and as a result, client will not see these events till the page refresh (in the case of the browser client)

Steps to reproduce

1. Open any thread in Element app from user A
2. Other users should send replies to this thread
3. At the same time send a reply to the thread too
4. As a result the sync response should contain events from both, user A and other users in a single batch. (i.e. rooms.join[<roomId>].timeline.events has multiple events related to the thread)
5. Make sure the sequence of events has the following order

[
  ...replies from other user(s) (target replies)
  reply from the user A (from the step 3)
]

it is important that reply from user A happened after reply(s) from other users but they still are in the same sync

In this case, matrix-SDK will process only the reply from user A and skip all previous replies (target replies) from other users

Technical details
Based on my discovery the following flow is happening

1. During the sync processing, all events for the target room are passed to room.addLiveEvents() method here
2. Then there is a loop through all events here

for (const event of events) {
 ...
}

3. In this loop all replies from other users will go to eventsByThread[threadId] map here for further processing

eventsByThread[threadId ?? ""]?.push(event);

while replies from a current user will be processed immediately here and won't go to the eventsByThread[threadId] map because they have transaction_id as well

if (event.getUnsigned().transaction_id) {
  ...
}

4. After the loop by events there is another loop to process all threaded events from the map

  Object.entries(eventsByThread).forEach(([threadId, threadEvents]) => {
    this.addThreadedEvents(threadId, threadEvents, false);
  });

5. At the time when a particular threaded event from the previous step will be processed by the thread (thread.addEvents(events, toStartOfTimeline);) all replies from the current user will be already processed, and because replies from the current user happened later than other replies (based on a sync response events order) these replies will be skipped in Thread.addEvent() function here because they won't match any condition

1. (!Thread.hasServerSideSupport) -> false, because server has support
2. (!toStartOfTimeline && this.initialEventsFetched && isNewestReply) -> **false, because isNewestReply is now false**
3. (event.isRelation(RelationType.Annotation) || event.isRelation(RelationType.Replace)) -> false

If there were no replies from the current user, condition 2 will be true (isNewestReply = true) and all events are processed normally