feat(webview-accounts): faster + clearer cold opens (#1233 #1284)#1285
Merged
senamakel merged 15 commits intotinyhumansai:mainfrom May 6, 2026
Merged
feat(webview-accounts): faster + clearer cold opens (#1233 #1284)#1285senamakel merged 15 commits intotinyhumansai:mainfrom
senamakel merged 15 commits intotinyhumansai:mainfrom
Conversation
…rmup (tinyhumansai#1233) Replace the fixed 500ms `sleep` before the first CDP attach attempt with a 4-step retry schedule (0 / 50 / 150 / 400 ms) that probes immediately and escalates only on failure. The CEF prewarm at app startup typically has the target available almost instantly, so the warm path now skips the old half-second penalty entirely; the worst case before falling through to the existing 2s `ATTACH_BACKOFF` loop is ~600ms. Adds a unit test pinning the schedule shape (4 attempts, ≤600ms total, first attempt at t=0) so the budget can't drift silently. Refs tinyhumansai#1233 (embedded web apps load slowly).
…mansai#1233) Add a background prewarm path so the most-recently-active account's CEF profile and provider page can be hot before the user clicks the rail icon. The frontend dispatches `webview_account_prewarm`; Rust spawns the same cold-load codepath at a fixed off-screen 1×1 rect and attaches a CDP session that drives `Page.navigate` to the real provider URL. Load events flow through `emit_load_finished` which now short-circuits while the account is in the new `prewarm_accounts` set, so the React UI never sees a load/timeout signal for an account it hasn't asked to open. When the user later clicks the prewarmed account, `webview_account_open` hits the existing warm-reopen branch, clears the prewarm flag, resizes the on-screen rect, and emits `state:"reused"` synchronously — no cold spinner, no double event. Bookkeeping covers the full lifecycle: - `prewarm_accounts` flag cleared on warm-reopen, close, purge, and `drain_for_shutdown` so a relaunch can't suppress events for accounts prewarmed in the previous session. - Idempotent — `webview_account_prewarm` is a no-op if the webview is already in `state.inner`. - Best-effort errors only surface to the frontend; the worst case is a normal cold open later. Tests cover prewarm flag insert/remove + drain_for_shutdown clearing. The full prewarm → reuse cycle requires a CEF runtime and is verified manually. Refs tinyhumansai#1233.
…umansai#1233) Wire the new `webview_account_prewarm` Tauri command into the React side so the most-recently-active account is warmed in the background as soon as the user lands on the Accounts page. - `webviewAccountService.prewarmWebviewAccount(id, provider)` — fire-and- forget invoke; errors logged and swallowed (worst case: normal cold open later). - `Accounts.tsx` — track `lastActiveAccountId` in localStorage on every rail click / new-account pick. On mount, dispatch a single prewarm for the MRU account when `accounts.length ≤ PREWARM_MAX_ACCOUNTS = 5`, the MRU account exists, isn't already active, and isn't already warm/loading. Power users with many accounts auto-opt-out so the spawn cost stays bounded. Refs tinyhumansai#1233.
…ost (tinyhumansai#1233) Eliminate the perceived blank-screen gap when an embedded web app is still spinning up: - A non-interactive provider-branded placeholder (icon + name) is rendered unconditionally so the host area is never visually empty while the native CEF subview is still parked off-screen. The native view composites above it on reveal, so the placeholder is only visible during the loading window. - The loading spinner now shows from frame 1 (treats unknown account status as loading), not just `pending`/`loading` — closes the brief race between mount and the `setAccountStatus('pending')` dispatch inside `openWebviewAccount`. - Adds an elapsed-counter-driven phase hint that appears at 5s ("Restoring session...") and escalates at 10s ("Almost ready..."), giving the user feedback that something is happening on slow loads without feeling patronising on the happy path. Counter resets when status leaves the loading set so warm reopens (`state:"reused"`) never trigger the hint. Vitest covers the full visibility lifecycle: placeholder always visible, spinner from frame 1, hint timing at 5s/10s, hint cleared on flip to `open`, timeout overlay at `timeout`. Counter-based timer (rather than `Date.now()`) keeps the test deterministic under fake timers without needing to mock `Date`. Refs tinyhumansai#1233.
Cargo bumped the lockfile when `cargo check` ran in the worktree to match the Cargo.toml version that landed in `chore(staging): v0.53.18` (a3a1843). No dependency changes.
…inyhumansai#1233) The first cut of issue tinyhumansai#1233's loading UX rendered two absolute flex columns inside the host: a placeholder (icon + provider name, 70% opacity) and a loading overlay (icon + spinner + "Loading {Provider}…"). Each centered independently, so the loading column shifted up to make room for the spinner — the placeholder icon was visible behind it, slightly offset, looking like two ghosted versions of the brand. Collapse them into a single absolute container: - Always render the icon + provider-name pair from frame 1. - While loading, swap the name for `Loading {Provider}…` and append the spinner + 5s/10s phase hint inside the same column. - Hide the whole stack when status is `'timeout'` so the dedicated retry overlay owns the host area uncontested. Existing Vitest assertions (placeholder visible, spinner from frame 1, hint timing, hint cleared on `'open'`, timeout overlay) still pass; the testids remain on the same DOM nodes the tests query. Refs tinyhumansai#1233.
Sidebar rail tooltips rendered to the right of the icon (`left-full ml-3`), which placed them squarely inside the area occupied by the embedded CEF webview. The native subview composites above the HTML layer, so DOM z-index can't lift the tooltip — it was hidden behind the webview every time a provider account was open. Move both rail tooltips (every `RailButton` + the `Add app` `+` button) BELOW the icon via `top-full mt-1 left-1/2 -translate-x-1/2`. Below-icon keeps the tooltip near the cursor and never blocks the icon currently being hovered. Above-icon was tried first but pushed the tooltip far enough up to occlude the icon above the hovered one (gap-2 between rail buttons is only 8px; tooltip height is ~24px, so it always overflows the gap into the previous button). Refs tinyhumansai#1284. Bundled with tinyhumansai#1233 since both touch the same surface.
… order (tinyhumansai#1284) Previous tooltip-below-icon fix was still hidden behind the next sibling icon for some hovered buttons. Root cause: `hover:scale-105` on a non-active rail button establishes a CSS transform → new stacking context, which traps the tooltip's `z-50` inside the hovered button. Sibling buttons later in DOM order then paint above the tooltip rectangle even though the tooltip declared a higher z-index — z-index only ranks within the same stacking context. Add `hover:z-50` to both `RailButton` and the `Add app` `+` button so the entire button (and its tooltip child) lifts above neighbours during hover regardless of whether the transform is active. Refs tinyhumansai#1284.
Contributor
📝 WalkthroughWalkthroughThis PR implements webview prewarming with most-recently-used (MRU) account tracking across the frontend and Tauri backend. It introduces a four-step CDP session initial-attach retry schedule, adds a ChangesWebview Prewarming with MRU Account Tracking
Sequence DiagramsequenceDiagram
participant User
participant Frontend as Frontend<br/>(Accounts)
participant Service as Service<br/>(prewarm)
participant Tauri as Tauri Backend<br/>(webview_accounts)
participant CDP as CDP Session<br/>(attach)
Note over Frontend,CDP: Initial Prewarm (App Mount)
User->>Frontend: App starts / Accounts page mounts
Frontend->>Frontend: usePrewarmMostRecentAccount hook fires
Frontend->>Service: prewarmWebviewAccount(mru_id, provider)
Service->>Tauri: invoke webview_account_prewarm(PrewarmArgs)
Tauri->>Tauri: Create 1×1 offscreen webview
Tauri->>CDP: Initialize CDP session, start attach retry loop
CDP->>CDP: Attempt attach: 0ms (fail) → 50ms (fail) → ... → success
Tauri->>Tauri: Load provider URL in hidden webview
Tauri->>Tauri: Mark account as prewarmed
Note over Frontend,CDP: User Opens Prewarmed Account
User->>Frontend: Click on account
Frontend->>Tauri: invoke webview_account_open(account_id)
Tauri->>Tauri: Detect prewarm flag set
Tauri->>Tauri: Promote hidden webview to visible<br/>(restore bounds, clear prewarm mark)
Tauri->>Frontend: Return success
Frontend->>Frontend: WebviewHost shows account<br/>(placeholder + status update)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src-tauri/src/cdp/session.rs (1)
178-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe new warm-path still performs a fifth immediate attach attempt.
If all four scheduled attempts fail, execution drops straight into the steady-state
loopand callsrun_session_cycleonce more before sleepingATTACH_BACKOFF. The real cadence is therefore0, 50, 150, 400, ~0, 2000..., not the documented four-step schedule. Please insert the 2s backoff before the first steady-state retry, or otherwise track that the initial pass already consumed an attempt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/cdp/session.rs` around lines 178 - 220, The initial attach schedule currently falls through into the steady-state loop and immediately calls run_session_cycle a fifth time; update the code around INITIAL_ATTACH_SCHEDULE and the subsequent loop so that if the for loop completes without breaking (i.e., none of the scheduled attempts succeeded) you await sleep(ATTACH_BACKOFF) once before entering the steady-state loop — you can detect this by recording a boolean like initial_succeeded (set true inside the Ok branch where you break) or by checking whether the for loop broke; ensure run_session_cycle is not called again immediately when the initial pass failed.app/src-tauri/src/webview_accounts/mod.rs (1)
1694-1745:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't emit
reuseduntil the prewarmed page is actually ready.This branch treats any prewarmed webview as instantly warm, but a user can click while the off-screen prewarm is still on
about:blankor mid-navigation. In that race we clear the prewarm flag, reveal the child, and emitstate:"reused"anyway, which skips the loading overlay and can bring back the blank-gap behavior this PR is trying to remove. Please gate the fast-path on actual readiness (loaded_accounts/ non-placeholder URL) and otherwise let the normal load lifecycle complete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/webview_accounts/mod.rs` around lines 1694 - 1745, The code currently clears state.prewarm_accounts for args.account_id and immediately emits a "reused" load event even if the off-screen prewarmed WebView is still at about:blank or mid-navigation; change the fast-path to only emit state:"reused" when the WebView is actually ready (e.g. present in the loaded_accounts dedup set or existing.url() is not a placeholder/blank), otherwise avoid emitting "reused" and let the normal emit_load_finished/load lifecycle run. Concretely: after removing the prewarm flag (state.prewarm_accounts.remove) check loaded_accounts (or inspect existing.url() != "about:blank" and not a placeholder) before building reuse_url and calling app.emit("webview-account:load", {"state":"reused",...}); if the check fails, skip the emit and keep existing behavior that will allow emit_load_finished to trigger later.
🧹 Nitpick comments (1)
app/src/pages/Accounts.tsx (1)
37-58: 💤 Low valueConsider using persisted Redux slice for MRU tracking.
The coding guidelines prefer Redux over ad-hoc
localStoragefor state management. While this MRU key is a simple optimization hint (not critical state), centralizing it in a persisted Redux slice would maintain consistency and enable easier debugging/testing.That said, the current approach is reasonable for a single cross-session hint that doesn't affect app correctness — the worst case is a cold open if localStorage is unavailable.
As per coding guidelines: "Use Redux (persisted where configured) over ad-hoc
localStoragefor state management in the frontend".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Accounts.tsx` around lines 37 - 58, The MRU localStorage helpers (MRU_ACCOUNT_KEY, PREWARM_MAX_ACCOUNTS, readMruAccountId, writeMruAccountId) should be migrated to a persisted Redux slice so MRU tracking follows the app guideline; create a new slice (e.g., mruAccountSlice) with state for lastActiveAccountId and keep PREWARM_MAX_ACCOUNTS where used, replace direct calls to readMruAccountId/writeMruAccountId with a selector and an action dispatcher (and initialize the slice by reading existing localStorage once on startup to preserve backwards compatibility), and remove direct localStorage reads/writes from Accounts.tsx so the component uses the persisted Redux state instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/src/webview_accounts/mod.rs`:
- Around line 2468-2479: The prewarm path is creating a Webview without the
normal handlers and bootstrapping used by the open path (missing on_navigation,
on_new_window, on_page_load handlers, scanner bootstrap, CEF/native notification
registration, and the same CDP/Page session wiring), causing prewarmed views to
miss link interception, popup policy, load/timeout events and notifications;
modify webview_account_prewarm to reuse the exact builder/setup used by
webview_account_open by extracting the common builder+handler/bootstrap logic
into a shared function (or call the same builder routine) and only override
bounds/visibility for prewarm, ensuring the shared function wires on_navigation,
on_new_window, on_page_load, scanner bootstrap, CEF notification registration,
and CDP Page.navigate/emit_load_finished behavior so both prewarm and open get
identical configuration.
In `@app/src/components/accounts/WebviewHost.tsx`:
- Around line 80-91: The effect in WebviewHost currently calls setElapsedMs(0)
synchronously which violates the react-hooks/set-state-in-effect rule; remove
those direct setElapsedMs calls from the useEffect and instead either (A)
maintain a mutable ref (e.g., elapsedMsRef) that you reset synchronously when
isLoading/accountId change and have the interval callback update state from that
ref (only calling setElapsedMs inside the timer), or (B) gate the timer with a
derived loadSignal (computed in render from isLoading/accountId or a key change)
so the effect starts with the correct initial value without calling setElapsedMs
synchronously; update the useEffect that references setInterval/clearInterval
and the setElapsedMs usage accordingly.
---
Outside diff comments:
In `@app/src-tauri/src/cdp/session.rs`:
- Around line 178-220: The initial attach schedule currently falls through into
the steady-state loop and immediately calls run_session_cycle a fifth time;
update the code around INITIAL_ATTACH_SCHEDULE and the subsequent loop so that
if the for loop completes without breaking (i.e., none of the scheduled attempts
succeeded) you await sleep(ATTACH_BACKOFF) once before entering the steady-state
loop — you can detect this by recording a boolean like initial_succeeded (set
true inside the Ok branch where you break) or by checking whether the for loop
broke; ensure run_session_cycle is not called again immediately when the initial
pass failed.
In `@app/src-tauri/src/webview_accounts/mod.rs`:
- Around line 1694-1745: The code currently clears state.prewarm_accounts for
args.account_id and immediately emits a "reused" load event even if the
off-screen prewarmed WebView is still at about:blank or mid-navigation; change
the fast-path to only emit state:"reused" when the WebView is actually ready
(e.g. present in the loaded_accounts dedup set or existing.url() is not a
placeholder/blank), otherwise avoid emitting "reused" and let the normal
emit_load_finished/load lifecycle run. Concretely: after removing the prewarm
flag (state.prewarm_accounts.remove) check loaded_accounts (or inspect
existing.url() != "about:blank" and not a placeholder) before building reuse_url
and calling app.emit("webview-account:load", {"state":"reused",...}); if the
check fails, skip the emit and keep existing behavior that will allow
emit_load_finished to trigger later.
---
Nitpick comments:
In `@app/src/pages/Accounts.tsx`:
- Around line 37-58: The MRU localStorage helpers (MRU_ACCOUNT_KEY,
PREWARM_MAX_ACCOUNTS, readMruAccountId, writeMruAccountId) should be migrated to
a persisted Redux slice so MRU tracking follows the app guideline; create a new
slice (e.g., mruAccountSlice) with state for lastActiveAccountId and keep
PREWARM_MAX_ACCOUNTS where used, replace direct calls to
readMruAccountId/writeMruAccountId with a selector and an action dispatcher (and
initialize the slice by reading existing localStorage once on startup to
preserve backwards compatibility), and remove direct localStorage reads/writes
from Accounts.tsx so the component uses the persisted Redux state instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d24ee3a7-34cd-4a12-92fd-593c0a29a2fa
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
app/src-tauri/src/cdp/session.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/webview_accounts/mod.rsapp/src/components/accounts/WebviewHost.tsxapp/src/components/accounts/__tests__/WebviewHost.test.tsxapp/src/pages/Accounts.tsxapp/src/services/webviewAccountService.ts
…mansai#1233) CodeRabbit (PR tinyhumansai#1285): the previous `webview_account_prewarm` built its own stripped-down `WebviewBuilder` — no `on_navigation`, `on_new_window`, `on_page_load`, scanner bootstrap, or CEF notification registration. Because `webview_account_open`'s warm-reopen branch returns early on the later user click, none of those handlers ever got wired retroactively, so a prewarmed account would silently miss link interception, popup policy, timeout/load events, scanners, and native notification bridging. Fix: route prewarm through the same code path as a normal cold open behind a `prewarm: bool` flag on `OpenArgs`. The cold-spawn block forks on the flag at exactly two points — initial position+size (off-screen 1×1 vs requested rect) and `state.prewarm_accounts` membership (insert vs not). Everything else — handlers, scanners, notification register — runs identically. The warm-reopen branch short-circuits when called with `prewarm: true` against an existing webview so the prewarm command stays idempotent without flipping the flag or emitting `state:"reused"`. The promote-on-click path in the warm-reopen branch is unchanged: clear the prewarm flag, resize/show, emit `state:"reused"`. So the user experience is the same, but the prewarmed webview is now fully configured before the click ever happens. Also: lib.rs's four existing `OpenArgs { ... }` constructors get `prewarm: false` to satisfy the new mandatory field. Field is `#[serde(default)]` so frontends that omit it from JSON keep working. Refs tinyhumansai#1233. Addresses CodeRabbit Major review on PR tinyhumansai#1285.
…mansai#1233) CodeRabbit (PR tinyhumansai#1285): the initial-attach retry schedule (4 attempts at 0/50/150/400ms) drops straight into the steady-state `loop` on exhaustion, and the loop's `sleep(ATTACH_BACKOFF)` runs AFTER the cycle call — so a fully exhausted initial schedule fires a fifth back-to-back attach in <1s before any 2s backoff kicks in. Move the `sleep(ATTACH_BACKOFF)` to the top of the steady loop. Now the exhausted-initial path waits the proper 2s before its fifth attempt, and a successful session that ends cleanly also waits the backoff before reconnecting (avoiding tight-loop reconnects against a renderer that just torched). Refs tinyhumansai#1233. Addresses CodeRabbit Major review on PR tinyhumansai#1285.
…table units (tinyhumansai#1233) Three small extractions that make the prewarm flow + loading UX cover ≥80% on diff: - `app/src/utils/webviewAccountMru.ts` — `MRU_ACCOUNT_KEY`, `PREWARM_MAX_ACCOUNTS`, `readMruAccountId`, `writeMruAccountId`. Pure helpers extracted from `Accounts.tsx`. Both read + write swallow storage errors so private-mode / sandboxed environments don't break the page. - `app/src/hooks/usePrewarmMostRecentAccount.ts` — single-fire on-mount hook that wraps the MRU prewarm gating logic (no MRU id, empty accounts, > PREWARM_MAX_ACCOUNTS, MRU is active, MRU already in pending/loading/open). `Accounts.tsx` shrinks to one call site. - `WebviewHost.tsx` — phase-hint timer moved into a child `LoadingPhaseHint` component that mounts only while loading. Its elapsed counter resets purely via mount/unmount, so the parent effect no longer needs the synchronous `setElapsedMs(0)` calls CodeRabbit flagged as `react-hooks/set-state-in-effect`. The child uses a counter-based interval (no `Date.now()`), so fake-timer tests remain deterministic. No behaviour change for users — every visible UX path (placeholder, spinner from frame 1, 5s/10s phase hints, timeout overlay, MRU prewarm gating, MRU writes on rail click + new-account pick) renders identically. Refs tinyhumansai#1233. Addresses CodeRabbit Quick Win review on PR tinyhumansai#1285 + unblocks Coverage Gate via the test files added in the next commit.
…sai#1233) Backfills the diff-cover gate for PR tinyhumansai#1285 by exercising the three new testable units: - `webviewAccountMru.test.ts` (7 cases) — read/write round-trip, overwrite, empty-storage default, both error-swallow branches (`getItem` + `setItem` throwing, simulating private mode / quota). - `usePrewarmMostRecentAccount.test.tsx` (10 cases) — happy path, every skip branch (no MRU id, empty accounts, > PREWARM_MAX_ACCOUNTS, MRU not in store, MRU is active, status pending/loading/open) plus the still-prewarm path for status `timeout`/`error`. - `webviewAccountService.prewarm.test.ts` (3 cases) — `prewarmWebviewAccount` invokes the Tauri command with snake-case args, swallows errors, and is a no-op outside Tauri. 37 new test cases, all green via `pnpm test:unit` + targeted file runs. Refs tinyhumansai#1233.
`cargo check` / `cargo fmt` in the worktree refreshed the workspace lockfile to match the version that landed in `chore(staging): v0.53.18` (a3a1843). No dependency changes — sibling commit `ae4b74bf` did the same for `app/src-tauri/Cargo.lock`.
Contributor
Author
Review feedback addressed
Tip: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src-tauri/src/webview_accounts/mod.rs (2)
2240-2261:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRollback
prewarm_accountswhenadd_childfails.
prewarm_accountsis populated beforeadd_child(...), but the error path at Line 2261 returns immediately. If child creation fails, a later non-prewarm open for the same account will still hit the suppression check inemit_load_finished, even though no prewarmed webview exists. Clear the flag before returning the error.Suggested fix
- let webview = parent_window - .add_child(builder, initial_position, initial_size) - .map_err(|e| format!("add_child failed: {e}"))?; + let webview = match parent_window.add_child(builder, initial_position, initial_size) { + Ok(webview) => webview, + Err(e) => { + if args.prewarm { + state + .prewarm_accounts + .lock() + .unwrap() + .remove(&args.account_id); + } + return Err(format!("add_child failed: {e}")); + } + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/webview_accounts/mod.rs` around lines 2240 - 2261, The code inserts args.account_id into state.prewarm_accounts before calling parent_window.add_child but does not remove it if add_child returns an error; update the add_child error path in the function containing the prewarm logic so that if parent_window.add_child(...) fails you remove the same account id from state.prewarm_accounts (use the same locking/unwrapping pattern as the insert/remove calls) before returning the Err; keep references to state, prewarm_accounts, args.account_id, and parent_window.add_child and ensure emit_load_finished’s suppression won’t be triggered by a leftover entry.
1720-1767:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly fast-path to
reusedafter prewarm actually finished.This branch promotes and emits
state:"reused"for any existing prewarmed webview. If the user clicks before the background load reaches a terminal success, or after that background load already timed out, the frontend skips its normal loading/timeout path even thoughloaded_accountswas never set and the page may still be blank or on an error page. Gate the reuse fast path onloaded_accounts.contains(&args.account_id); when it is still false, just clear the prewarm flag and stashrequested_boundsso the in-flightemit_load_finishedcall can reveal normally.Suggested direction
let was_prewarmed = state .prewarm_accounts .lock() .unwrap() .remove(&args.account_id); + let prewarm_ready = state + .loaded_accounts + .lock() + .unwrap() + .contains(&args.account_id); if was_prewarmed { log::info!( "[webview-accounts] prewarm hit account={} label={} — promoting to live", args.account_id, existing_label ); } + if was_prewarmed && !prewarm_ready { + if let Some(b) = args.bounds { + state + .requested_bounds + .lock() + .unwrap() + .insert(args.account_id.clone(), b); + } + log::info!( + "[webview-accounts] prewarm still loading account={} label={} — waiting for terminal load", + args.account_id, + existing_label + ); + return Ok(existing_label); + } if let Some(b) = args.bounds { let _ = existing.set_position(LogicalPosition::new(b.x, b.y)); let _ = existing.set_size(LogicalSize::new(b.width, b.height)); state .requested_bounds🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/webview_accounts/mod.rs` around lines 1720 - 1767, The current fast-path always emits state:"reused" for prewarmed views; change it to check loaded_accounts first: after removing from state.prewarm_accounts, query state.loaded_accounts.lock().unwrap().contains(&args.account_id) and only promote/emit the "reused" event when that contains() is true; if it is false, keep the removal (clearing the prewarm flag), still stash requested_bounds (state.requested_bounds.insert(...)) and show the existing webview but do NOT call app.emit(... { "state":"reused" })—let the in-flight emit_load_finished path handle the frontend reveal normally. Ensure you reference the existing symbols: prewarm_accounts, loaded_accounts, requested_bounds, existing, and the app.emit call when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/accounts/WebviewHost.tsx`:
- Around line 217-223: The LoadingPhaseHint component's internal timer is being
reused across accounts because React reuses the component instance; to reset
elapsedMs per account, force a remount by adding a unique key based on accountId
to the LoadingPhaseHint usage (e.g., key={accountId} on the LoadingPhaseHint
element) so each account gets a fresh component instance and its timer starts
from zero.
In `@app/src/utils/webviewAccountMru.ts`:
- Around line 13-39: The MRU logic currently using MRU_ACCOUNT_KEY,
readMruAccountId and writeMruAccountId should be migrated into the persisted
Redux state: remove ad-hoc localStorage reads/writes and instead add a new
persisted field (e.g., lastActiveAccountId) in the Accounts slice, expose a
selector (e.g., selectLastActiveAccountId) and an action (e.g.,
setLastActiveAccountId) to update it; update any consumers that call
readMruAccountId/writeMruAccountId (and any prewarm logic that uses
PREWARM_MAX_ACCOUNTS) to read from the selector and dispatch the action so
prewarm decisions use the single persisted Redux source of truth.
---
Outside diff comments:
In `@app/src-tauri/src/webview_accounts/mod.rs`:
- Around line 2240-2261: The code inserts args.account_id into
state.prewarm_accounts before calling parent_window.add_child but does not
remove it if add_child returns an error; update the add_child error path in the
function containing the prewarm logic so that if parent_window.add_child(...)
fails you remove the same account id from state.prewarm_accounts (use the same
locking/unwrapping pattern as the insert/remove calls) before returning the Err;
keep references to state, prewarm_accounts, args.account_id, and
parent_window.add_child and ensure emit_load_finished’s suppression won’t be
triggered by a leftover entry.
- Around line 1720-1767: The current fast-path always emits state:"reused" for
prewarmed views; change it to check loaded_accounts first: after removing from
state.prewarm_accounts, query
state.loaded_accounts.lock().unwrap().contains(&args.account_id) and only
promote/emit the "reused" event when that contains() is true; if it is false,
keep the removal (clearing the prewarm flag), still stash requested_bounds
(state.requested_bounds.insert(...)) and show the existing webview but do NOT
call app.emit(... { "state":"reused" })—let the in-flight emit_load_finished
path handle the frontend reveal normally. Ensure you reference the existing
symbols: prewarm_accounts, loaded_accounts, requested_bounds, existing, and the
app.emit call when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33211337-86ca-4514-8425-9d34d7dfb900
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
app/src-tauri/src/cdp/session.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/webview_accounts/mod.rsapp/src/components/accounts/WebviewHost.tsxapp/src/hooks/__tests__/usePrewarmMostRecentAccount.test.tsxapp/src/hooks/usePrewarmMostRecentAccount.tsapp/src/pages/Accounts.tsxapp/src/services/__tests__/webviewAccountService.prewarm.test.tsapp/src/utils/__tests__/webviewAccountMru.test.tsapp/src/utils/webviewAccountMru.ts
tinyhumansai#1233) CodeRabbit (PR tinyhumansai#1285): React reuses the `LoadingPhaseHint` instance across rapid account switches because it sits at the same position in the WebviewHost render tree. The internal `elapsedMs` counter then carried over from the previous account, so a new account in a slow load could jump straight to "Restoring session..." or "Almost ready..." instead of starting at zero. Pass `key={accountId}` so React unmounts the hint when the account id changes, forcing a fresh mount with `elapsedMs = 0` on every new loading run. Refs tinyhumansai#1233. Addresses CodeRabbit Quick Win review on PR tinyhumansai#1285.
…lStorage helpers (tinyhumansai#1233) CodeRabbit (PR tinyhumansai#1285): the previous cut introduced a separate `localStorage` channel for the most-recently-active account id, which breaks the repo convention from CLAUDE.md ("Use Redux (persisted where configured) over ad-hoc `localStorage` for state management in the frontend") and creates a hidden state read/write the rest of Accounts doesn't see. Migrate to the existing persisted Accounts slice: - `AccountsState.lastActiveAccountId: string | null` — new field on the slice, persisted via the `lastActiveAccountId` whitelist entry on `accountsPersistConfig` in `store/index.ts`. Reuses the same redux-persist transport as `accounts` / `order` / `activeAccountId`, so the field survives sessions through the same channel. - `setLastActiveAccount` reducer + `selectLastActiveAccountId` selector exported from `accountsSlice.ts`. - `removeAccount` clears the MRU pointer when the deleted account was the last-active one (mirrors the `activeAccountId` fallback to `state.order[0]`) so the next session can't try to prewarm a gone account. - `Accounts.tsx`'s `selectAccount` + `handlePickProvider` callbacks now dispatch `setLastActiveAccount` instead of writing to `localStorage`. - `usePrewarmMostRecentAccount` reads the MRU id via `useAppSelector(selectLastActiveAccountId)`. Constants `PREWARM_MAX_ACCOUNTS` moved into the hook module so its single consumer doesn't need a separate utils file. - Removed `app/src/utils/webviewAccountMru.ts` + its tests; coverage for the helper functions is no longer needed because they don't exist. Hook tests rebased on the Redux store. Behaviour-only refactor — every user-visible flow (prewarm gate, MRU write on rail click + new-account pick, cleanup on remove) is preserved, and persistence now travels through the same channel as the rest of the slice. Refs tinyhumansai#1233. Addresses CodeRabbit Major review on PR tinyhumansai#1285.
Contributor
Author
Round 2 review feedback addressed
Tip: |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/accounts/WebviewHost.tsx (1)
102-108: ⚡ Quick winLog when this host enters the timeout state.
The new timeout/loading UX is a key state transition for this PR, but this branch only logs the retry click. A small
useEffectonisTimeoutwould make slow-open reports much easier to correlate in production logs.🪵 Minimal addition
const isLoading = status === undefined || LOADING_STATUSES.has(status); const isTimeout = status === 'timeout'; const providerName = PROVIDER_COPY[provider] ?? 'app'; + + useEffect(() => { + if (!isTimeout) return; + log('timeout overlay visible account=%s provider=%s status=%s', accountId, provider, status); + }, [accountId, isTimeout, provider, status]);As per coding guidelines: "Add substantial, development-oriented logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths; use namespaced
debuglogs in production app code."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/accounts/WebviewHost.tsx` around lines 102 - 108, In WebviewHost, add a useEffect that watches the isTimeout boolean and emits a namespaced debug/log entry when isTimeout transitions to true (and optionally when it transitions away) so slow-open events are recorded; reference the existing isTimeout and isLoading variables and include contextual identifiers (e.g., account id or status) in the log message and use the component's standard logger (e.g., debug or the app logger used elsewhere in the file) to follow the project's logging guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/components/accounts/WebviewHost.tsx`:
- Around line 102-108: In WebviewHost, add a useEffect that watches the
isTimeout boolean and emits a namespaced debug/log entry when isTimeout
transitions to true (and optionally when it transitions away) so slow-open
events are recorded; reference the existing isTimeout and isLoading variables
and include contextual identifiers (e.g., account id or status) in the log
message and use the component's standard logger (e.g., debug or the app logger
used elsewhere in the file) to follow the project's logging guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00fdbcf9-37d9-4731-81bc-71d874c8ec02
📒 Files selected for processing (7)
app/src/components/accounts/WebviewHost.tsxapp/src/hooks/__tests__/usePrewarmMostRecentAccount.test.tsxapp/src/hooks/usePrewarmMostRecentAccount.tsapp/src/pages/Accounts.tsxapp/src/store/accountsSlice.tsapp/src/store/index.tsapp/src/types/accounts.ts
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
2 participants
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
sleepbefore the first CDP attach with a tight retry schedule (0/50/150/400ms) — saves up to ~500ms wall-clock on every cold open. CEF prewarm hits commonly skip the wait entirely.webview_account_prewarmTauri command +lastActiveAccountIdMRU tracking. On Accounts page mount, the most-recently-active account is warmed in the background so the next click hits the warm-reopen branch and emitsstate:"reused"with no spinner. Capped ataccounts.length ≤ 5so power users opt out automatically.CLAUDE.md).Problem
#1233reports embedded apps (Slack, Google Meet, Telegram, ...) staying "blank or slow" before usable, with users sometimes needing to tab-switch or restart to recover. Three compounding issues on the cold-load path:WebviewHost.tsx:104only dispatchesopenWebviewAccountafter the firstResizeObservermeasure, and the spinner is gated on Redux status'pending' | 'loading'(WebviewHost.tsx:142-152). Brief but visible window between mount and spinner visibility.cdp/session.rs:166slept 500ms before the first attach attempt, then per-provider blocked navigation behind the notification-permission shim injection (webview_accounts/mod.rs:1615-1620). Up to ~500ms wall-clock cost on every cold open.lib.rs:903warmed only the CEF process via a 1×1 hiddenabout:blank. Per-account profile loaded cold every app start; the existing warm-reopen path (mod.rs:1631-1671) is fast but only fires after the first user click.The 15s timeout / retry UI shipped in #1043 already handles permanently stuck loads — this PR targets faster loads + cleaner perceived UX.
Solution
Frontend —
WebviewHost.tsx:setAccountStatus('pending')race.Datemocking needed) drives a 5s/10s phase hint that resets when status leaves the loading set, so warm reopens (state:"reused") never trigger it.Frontend —
Accounts.tsx+webviewAccountService.ts:lastActiveAccountIdinlocalStorageon every rail click / new-account pick. Failures swallowed so private mode / sandboxing never breaks the page.prewarmWebviewAccount(id, provider)helper invokeswebview_account_prewarm. Errors logged + swallowed; worst case is a normal cold open later.accounts.length ≤ PREWARM_MAX_ACCOUNTS = 5, the account exists, isn't already active, and isn't already warm/loading.Backend —
cdp/session.rs:INITIAL_ATTACH_SCHEDULE: [Duration; 4]=[0, 50, 150, 400]ms. First pass tries each in turn, then falls through to the existing 2sATTACH_BACKOFFreconnect loop. Locked under 600ms total byinitial_attach_schedule_under_600ms_totaltest.Backend —
webview_accounts/mod.rs:prewarm_accounts: Mutex<HashSet<String>>inWebviewAccountsState.webview_account_prewarm(account_id, provider)Tauri command — idempotent, off-screen 1×1 spawn at(-20000, -20000), attaches CDP session that drives the realPage.navigate.emit_load_finishedshort-circuits whileprewarm_accountscontains the id, so the React UI never sees load/timeout signals for an account it didn't open.webview_account_openclears the prewarm flag before resizing/showing/emittingstate:"reused". On user click, the prewarmed webview is promoted to live in one round-trip.drain_for_shutdownso a relaunch can't suppress events for accounts prewarmed in the previous session.Deferred follow-ups (out of scope for this PR):
feedback_cef_runtime_gaps.md(gap Refactor testing scripts in package.json and update dependencies #4 was reclassified post-fix(cef): popup paint dies after first frame — skip blank-page guard for popups (#1079) #1182, but related repaint edge cases may remain).Submission Checklist
docs/TESTING-STRATEGY.mdWebviewHostlifecycle (placeholder, spinner from frame 1, phase hints at 5s/10s, hint cleared onopen, timeout overlay). cargo-llvm-cov coversINITIAL_ATTACH_SCHEDULEshape,prewarm_accountsinsert/remove +drain_for_shutdownclearing.## Related— see above row.docs/TESTING-STRATEGY.md)Closes #NNNin the## RelatedsectionImpact
app/src/) + Tauri shell (app/src-tauri/). No CLI / mobile / web changes.accounts ≤ 5.data_directoryand CDP session pipeline as a normal open, including the existing notification-permission shim and per-origin permission grants.localStoragekeywebview-accounts:lastActiveis new; absence is handled (no prewarm, no error). No breaking changes to existing Tauri command surface.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/1233-webview-load-perf5e7d1c53(tip)Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --lib webview_accounts62/62,cargo test --lib cdp::session11/11,vitest WebviewHost7/7,vitest webviewAccountService.loadListener10/10cargo fmt --check✅,cargo check✅ (pre-existing warnings unrelated)Validation Blocked
command:cargo clippy --tests -- -D warningserror:Pre-existing failures onupstream/maininmascot_native_window,agent/pformat.rs, etc. Not introduced by this PR; unchanged files.impact:None — focused clippy on changed files (cdp/session.rs, webview_accounts/mod.rs, lib.rs) clean. Workspace-wide clippy gate is broken on main perfeedback_validation_test_target.md.Behavior Changes
Parity Contract
state:"reused"path unchanged (still bypassesemit_load_finished). 15s watchdog timeout overlay still fires on permanently stuck loads. Notification-permission shim, per-origin grants, per-account data dir all unchanged.drain_for_shutdownso cross-session state can't suppress legitimate events.Duplicate / Superseded PR Handling
Summary by CodeRabbit