feat(threads): automatically subscribe a user to a thread under the semantics of MSC4306 #5462

bnjbvr · 2025-07-29T13:47:03Z

Based on #5455 + #5461, this implements automatic subscription to threads which explicitly mention the current user (via an intentional room or user mention), and likewise sends a manual subscription to a thread when replying into it.

There's a single background task, owned by the event cache, that will listen to two input streams, and create automatic thread subscriptions based on these streams.

one stream is the global event cache linked chunk update stream, introduced in this PR (and that could be fused in the future with the RoomEventCacheGenericUpdate stream). Based on the new events propagated in a linked chunk update, we can compute the notifications for those, and decide to subscribe to a thread, based on those.
the other stream is the global send queue stream (thanks for adding this in another PR!). Ideally, we'd have the full event's content when it's sent, and we could only react to Sent updates. We could embed the sent event in the Sent variant, as a simplification; otherwise, we keep just a bit of state, to link the to-be-sent event (transaction id) to its thread root.

Topics to discuss:

having the event cache own this task is a bit strange since it also listens to the send queue. We could put it in the client, but at this point, since both the send q and the event cache should be enabled at the same time, and we'd want to have the event cache stream around, I thought it was just as simple to put it in the event cache.
as hinted above: the send q update Sent could embed the sent event, which would avoid the mapping of to-be-sent event to a thread root.

PR is a bit big, but mostly because of the tests; the core code isn't that big.

Part of #4869.

codecov · 2025-07-29T14:00:03Z

Codecov Report

❌ Patch coverage is 91.74312% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.58%. Comparing base (247ec1d) to head (ec33f24).
⚠️ Report is 16 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/event_cache/mod.rs	80.76%	5 Missing ⚠️
crates/matrix-sdk/src/event_cache/room/mod.rs	86.66%	0 Missing and 2 partials ⚠️
crates/matrix-sdk-common/src/serde_helpers.rs	85.71%	0 Missing and 1 partial ⚠️
crates/matrix-sdk/src/room/mod.rs	96.55%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5462      +/-   ##
==========================================
+ Coverage   88.56%   88.58%   +0.02%     
==========================================
  Files         340      340              
  Lines       93686    93779      +93     
  Branches    93686    93779      +93     
==========================================
+ Hits        82975    83078     +103     
+ Misses       6578     6569       -9     
+ Partials     4133     4132       -1

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

codspeed-hq · 2025-07-29T14:01:19Z

CodSpeed Performance Report

Merging #5462 will not alter performance

_{Comparing bnjbvr/auto-sub-threads (ec33f24) with main (0e70a2f)}

Summary

✅ 31 untouched benchmarks

Hywan

Looks good. I don't understand all the details for the moment. I've left a couple of questions. Thanks for your contributions!

Hywan · 2025-08-13T07:21:32Z

crates/matrix-sdk/src/event_cache/pagination.rs

@@ -298,6 +303,8 @@ impl RoomPagination {
                });
            }

+            subscribe_to_new_threads(&room, new_thread_subs).await;


Can you add a comment here to explain why we call subscribe_to_new_threads at this place please?

Re-reading this, I don't understand why we subscribe to threads here :-].

This is the "main" events timeline, which has threaded and non-threaded events. So when we're back-paginating, we might run into threaded for which no subscriptions have ever been computed (i.e. the threads existed before we created the subscription mechanism). In this case, we still want to be able to "catch up" and automatically subscribe the user.

Hywan · 2025-08-13T07:28:53Z

crates/matrix-sdk/src/event_cache/room/threads.rs

+/// [`push_context_for_threads_subscriptions`] function, available in the same
+/// module.
+#[derive(Debug, Default)]
+pub struct ThreadPushContext(Option<PushContext>);


Why not type ThreadPushContext = Option<PushContext>? What the benefits of having a new type?

Newtypes allow hiding the inner workings. I don't want users to pass a general ThreadPushContext to the method that determines whether we should subscribe to a thread or not, because the push context shouldn't have the thread-subscription check. So it has to be a specific flavor of PushContext to be passed in there, and I don't want the user to care about this; they can create a ThreadPushContext with the other function, and not think too much about the implementation details.

