Skip to content

feat(event cache): include read receipts that were dormant in the state store#6406

Merged
bnjbvr merged 3 commits intomainfrom
bnjbvr/handle-dormant-read-receipts
Apr 2, 2026
Merged

feat(event cache): include read receipts that were dormant in the state store#6406
bnjbvr merged 3 commits intomainfrom
bnjbvr/handle-dormant-read-receipts

Conversation

@bnjbvr
Copy link
Copy Markdown
Member

@bnjbvr bnjbvr commented Apr 2, 2026

When computing unread counts, if there was no read receipt marked as latest_active, it could be that a read receipt was dormant in the state store, before the code was migrated. In this case, we wouldn't take it into account for the current room, and this would result in an incorrect unread count for that room, until a new receipt is sent for that room.

I think this fixes #6211, as this situation would likely arise for a user with sync v2, migrating to the new SDK where the event cache handles read receipts.

  • I've documented the public API Changes in the appropriate CHANGELOG.md files.
  • This PR was made with the help of AI autocomplete.

@bnjbvr bnjbvr requested a review from a team as a code owner April 2, 2026 09:06
@bnjbvr bnjbvr requested review from poljar and removed request for a team April 2, 2026 09:06
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.86%. Comparing base (32e9626) to head (3063a11).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6406      +/-   ##
==========================================
+ Coverage   89.85%   89.86%   +0.01%     
==========================================
  Files         378      378              
  Lines      103384   103372      -12     
  Branches   103384   103372      -12     
==========================================
+ Hits        92893    92895       +2     
+ Misses       6928     6910      -18     
- Partials     3563     3567       +4     

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

@bnjbvr bnjbvr force-pushed the bnjbvr/handle-dormant-read-receipts branch from e35f7a3 to dfd75c9 Compare April 2, 2026 09:26
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 2, 2026

Merging this PR will improve performance by ×2.5

⚡ 1 improved benchmark
✅ 49 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation Restore session [memory store] 276.5 ms 112.3 ms ×2.5

Comparing bnjbvr/handle-dormant-read-receipts (3063a11) with main (a1157d2)

Open in CodSpeed

Copy link
Copy Markdown
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Looking good, thanks.

) {
// Implementation note: we want to prioritize a `ReadPrivate` (private) receipt
// over a `Read` (public) one, as it's more likely to be ahead.
for receipt_type in [ReceiptType::ReadPrivate, ReceiptType::Read] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might make sense to put this array into a global const since it's now used here as well as in select_best_receipt().

While it's not very likely that we're going to extend the interesting ReceiptTypes any time soon, when we do so, it's likely that those two places will need to be extended in the same manner.

@bnjbvr bnjbvr enabled auto-merge (rebase) April 2, 2026 12:13
@bnjbvr bnjbvr merged commit a65fec3 into main Apr 2, 2026
52 checks passed
@bnjbvr bnjbvr deleted the bnjbvr/handle-dormant-read-receipts branch April 2, 2026 12:29
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.

Problems with the number of unread notifications

2 participants