Skip to content

refactor: remove timesmearing#8226

Merged
link2xt merged 2 commits into
mainfrom
link2xt/remove-timesmearing
Jun 13, 2026
Merged

refactor: remove timesmearing#8226
link2xt merged 2 commits into
mainfrom
link2xt/remove-timesmearing

Conversation

@link2xt

@link2xt link2xt commented May 8, 2026

Copy link
Copy Markdown
Collaborator

One problem with removing it is that chat name or description may not be applied if it is done twice within the same second:

&& (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)

Previously the sender increased the timestamp each time, so it was clear which update is the latest, but now the tests that rename the chat multiple times in a row break without sleep(1).

Also had to insert time shift in some "golden" tests because otherwise they become flaky if the messages are sent by different senders and are received not in the order in which they are sent. If the second is the same, they were ordered in reception order, if the second is different they were ordered by timestamp and in the sending order.

@link2xt link2xt force-pushed the link2xt/remove-timesmearing branch 8 times, most recently from 8379b9d to 4f919ce Compare May 9, 2026 08:34
@link2xt link2xt marked this pull request as ready for review May 9, 2026 15:54
@link2xt link2xt force-pushed the link2xt/remove-timesmearing branch from 4f919ce to 2b97913 Compare May 15, 2026 00:05
@link2xt link2xt force-pushed the link2xt/remove-timesmearing branch from 2b97913 to 5e61bba Compare May 15, 2026 01:50
@link2xt link2xt requested review from Hocuri, iequidoo and r10s May 15, 2026 01:53

@iequidoo iequidoo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The problem with removing time smearing is that recently we started to sort messages almost purely by Date, so if messages having the same Date value (e.g. forwarded messages) are delivered reordered, they will be sorted wrongly. Maybe it makes sense to add some auto-incremented Chat-Message-Index header? Also this way, if a backup is transferred to a device with clock a bit in the past, messages sent from it will be ordered correctly.

EDIT: Probably it's better to be solved by ordering by References and In-Reply-To.

@r10s

r10s commented May 17, 2026

Copy link
Copy Markdown
Contributor

idea was to make things simpler, less discussions etc. assumption was to try to remove timesmearing as it might be superfluous today - it was initially introduced to force sorting in classic mua. that assumtion seems to be wrong.

timesmearing still seems to be needed.

surely things can be done differently. however, at this point, before adding new sorting criteria or sort using a tree of replies, add new complexity, new bugs, new work, new discussions: it may be better to keep timesmearing and call it a day. it is comparable few, tested code

@link2xt link2xt force-pushed the link2xt/remove-timesmearing branch from 5e61bba to ed210dc Compare May 19, 2026 23:04
@link2xt link2xt merged commit 293b524 into main Jun 13, 2026
31 checks passed
@link2xt link2xt deleted the link2xt/remove-timesmearing branch June 13, 2026 15:43
@link2xt

link2xt commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

This may cause problems when forwarding 10 messages at once and if these messages get reordered. If this happens often we can reintroduce timesmearing, but only for forwarding (e.g. just by adding 1 second to each date of the message sent in a single forwarding call). For other cases this timesmearing mostly causes problems, e.g. in multi-device tests.

@link2xt

link2xt commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

Merging this broke chat::chat_tests::test_broadcast_change_name and test_broadcasts_name_and_avatar after rebasing. Made a fixup at #8332

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.

3 participants