Fix startup chat stacking and defer init work#425
Merged
benthecarman merged 4 commits intosledtools:masterfrom Mar 4, 2026
Merged
Fix startup chat stacking and defer init work#425benthecarman merged 4 commits intosledtools:masterfrom
benthecarman merged 4 commits intosledtools:masterfrom
Conversation
📝 WalkthroughWalkthroughDefers heavy post-login and foreground refresh work into two new internal events ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as "UI / Navigation"
participant Core as "AppCore"
participant Storage as "Local Storage"
participant Network as "Network / Profile API"
participant Push as "Push Service"
UI->>Core: Foregrounded (AppAction::Foregrounded)
Core->>Core: post InternalEvent::RefreshAfterForeground
Note over Core,Storage: when processed (if logged in & not coalesced)
Core->>Storage: refresh_from_storage
Core->>Network: refresh_my_profile
Core->>Network: refresh_follow_list
UI->>Core: start_session_with_signer (login)
Core->>Core: set deferred_session_init_pending + post InternalEvent::CompleteSessionInit
Note over Core,Storage: when processed (if logged in & not coalesced)
Core->>Storage: cache_missing_profile_pics
Core->>Network: refresh_my_profile
Core->>Core: hydrate_follow_list_from_cache
Core->>Network: refresh_follow_list
Core->>Network: publish_subscriptions
Core->>Push: register_push_device
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
During session restoration the Rust actor is busy with blocking init work while the chat list is already visible. Taps dispatch OpenChat actions that queue up and all fire when the actor finishes, stacking multiple chat screens. Fix both sides: - Rust: clear existing Chat screens from the nav stack before pushing - iOS: debounce chat list taps so only the first fires until navigation completes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
During session restore and foreground transitions, the Rust actor was blocked for the entire duration of initialization, preventing any queued user actions (like chat taps) from being processed. Split the work: critical path (auth + chat list) runs synchronously, then remaining work (profile sync, follow list, key packages, subscriptions) is deferred via internal events so pending user actions in the channel are processed first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
benthecarman
force-pushed
the
slow-startup
branch
from
9068b62 to
1b10ff2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
rust/src/core/session.rs (1)
88-92: Attach session identity to deferred init event.On Line 90–92,
CompleteSessionInitis queued without session context. If the user session changes before processing, stale deferred work can run against the current session. Consider sending a session discriminator (e.g., pubkey/revision token) and validating it in the event handler.Proposed direction
- let _ = self.core_sender.send(CoreMsg::Internal(Box::new( - InternalEvent::CompleteSessionInit, - ))); + let _ = self.core_sender.send(CoreMsg::Internal(Box::new( + InternalEvent::CompleteSessionInit { + session_pubkey_hex: pubkey_hex.clone(), + }, + )));- CompleteSessionInit, + CompleteSessionInit { session_pubkey_hex: String },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/core/session.rs` around lines 88 - 92, The deferred CompleteSessionInit event is sent without any session context (see CoreMsg::Internal and InternalEvent::CompleteSessionInit), risking stale work if the user session changes before the message is handled; update the InternalEvent enum to include a session discriminator (e.g., session_pubkey or revision token), change the send site (core_sender.send(CoreMsg::Internal(Box::new(InternalEvent::CompleteSessionInit, session_token)))) to attach the current session identifier, and in the handler for InternalEvent::CompleteSessionInit validate the attached token against the current session before applying any init work (return/skip if it does not match).rust/src/core/mod.rs (1)
4578-4582: Consider coalescing repeated foreground refresh events.Each
Foregroundeddispatch enqueuesRefreshAfterForeground; rapid foreground bursts can trigger redundantrefresh_all_from_storage/profile/follow refresh cycles. A small pending flag would collapse duplicates.♻️ Suggested coalescing pattern
struct AppCore { + refresh_after_foreground_pending: bool, ... } // in AppCore::new initializer +refresh_after_foreground_pending: false, // AppAction::Foregrounded branch - let _ = self.core_sender.send(CoreMsg::Internal(Box::new( - InternalEvent::RefreshAfterForeground, - ))); + if !self.refresh_after_foreground_pending { + self.refresh_after_foreground_pending = true; + let _ = self.core_sender.send(CoreMsg::Internal(Box::new( + InternalEvent::RefreshAfterForeground, + ))); + } // InternalEvent::RefreshAfterForeground handler InternalEvent::RefreshAfterForeground => { + self.refresh_after_foreground_pending = false; if !self.is_logged_in() { return; } self.refresh_all_from_storage(); self.refresh_my_profile(false); self.refresh_follow_list(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/core/mod.rs` around lines 4578 - 4582, Repeated Foregrounded dispatches enqueue duplicate InternalEvent::RefreshAfterForeground messages causing redundant refresh_all_from_storage/profile/follow refresh cycles; add a small pending flag on the actor (e.g., a boolean field like refresh_after_foreground_pending or an AtomicBool) and change the code that sends CoreMsg::Internal(Box::new(InternalEvent::RefreshAfterForeground)) to first check-and-set that flag so it only sends when not already pending, and ensure the flag is cleared when the consumer handles InternalEvent::RefreshAfterForeground (i.e., at the start or end of the refresh_all_from_storage/profile/follow refresh handling) so future foreground events can re-enqueue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust/src/core/mod.rs`:
- Around line 4578-4582: Repeated Foregrounded dispatches enqueue duplicate
InternalEvent::RefreshAfterForeground messages causing redundant
refresh_all_from_storage/profile/follow refresh cycles; add a small pending flag
on the actor (e.g., a boolean field like refresh_after_foreground_pending or an
AtomicBool) and change the code that sends
CoreMsg::Internal(Box::new(InternalEvent::RefreshAfterForeground)) to first
check-and-set that flag so it only sends when not already pending, and ensure
the flag is cleared when the consumer handles
InternalEvent::RefreshAfterForeground (i.e., at the start or end of the
refresh_all_from_storage/profile/follow refresh handling) so future foreground
events can re-enqueue.
In `@rust/src/core/session.rs`:
- Around line 88-92: The deferred CompleteSessionInit event is sent without any
session context (see CoreMsg::Internal and InternalEvent::CompleteSessionInit),
risking stale work if the user session changes before the message is handled;
update the InternalEvent enum to include a session discriminator (e.g.,
session_pubkey or revision token), change the send site
(core_sender.send(CoreMsg::Internal(Box::new(InternalEvent::CompleteSessionInit,
session_token)))) to attach the current session identifier, and in the handler
for InternalEvent::CompleteSessionInit validate the attached token against the
current session before applying any init work (return/skip if it does not
match).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e8e5f26-3211-4fd3-a61c-4ab106b73950
📒 Files selected for processing (3)
rust/src/core/mod.rsrust/src/core/session.rsrust/src/updates.rs
Address review feedback: retain filter now also strips orphaned GroupInfo screens when clearing stacked Chat screens. Add tests for screen dedup and for the logged-out guard on the deferred init events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rapid foreground bursts could enqueue multiple RefreshAfterForeground events, each triggering a full refresh_all_from_storage cycle. Add pending flags that collapse duplicates: the flag is set when enqueuing and cleared when the handler runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/src/core/mod.rs (1)
4578-4582: Consider coalescing foreground refresh events.Repeated foreground actions can queue multiple
RefreshAfterForegroundjobs and redo the same heavy refresh work. A simple in-flight/pending flag (similar to your subscription recompute pattern) would avoid redundant cycles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/core/mod.rs` around lines 4578 - 4582, The foreground refresh scheduling currently always enqueues CoreMsg::Internal(InternalEvent::RefreshAfterForeground) which can result in many redundant refresh jobs; add a boolean flag on the actor state (e.g., refresh_in_flight or pending_refresh) and check it before calling self.core_sender.send in the site where RefreshAfterForeground is enqueued, set the flag when you enqueue it, and clear the flag in the handler that processes InternalEvent::RefreshAfterForeground once the heavy refresh completes (mirroring your subscription recompute pattern) so duplicate refreshes are coalesced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust/src/core/mod.rs`:
- Around line 4578-4582: The foreground refresh scheduling currently always
enqueues CoreMsg::Internal(InternalEvent::RefreshAfterForeground) which can
result in many redundant refresh jobs; add a boolean flag on the actor state
(e.g., refresh_in_flight or pending_refresh) and check it before calling
self.core_sender.send in the site where RefreshAfterForeground is enqueued, set
the flag when you enqueue it, and clear the flag in the handler that processes
InternalEvent::RefreshAfterForeground once the heavy refresh completes
(mirroring your subscription recompute pattern) so duplicate refreshes are
coalesced.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/src/core/mod.rs (1)
7304-7385: Consider adding one direct coalescing test for duplicate scheduling.These tests cover route dedup and logged-out no-op well. A focused test that triggers repeated foreground/session-init scheduling and asserts collapse-to-one would lock in the coalescing contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/core/mod.rs` around lines 7304 - 7385, Add a focused test that verifies duplicate scheduling of the same internal event is coalesced: create a core via make_core, call core.handle_internal(crate::updates::InternalEvent::RefreshAfterForeground) (or ::CompleteSessionInit) twice in a row, then assert only one instance of that event is pending/scheduled (by inspecting the core state/queue that holds pending internal events) or by advancing/processing scheduled work and asserting the side-effect ran exactly once; reference core.handle_internal and the enum variants InternalEvent::RefreshAfterForeground and InternalEvent::CompleteSessionInit to locate where to add this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust/src/core/mod.rs`:
- Around line 7304-7385: Add a focused test that verifies duplicate scheduling
of the same internal event is coalesced: create a core via make_core, call
core.handle_internal(crate::updates::InternalEvent::RefreshAfterForeground) (or
::CompleteSessionInit) twice in a row, then assert only one instance of that
event is pending/scheduled (by inspecting the core state/queue that holds
pending internal events) or by advancing/processing scheduled work and asserting
the side-effect ran exactly once; reference core.handle_internal and the enum
variants InternalEvent::RefreshAfterForeground and
InternalEvent::CompleteSessionInit to locate where to add this test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c9148f8-635d-42c4-8163-1adc208223d5
📒 Files selected for processing (2)
rust/src/core/mod.rsrust/src/core/session.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reopen_mdk()runs synchronously, heavy refresh is deferredTest plan
cargo testpasses (225 unit + 31 integration)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Performance Improvements
Tests