Expose MSC4354 Sticky Events over MSC4186 (Simplified) Sliding Sync. by reivilibre · Pull Request #19591 · element-hq/synapse

reivilibre · 2026-03-20T14:46:01Z

Follows: #19487
Part of: MSC4354 whose experimental feature tracking issue is #19409

This PR implements the Sliding Sync (MSC4186) extension described in MSC4354, allowing sliding sync clients
to receive sticky events in a reliable way.

The logic is much the same as for oldschool sync (implementation in #19487),
although in the sliding sync extension, the client can choose their own limit
and must control their own pagination through an extra token in the extension request/response bodies.

EDIT: Note this PR does not yet send down existing sticky events in the room when the room has been newly-joined.
This newly-discovered gap is tracked at #19662 and will be addressed for both current sync and MSC4186 SSS soon.

This pull request is commit-by-commit review friendly.

Add gather_optional_coroutines/7 overload
Add fields for sticky events sliding sync extension
Add explicit Absent utility type
Implement sliding sync extension for sticky events
Add sliding sync extension test
drive-by docstring tweak on ordering

reivilibre · 2026-03-20T15:40:21Z

+    If you want a Pydantic-compatible Sentinel that is suitable for expressing
+    'absent from some parsed JSON payload' or equivalent, see `Absent`.


Being honest, I discovered Sentinel.UNSET_SENTINEL after having written Absent, so this is post-hoc justification, however I also think it's valuable to have a clean domain-specific 'absent' marker that is not also repurposed for other uses throughout the code. The name Absent also seems clearer to me than Sentinel if I was reading a Pydantic schema, but it's also fair to say that a quick go-to-definition would clear that up.

Open to opinions though; it could be the case that it makes sense to unify them after all.

Probably makes sense to unify all of them as all of the use cases are absent use cases.

I'm not really understanding the distinction between "non-API-facing" either.

Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>

Fixes builtins.NotImplementedError: Cannot check isinstance when validating from json, use a JsonOrPython validator instead.

This reverts commit 6883dcd. EPEL 10 only had 2.9.2 so try to work around issue without updating

builtins.NotImplementedError: Cannot check isinstance when validating from json, use a JsonOrPython validator instead.

MadLittleMods · 2026-04-29T22:46:04Z

+    For a Sentinel for internal (non-API-facing) use, instead consider
+    `Sentinel.UNSET_SENTINEL`.


Wording seems off

Suggested change

For a Sentinel for internal (non-API-facing) use, instead consider

`Sentinel.UNSET_SENTINEL`.

For internal (non-API-facing) use, consider `Sentinel.UNSET_SENTINEL`.

(also applies to the Absent docstring below)

MadLittleMods · 2026-04-29T22:47:35Z

+    # Making this an Enum member makes this compatible with type narrowing,
+    # meaning `x is not Absent` will narrow `x: int | AbsentType` to `x: int` etc.
+    _Absent = object()


We should align the comments between this and Sentinel.UNSET_SENTINEL

MadLittleMods · 2026-04-29T22:48:35Z

+    @classmethod
+    def __get_pydantic_core_schema__(
+        cls, source_type: object, handler: GetCoreSchemaHandler
+    ) -> CoreSchema:


We should comment why we define this

MadLittleMods · 2026-04-29T22:50:04Z

+NonNegativeStrictInt = Annotated[StrictInt, annotated_types.Ge(0)]
+"""A strict integer that must be greater than or equal to zero.
+
+Should be preferred in place of Pydantic's own (lax) NonNegativeInt.


Why? (comment)

MadLittleMods · 2026-04-29T22:52:09Z

+    If you want a Pydantic-compatible Sentinel that is suitable for expressing
+    'absent from some parsed JSON payload' or equivalent, see `Absent`.


Probably makes sense to unify all of them as all of the use cases are absent use cases.

I'm not really understanding the distinction between "non-API-facing" either.

MadLittleMods · 2026-04-29T23:11:09Z

+        since_token = sticky_events_request.since or SlidingSyncStickyEventsToken(
+            sticky_events_stream_id=0
+        )
+        (
+            sticky_events_to_id,
+            room_to_event_ids,
+        ) = await self.store.get_sticky_events_in_rooms(
+            all_interested_room_ids,
+            from_id=since_token.sticky_events_stream_id,
+            to_id=to_token.sticky_events_key,
+            now=now,
+            limit=min(sticky_events_request.limit, StickyEvent.MAX_EVENTS_IN_SYNC),
+        )


The Sticky Events extension having it's own pagination inside Sliding Sync seems like a smell to me.

Related discussion: matrix-org/matrix-spec-proposals#4354 (comment)

MadLittleMods · 2026-04-29T23:19:10Z

+    PATTERN = re.compile(r"^sticky_([0-9]+)$")
+
+    def __init__(self, *, sticky_events_stream_id: int) -> None:
+        self.sticky_events_stream_id = sticky_events_stream_id


Re: #19365 (comment)

Seems like we should be using MultiWriterStreamToken or we should at-least FIXME with our desire

MadLittleMods · 2026-04-29T23:20:34Z

+    @classmethod
+    def __get_pydantic_core_schema__(
+        cls, source_type: object, handler: GetCoreSchemaHandler
+    ) -> CoreSchema:


Comment to explain why we have these methods

MadLittleMods · 2026-04-29T23:26:50Z

+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self._assert_sticky_events_response(response_body, None)
+
+    def test_wait_for_new_data(self) -> None:


I would expect to also see test_wait_for_new_data_timeout (like other extensions)

MadLittleMods · 2026-04-29T23:28:29Z

+"""
+
+
+class SlidingSyncStickyEventsExtensionTestCase(SlidingSyncBase):


I didn't analyze the tests that closely yet. Skimming-wise the coverage seemed pretty good

reivilibre commented Mar 20, 2026

View reviewed changes

reivilibre force-pushed the rei/sticky_events_sliding_sync branch 4 times, most recently from 360c60b to adb3191 Compare March 20, 2026 16:20

reivilibre commented Mar 20, 2026

View reviewed changes

Comment thread pyproject.toml Outdated

reivilibre added 4 commits March 20, 2026 17:14

Add gather_optional_coroutines/7 overload

7da440c

Add explicit Absent utility type

f1e200e

Add NonNegativeStrictInt utility type

46fab79

Add fields for sticky events sliding sync extension

89009df

reivilibre force-pushed the rei/sticky_events_sliding_sync branch from 97efd43 to 6883dcd Compare March 20, 2026 17:24

reivilibre marked this pull request as ready for review March 20, 2026 17:25

reivilibre requested a review from a team as a code owner March 20, 2026 17:25

erikjohnston reviewed Mar 24, 2026

View reviewed changes

Comment thread synapse/handlers/sliding_sync/extensions.py Outdated

MadLittleMods added the A-Sync label Mar 24, 2026

reivilibre added 10 commits April 7, 2026 14:31

Implement sliding sync extension for sticky events

bb4b53e

Add sliding sync extension test

ff8f6b3

drive-by docstring tweak on ordering

6b4cf72

Newsfile

d9a1c43

Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>

drive-by docstring typo fix

9f153a3

Bump Pydantic to >= 2.10

11d2ac7

Fixes builtins.NotImplementedError: Cannot check isinstance when validating from json, use a JsonOrPython validator instead.

Revert "Bump Pydantic to >= 2.10"

ea610b6

This reverts commit 6883dcd. EPEL 10 only had 2.9.2 so try to work around issue without updating

Work around Pydantic < 2.10 error

47c930f

builtins.NotImplementedError: Cannot check isinstance when validating from json, use a JsonOrPython validator instead.

Move SlidingSyncInterestedRooms attributes docs to attributes

37412f4

Add docstring on SlidingSyncInterestedRooms.all_rooms

c5c1b01

reivilibre force-pushed the rei/sticky_events_sliding_sync branch from 6883dcd to c5c1b01 Compare April 7, 2026 16:14

reivilibre requested review from a team and erikjohnston and removed request for erikjohnston April 7, 2026 17:44

MadLittleMods reviewed Apr 29, 2026

View reviewed changes

		If you want a Pydantic-compatible Sentinel that is suitable for expressing
		'absent from some parsed JSON payload' or equivalent, see `Absent`.

		For a Sentinel for internal (non-API-facing) use, instead consider
		`Sentinel.UNSET_SENTINEL`.

	For a Sentinel for internal (non-API-facing) use, instead consider
	`Sentinel.UNSET_SENTINEL`.
	For internal (non-API-facing) use, consider `Sentinel.UNSET_SENTINEL`.

		"""


		class SlidingSyncStickyEventsExtensionTestCase(SlidingSyncBase):

Conversation

reivilibre commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reivilibre commented Mar 20, 2026 •

edited

Loading