Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3773: Notifications for threads #3773

Merged
merged 11 commits into from
Sep 20, 2022
159 changes: 159 additions & 0 deletions proposals/3773-notifications-for-threads.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
# MSC3773: Notifications for threads

Since the unread notification count does not consider threads, a client is unable
to separate the unread message counts into threads (as defined by
[MSC3440](https://github.com/matrix-org/matrix-doc/pull/3440))
without iterating over every missing message. Without this, clients are unable to:

* Let users know that a thread has new messages since they last read it.
* Accurately display a count of unread messages in a room (or a thread).

## Proposal

### Modification to push rule processing

When an event which is part of a thread (i.e. has a valid `m.relates_to` with
`rel_type` of `m.thread`) matches a push rule which results in a `notify` action
then the homeserver should note that the notification applies in the context of
the thread ID (parent event ID).
clokep marked this conversation as resolved.
Show resolved Hide resolved

Similar behavior should be applied for an event which results in `notify` action
with a `highlight` tweak set.

### Unread thread notifications in the sync response

Threaded clients can opt into receiving unread thread notifications by passing
a new `unread_thread_notifications` parameter
[as part of the `RoomEventFilter`](https://spec.matrix.org/v1.2/client-server-api/#filtering).
(This is [similar to `lazy_load_members`](https://spec.matrix.org/v1.2/client-server-api/#lazy-loading-room-members),
but only applies to the `/sync` endpoint.):

* `unread_thread_notifications`: If `true`, enables partitioning of unread notification
counts by thread. Defaults to false.

If this flag is set to `true`, for each ["Joined Room" in the `/sync` response](https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3sync)
clokep marked this conversation as resolved.
Show resolved Hide resolved
a new field is added:

* `unread_thread_notifications`: Counts of unread thread notifications for this
room, an object which maps thread ID (the parent event ID) to
`Unread Notification Counts`.

Additionally, the `unread_notifications` dictionary is modified to only include
unread notifications from events which are not part of a thread.

An example of a joined room from a sync response:

```json5
{
"account_data": {
// ...
},
"ephemeral": {
// ...
},
"state": {
// ...
},
"summary": {
// ...
},
"timeline": {
"events": [
{
"event_id": "$143273582443PhrSn:example.org",
// other fields ...
},
{
"event_id": "$SGNxGPGUopcPBUoTTL:example.org",
"m.relates_to": {
"event_id": "$143273582443PhrSn:example.org",
"rel_type": "m.thread"
}
// other fields ...
}
]
},
"unread_notifications": {
"highlight_count": 2,
"notification_count": 18
},
"unread_thread_notifications": {
"$143273582443PhrSn:example.org": {
"highlight_count": 0,
"notification_count": 1
}
}
}
```

## Potential issues
germain-gg marked this conversation as resolved.
Show resolved Hide resolved

### Events related to thread events

Events which are related to thread events (e.g. a reaction to a message which is
part of a thread) would not be represented in the notification count for that
thread, it would still appear in the main timeline's notification count.

With the default push rules this does not seem to be a problem as thread notifications
mostly apply to `m.room.message` and `m.room.encrypted` events.
clokep marked this conversation as resolved.
Show resolved Hide resolved

### Scalability

Rooms with many unread threads could cause some downsides:

* The size of the `/sync` response would increase without bound.
Copy link
Member

Choose a reason for hiding this comment

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

To help with this, could a homeserver choose to omit the unread_thread_notifications field altogether if the unread counts have not changed since the previous /sync response?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean omit the entire field or the individual threads in it? If the latter, I think that would be quite hard to do, but might be doable! The former seems likely doable, but I don't love making it act differently than unread_notifications?

Copy link
Member

@anoadragon453 anoadragon453 Sep 13, 2022

Choose a reason for hiding this comment

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

The former, omitting the field when a new event has appeared in the main timeline (and thus would not update unread thread counts. Especially since many rooms won't have threads in them.

The former seems likely doable, but I don't love making it act differently than unread_notifications?

I agree, and we could change always make this change later of course. I also wouldn't mind making the same change to unread_notifications in the future.

* The effort to generate and process the receipts for each room would increase
without bound.

This is not dissimilar to rooms which are never read, however, as their unread
counts are continually tracked and returned as part of the `/sync` response.

### Clearing unread notifications

This MSC does not attempt to modify how unread notifications (for a thread or
otherwise) are cleared. It currently assumes the rules set forth by
[read receipts](https://spec.matrix.org/latest/client-server-api/#receiving-notifications)
clokep marked this conversation as resolved.
Show resolved Hide resolved
still apply. This will cause some flakiness with unread notifications, as the current
receipt infrastructure assumes that a room's timeline is linear, which is no
longer true.

[MSC3771](https://github.com/matrix-org/matrix-spec-proposals/pull/3771) is a
potential solution for this.
germain-gg marked this conversation as resolved.
Show resolved Hide resolved

## Alternatives
clokep marked this conversation as resolved.
Show resolved Hide resolved

### Using push rules

It might seem that a new push rule `action` (or `tweak`) should be used to control
the behavior of whether an event generates a notification for a thread or the
room itself. There are issues with either approach though:

A new `action` (e.g. `notify_thread`) would mean that additional logic would
need to be defined and added for events which aren't part of a thread but attempt
to use this action. It also conflicts with [MSC3768](https://github.com/matrix-org/matrix-spec-proposals/pull/3768),
which attempts to define another `action` which should also work fine for threads.

A new `tweak` (e.g. `threaded`) does make much sense since there is not need to
clokep marked this conversation as resolved.
Show resolved Hide resolved
pass this through to the push server, which is at odds with the current `tweaks`
mechanism.

Regardless, the main issue with using push rules is that it becomes necessary to
define rules which match threaded events. Whenever adding a new rule, matching rules
would need to be added, but as a thread-specific version.

## Security considerations

N/A

## Unstable prefix

While this feature is in development the following unstable prefixes should be used:

* `unread_thread_notifications` --> `org.matrix.msc3773.unread_thread_notifications`

To detect server support, clients can either rely on the spec version (when stable)
or the presence of a `org.matrix.msc3773` flag in `unstable_features` on `/versions`.

## Dependencies

N/A