Skip to content

Conversation

@Johennes
Copy link
Contributor

@Johennes Johennes commented Dec 4, 2025

Room::load_event_with_relations currently tries to load the event and any known relations from the cache first. If that fails, /event is called to retrieve the event itself. Nothing is done to also load the relations, however.

This PR extends load_event_with_relations to also issue /relations requests to load any relations when the cache look-up failed.

  • Public API changes documented in changelogs (optional)

@Johennes Johennes force-pushed the johannes/event-with-relations-from-remote branch from 97559da to 6704df8 Compare December 4, 2025 12:00
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 85.93750% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.50%. Comparing base (238e4e8) to head (2e6b214).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...matrix-sdk-ui/src/timeline/pinned_events_loader.rs 72.00% 3 Missing and 4 partials ⚠️
crates/matrix-sdk/src/event_cache/room/mod.rs 93.93% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5930   +/-   ##
=======================================
  Coverage   88.50%   88.50%           
=======================================
  Files         362      362           
  Lines      103291   103351   +60     
  Branches   103291   103351   +60     
=======================================
+ Hits        91413    91471   +58     
+ Misses       7535     7533    -2     
- Partials     4343     4347    +4     

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

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 4, 2025

CodSpeed Performance Report

Merging #5930 will not alter performance

Comparing Johennes:johannes/event-with-relations-from-remote (2e6b214) with main (238e4e8)

Summary

✅ 50 untouched

@Johennes Johennes marked this pull request as ready for review December 4, 2025 12:25
@Johennes Johennes requested a review from a team as a code owner December 4, 2025 12:25
@Johennes Johennes requested review from andybalaam and removed request for a team December 4, 2025 12:25
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Good catch. Looks good to me but I'm going to ask someone from the Rust team to have a quick check, especially on your question about limiting the number to fetch.

@Johennes Johennes force-pushed the johannes/event-with-relations-from-remote branch from 6704df8 to 64aee66 Compare December 5, 2025 10:09
…alling back to the network

Signed-off-by: Johannes Marbach <[email protected]>
@Johennes Johennes force-pushed the johannes/event-with-relations-from-remote branch from 64aee66 to 956c6d7 Compare December 5, 2025 10:12
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

A couple of questions to resolve.

… when falling back to the network

Change limit to 256

Signed-off-by: Johannes Marbach <[email protected]>
… when falling back to the network

Cache related events after loading them

Signed-off-by: Johannes Marbach <[email protected]>
… when falling back to the network

Try loading relations from the cache before falling back to the server

Signed-off-by: Johannes Marbach <[email protected]>
@Johennes Johennes requested a review from andybalaam December 5, 2025 17:28
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