Skip to content

feat(sdk): Spaces #5509

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

feat(sdk): Spaces #5509

wants to merge 27 commits into from

Conversation

stefanceriu
Copy link
Member

@stefanceriu stefanceriu commented Aug 8, 2025

This series of patches introduces a top level UI oriented interface for interacting with spaces.

The SpaceService offers:

  • a method for retrieving one's top level joined_spaces and a subscribe_to_joined_spaces stream that automatically updates based on client activity
    • to do so the service builds a graph from all the m.space.parent and m.space.children state events it knows about, removing cycles and then only keeping parent spaces
  • a SpaceRoomList that can be used to interact with the rooms hierarchy endpoint and retrieve rooms referencing a certain parent
    • the room list is responsible for holding the pagination state as well as listening to client activity and automatically publishing room state updates through its subscribe_to_room_updates publisher (e.g. when joining one of the rooms)

I do appreciate this is a pretty chunky PR but it should be straight forward to review commit by commit.

@stefanceriu stefanceriu force-pushed the stefan/spaces branch 2 times, most recently from 655477a to f52ebdb Compare August 8, 2025 12:02
Copy link

codecov bot commented Aug 8, 2025

Codecov Report

❌ Patch coverage is 90.84605% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.62%. Comparing base (eefa9ff) to head (3aeface).
⚠️ Report is 67 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...x-sdk-base/src/response_processors/state_events.rs 29.62% 0 Missing and 19 partials ⚠️
crates/matrix-sdk-ui/src/spaces/mod.rs 94.34% 16 Missing ⚠️
crates/matrix-sdk-ui/src/spaces/room_list.rs 91.62% 13 Missing and 3 partials ⚠️
crates/matrix-sdk-ui/src/spaces/graph.rs 94.82% 4 Missing and 2 partials ⚠️
crates/matrix-sdk/src/test_utils/mocks/mod.rs 91.48% 4 Missing ⚠️
crates/matrix-sdk-ui/src/spaces/room.rs 91.89% 0 Missing and 3 partials ⚠️
crates/matrix-sdk/src/room/mod.rs 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5509      +/-   ##
==========================================
+ Coverage   88.57%   88.62%   +0.04%     
==========================================
  Files         340      344       +4     
  Lines       93690    94382     +692     
  Branches    93690    94382     +692     
==========================================
+ Hits        82989    83642     +653     
- Misses       6568     6599      +31     
- Partials     4133     4141       +8     

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

Copy link

codspeed-hq bot commented Aug 8, 2025

CodSpeed Performance Report

Merging #5509 will not alter performance

Comparing stefan/spaces (3aeface) with main (7adaf7b)

Summary

✅ 27 untouched benchmarks
🆕 4 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 Restore session [SQLite][clear] N/A 635.1 ms N/A
🆕 Restore session [SQLite][encrypted] N/A 1.2 s N/A
🆕 Restore session [memory store] N/A 180.6 ms N/A
🆕 Create a timeline with initial events[10000 events] N/A 731 ms N/A

@stefanceriu stefanceriu force-pushed the stefan/spaces branch 3 times, most recently from eab6c0c to 0b14496 Compare August 11, 2025 13:07
Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

First fast review. Keep going!

@stefanceriu stefanceriu force-pushed the stefan/spaces branch 2 times, most recently from 5024faa to a2a4ef8 Compare August 11, 2025 16:14
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

This is only a review on the FFI layer, but looks great!. Can't wait to see it land 🔭

spacex-2-3537887872

@stefanceriu stefanceriu force-pushed the stefan/spaces branch 5 times, most recently from aff6435 to 8dc94da Compare August 12, 2025 13:06
@bnjbvr bnjbvr changed the title Spaces feat(sdk): Spaces Aug 14, 2025
@stefanceriu stefanceriu force-pushed the stefan/spaces branch 2 times, most recently from cde1a3d to e761648 Compare August 14, 2025 14:57
@pixlwave
Copy link
Member

Something weird is going on with the latest version, all of my joined_spaces have a children_count of 0.

There's a branch here if you want to test the updates with diffing:
https://github.com/element-hq/element-x-ios/tree/doug/sdk-space-service

@stefanceriu
Copy link
Member Author

Something weird is going on with the latest version, all of my joined_spaces have a children_count of 0.

There's a branch here if you want to test the updates with diffing: https://github.com/element-hq/element-x-ios/tree/doug/sdk-space-service

Went ahead and fixed the remaining issues with VectorDiffs, seems fine now on your branch. Thanks for testing!

@stefanceriu stefanceriu requested a review from a team as a code owner August 19, 2025 08:35
@stefanceriu stefanceriu requested review from Hywan and removed request for a team August 19, 2025 08:35
Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

I've reviewed up to the graph thingy. Is it possible to squash commits that are updating the same part of the code, they are mostly fixup at this point and it would simplifiy the review process.

I'm also wondering if using Client::rooms_stream could remove the need for some tasks, what do you think?

Comment on lines 48 to 50
println!("Top level children for space {}: {:?}", space_id, result.rooms);

Ok(vec![])
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a debug code?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, with this being the initial commit. I'm happy to squash these if you think evolution history isn't relevant.

Comment on lines 98 to 104
let ok = if let Ok(parents) = room.parent_spaces().await {
pin_mut!(parents);
parents.any(|p| p.is_ok()).await == false
} else {
false
};
(room, ok)
Copy link
Member

Choose a reason for hiding this comment

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

What are you trying to do here? The ok refers to the “back-link”, like a space has a link to the room (from parent to child), and the room has a link to the space (from child to parent)?

I think it deserves a log when the child has no link to the parent, it's an important information.

Also, to remove the need to use tokio_stream, you can do:

let mut top_level_spaces = Vec::new();

for room in joined_spaces {
    if let Ok(parents) = room.parent_spaces().await {
        // do checks

        if everything_is_alright {
            top_level_spaces.push(room);
        }
    } else {
        warn!("…");
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This was trying to figure out what the top level parent spaces are, the spaces that don't have any parents of their own. It turned out to be the wrong approach as child to parent or parent to child relations aren't enforced so it can result in wrong results. It was replaced with a proper graph structure that fills in missing edges in b2ea99c

@stefanceriu stefanceriu force-pushed the stefan/spaces branch 3 times, most recently from 5bd6464 to 9a4893e Compare August 20, 2025 13:32
…states change i.e. they get joined or left.
…ted uniffi version

- automatic Arc inference was introduced in 0.27 and complement is using 0.25
…state events in the sliding sync required state as they're both required to build a full view of the space room hierarchy
…tions to correctly detect all the edges and be able to remove any cycles.

- cycle removing is done through DFS and keeping a list of visited nodes
…ption methods so it can be retained on the client side
…` responses and `SpaceServiceRoomList` instances
…tead automatically setup a client subscription when requesting the joined services subscription
…d components on both the UI and the FFI crates
…ned spaces and space room list subscriptions
…ting up a subscription

- fixes values being reported only after the first sync update
…e it within the space service to reduce the number of iterations required
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