perf(mobile): avoid redundant message list sorting - #7647
Conversation
Signed-off-by: Tom Brow <tomb@block.xyz>
🔐 Codex Security Review
|
|
@codex review |
|
@builderbot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@buzz-security-review 8f6e19f |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6e19f59f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@buzz-security-review 77e6594 |
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed: 4cd82f513214aad11c2b742ce7cc7c681e8e32a0..77e6594d7ac77e4ce012bc2e4d810f852efd0d0f (exact head 77e6594d7ac77e4ce012bc2e4d810f852efd0d0f)
Risk: medium — this changes the mobile live-message timeline construction path, where ordering, retained deep-link rows, and UI-thread burst cost matter; it does not alter relay, persistence, identity, permissions, widgets, scrolling, or accessibility contracts.
Behavior/contracts traced: relay buffer delivery → channel live callback → window-store merge/dedup → authoritative chronological flatten → optional retained deep-link merge. flattenChannelWindowEvents already deduplicates and sorts with the same comparator used by _withDeepLinkEvents; the fast path is selected only when the retained map is empty, while retained targets preserve the previous merge/dedup/sort path. Buffered callbacks remain synchronous and ordered, and equal-second ordering remains deterministic by descending event ID.
Findings: no blocking or non-blocking code defects found.
Author action: none.
Verification owner: reviewer/tooling for optional native iOS release-device frame/scroll and VoiceOver observation; no author rework is required.
Validation at matching clean HEAD:
- Full touched-package test gate:
just mobile-test— PASS, 2,124 passed / 1 skipped. - Focused provider suite — PASS, 26/26.
flutter analyze— PASS; changed Dart formatting andgit diff --check— PASS.- Causal mutation: restoring the redundant
_withDeepLinkEvents(...)call makes the new production-seam regression test fail (historyIdReads: 3, allowed <=2); restoring the candidate passes. - CI at the exact head: all selected required checks are successful, including Mobile, Desktop, relay-backed Desktop integration, macOS/Windows builds, Semgrep, zizmor, DCO, and the authorized security review.
- Same-host synthetic diagnostics through the real relay buffer/notifier seam independently reproduced a substantial reduction for 2,000 history / 1,000 live events. Observed magnitude varied by run/harness (~40% to ~80%), which reinforces the README's explicit debug/JIT limitation rather than a release-device latency claim.
Manual/native evidence: no native release-device run. No widget, focus, scroll-controller, input, or semantics code changed; the full package/widget suite passed.
Residual risk: the opt-in diagnostic is synthetic debug/JIT work and excludes rendering, networking, signature verification, and other subscriptions. It proves reduced synchronous consumer work and final-count correctness, not smooth frames or production-device latency under extreme bursts. The implementation and PR description scope that claim honestly.
— :bot: Jude’s code review agent
…ush-cache-offload * commit '41c5ace93740261ee5a5d962c2a17e8d846c2c1b': perf(mobile): move profile sig checks off main thread (#7648) perf(mobile): avoid redundant message list sorting (#7647) perf(mobile): avoid reparsing unchanged Markdown (#7649) Fix desktop onboarding regressions (#7659) Deduplicate thread context by ACP session delivery (#7620) feat(mobile): hard-block under-18 users on the store age signal (#4665) Document incoming Buzz turn structure (#7624) Refresh desktop onboarding flow (#7528) Fix Apple Silicon iOS simulator builds (#7646) Signed-off-by: Tom Brow <tomb@block.xyz>
When new messages arrive in a mobile channel, the app updates the list of loaded messages on the UI thread. It currently sorts that list twice. The second sort is needed only when the app has retained messages opened through a link to a specific message.
This change uses the first sorted list when there are no retained messages to merge. It reduces work during message bursts and keeps the existing merge and ordering behavior for message links.
A controlled debug test with 2,000 loaded messages and 1,000 incoming messages reduced processing time from 446 ms to 266 ms. This measures the message update code, not screen rendering or release-device performance. All 26 focused tests pass, including a regression test that fails if the extra pass returns. Tests also cover message order and retained messages.
Repository checks and downstream test/build suites passed, apart from unrelated Rust timing failures in the full local run; each passed an isolated retry. GitHub CI passed after retrying an unrelated desktop attachment test failure.