Batching receipts should be per room/thread, not just per room

[andybalaam]

Link to problem area: https://github.com/matrix-org/matrix-spec/blob/main/content/client-server-api/modules/receipts.md?plain=1#L211

Issue
What is wrong?

In "Server behaviour" under "Receipts" we say:

For efficiency, receipts SHOULD be batched into one event per room
before delivering them to clients.

However, if a user is sending receipts for multiple threads in the same room simultaneously, this batching would prevent sending some of those receipts.

Ideally, the format would not be:

room_id: receipt_type: user_id: info_including_thread_id

but something like:

room_id: thread_id: receipt_type: user_id: info

But it's probably too late for that.

Expected behaviour

So instead I suggest rewording the above sentence to something like:

For efficiency, receipts SHOULD be batched into one event per room /thread
before delivering them to clients.

I haven't been able to verify server behaviour in this situation, but maybe a server dev could comment - do we actually lose receipts in this circumstance at the moment?

[turt2live]

From memory, Synapse doesn't batch read receipts - the SHOULD in the spec makes this legal behaviour.

The added /thread text (maybe saying and thread instead?) sounds good as a clarification to me.

[richvdh]

Rendered spec link: https://spec.matrix.org/v1.9/client-server-api/#server-behaviour-2

[richvdh]

I guess this was missed when MSC3771 added threaded RRs

[kegsay]

From memory, Synapse doesn't batch read receipts - the SHOULD in the spec makes this legal behaviour.

It does batch read receipts, and it batches them incorrectly. Here's a test which demonstrates this matrix-org/complement#709 - Synapse source: https://github.com/element-hq/synapse/blob/v1.101.0/synapse/storage/databases/main/receipts.py#L463