crates/matrix-sdk/src/event_cache/room/threads.rs

crates/matrix-sdk/src/room/mod.rs

…updates

…f needed

…c4306 semantics

…iptions

…its own function, and introduce a select! at the top level

… into it

…be sent, for extracting its thread root

bnjbvr · 2025-08-14T11:58:56Z

Ready for another round of review 🥳 This also implements support for manually subscribing when an event is sent via the send queue, as per the semantics of MSC4306.

PTAL 🥳

Hywan

Great great! Exciting work. I've left a couple of feedback, but the overall design is nice.

crates/matrix-sdk/src/event_cache/mod.rs

crates/matrix-sdk/src/event_cache/room/threads.rs

Hywan · 2025-08-18T08:51:42Z

crates/matrix-sdk/src/event_cache/room/threads.rs

@@ -78,6 +93,22 @@ impl ThreadEventCache {
        }
    }

+    // TODO(bnjbvr): share more code with `RoomEventCacheState` to avoid the


Is this TODO addressed later? (just adding a comment to see if that's the case)

Nope it is not. I'd expect all these refactorings to happen around the same time, when we unify backpagination in threads and in the main room -> when persisting threads happens.

No problem.

crates/matrix-sdk/src/event_cache/mod.rs

crates/matrix-sdk-common/src/serde_helpers.rs

bnjbvr force-pushed the bnjbvr/auto-sub-threads branch 5 times, most recently from e95d796 to 8da211c Compare August 12, 2025 13:14

bnjbvr marked this pull request as ready for review August 12, 2025 13:14

bnjbvr requested a review from a team as a code owner August 12, 2025 13:14

bnjbvr requested review from Hywan and removed request for a team August 12, 2025 13:14

bnjbvr marked this pull request as draft August 13, 2025 08:16

Hywan reviewed Aug 13, 2025

View reviewed changes

bnjbvr force-pushed the bnjbvr/auto-sub-threads branch from d0f4d67 to 2376763 Compare August 14, 2025 10:50

bnjbvr added 8 commits August 14, 2025 13:50

feat(event cache): add a way to subscribe to any room's linked chunk …

67e3bee

…updates

feat(room): add a new function that will only subscribe to a thread i…

6dcc394

…f needed

feat(event cache): automatically subscribe to threads according to ms…

71b23c6

…c4306 semantics

test(event cache): add tests for automatic thread subscriptions

20f4c9d

feat(multiverse): enable threading support for multiverse with subscr…

08b99d3

…iptions

refactor(event cache): move the listening to linked chunk updates to …

aa23aab

…its own function, and introduce a select! at the top level

feat(event cache): also subscribe to a thread if we've sent a message…

0ac8699

… into it

refactor(event cache): avoid deserializing the full event content to …

a682a46

…be sent, for extracting its thread root

bnjbvr force-pushed the bnjbvr/auto-sub-threads branch from d2c99cb to a682a46 Compare August 14, 2025 11:53

bnjbvr marked this pull request as ready for review August 14, 2025 11:58

bnjbvr requested a review from Hywan August 15, 2025 14:13

Hywan requested changes Aug 18, 2025

View reviewed changes

chore: address review comments

ec33f24

bnjbvr requested a review from Hywan August 18, 2025 11:36

Hywan approved these changes Aug 18, 2025

View reviewed changes

bnjbvr merged commit b43aac1 into main Aug 18, 2025
52 checks passed

bnjbvr deleted the bnjbvr/auto-sub-threads branch August 18, 2025 13:10

feat(threads): automatically subscribe a user to a thread under the semantics of MSC4306 #5462

feat(threads): automatically subscribe a user to a thread under the semantics of MSC4306 #5462

Uh oh!

Conversation

bnjbvr commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #5462 will not alter performance

Summary

Uh oh!

Hywan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bnjbvr commented Aug 14, 2025

Uh oh!

Hywan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjbvr commented Jul 29, 2025 •

edited

Loading

codecov bot commented Jul 29, 2025 •

edited

Loading

codspeed-hq bot commented Jul 29, 2025 •

edited

Loading