Repository navigation
perf(mobile): gate dictation setup progress polling on foreground + single-flight - #9892
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a shared 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
mobile/src/dictation/dictation-setup-poll-controller.ts (1)
64-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd "why" comments to the non-obvious eligibility/coalescing branches.
The eligibility-resume branch (Line 64-67) and the immediate-refresh coalescing branch (Line 109-113) encode important lifecycle decisions (eager reconcile on resume; defer resolving waiters until the coalesced refresh actually runs) but have no rationale comment, unlike the well-explained catch block at Line 96.
As per coding guidelines, "For non-obvious TypeScript code, add a concise comment explaining why the constraint exists, not how the code works."
Also applies to: 109-113
Source: Coding guidelines
mobile/src/dictation/use-dictation-setup-poller.ts (1)
18-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the rationale for the ref indirection and the refresh-change effect.
refreshRef(Line 18-19) exists so the memoized controller (kept stable across renders) always calls the latestrefreshwithout being recreated — worth a one-line "why" comment. Similarly, the effect at Line 27-29 only has an observable effect on re-renders whererefreshchanges while already eligible (it's a no-op on initial mount, since it runs before the visibility/foreground effect establishes eligibility) — that ordering dependency is easy to break unknowingly during a future refactor.As per coding guidelines, "For non-obvious TypeScript code, add a concise comment explaining why the constraint exists, not how the code works."
Source: Coding guidelines
mobile/app/voice-settings.tsx (2)
71-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
refresh/pollingwiring; early return bypassesfinally.This
refreshimplementation, thepollingderivation, and theuseDictationSetupPollercall are duplicated verbatim inMobileDictationSetupSheet.tsx(Line 41-62 there). Separately, theif (!client) return falseat Line 72-74 sits outside thetry/finally, so it skipssetLoading(false)— currently hard to hit given thevisibleeligibility gate, but the controller'sstate.visiblecan lag one render behindclientbecomingnull, so a manually-triggeredrefreshSetup()(Line 114, Line 131) could still land here and leave the spinner stuck.See the consolidated comment for a suggested extraction that also closes this gap in one place.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated fetch/polling/poller-wiring block into a shared hook. Both integration sites copy-paste the same
refresh/polling/useDictationSetupPollerwiring, including the same!clientearly return that sits outside any completion signaling.
mobile/app/voice-settings.tsx#L71-94: replace this block with a shareduseDictationSetupState(client)-style hook that returns{ setup, error, polling, refreshSetup }; ensure the shared implementation setssetLoading(false)/resolves completion even on the!clientpath.mobile/src/components/MobileDictationSetupSheet.tsx#L41-62: adopt the same shared hook so both sites stay in sync and the!clientpath can't leavesetup(and its spinner) stuck.mobile/src/components/MobileDictationSetupSheet.tsx (1)
41-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
refresh/pollingwiring shared withvoice-settings.tsx.This block is structurally identical to
voice-settings.tsxLine 71-94 (fetch, derivepolling, wireuseDictationSetupPoller). Here the risk of the!clientearly return (Line 42-44) is thatsetupnever gets its first value if that branch is ever reached before a successful fetch, leaving the Line 137-140 spinner stuck indefinitely.See the consolidated comment for a suggested shared extraction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cdee10ea-f421-4d1f-85e9-1a313e17fe6e
📒 Files selected for processing (5)
mobile/app/voice-settings.tsxmobile/src/components/MobileDictationSetupSheet.tsxmobile/src/dictation/dictation-setup-poll-controller.test.tsmobile/src/dictation/dictation-setup-poll-controller.tsmobile/src/dictation/use-dictation-setup-poller.ts
Co-authored-by: Orca <help@stably.ai>
…ng intent An in-flight setup read resolving 'keep polling' after an explicit setPolling(false) wrote polling=true and rescheduled, resurrecting a poll the caller had just stopped. Snapshot a pollingRevision when each read starts and only apply its result if no explicit setPolling superseded it mid-flight — so a late true can't restart a stopped poll (nor a late false cancel a restart). Co-authored-by: Orca <help@stably.ai>
b7fd229 to
790a730
Compare
…ingle-flight (stablyai#9892) * fix(mobile): gate dictation setup polling Co-authored-by: Orca <help@stably.ai> * fix(mobile): fence a stale dictation refresh against a newer setPolling intent An in-flight setup read resolving 'keep polling' after an explicit setPolling(false) wrote polling=true and rescheduled, resurrecting a poll the caller had just stopped. Snapshot a pollingRevision when each read starts and only apply its result if no explicit setPolling superseded it mid-flight — so a late true can't restart a stopped poll (nor a late false cancel a restart). Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
What
The mobile dictation setup progress polls (
voice-settings.tsxscreen andMobileDictationSetupSheet.tsx) fired a setup-state RPC every 1.5s during model download/extraction, with no AppState/focus gate and no single-flight guard — so they kept polling while backgrounded/covered, and a slow relay could overlap successive ticks (up to ~40 RPC/min per poller).Fix: a shared
DictationSetupPollController(recursive-setTimeout, single in-flight request, immediate reconciliation on resume, stops when setup leaves download/extraction) plus auseDictationSetupPollerReact/AppState adapter. The screen requires route focus + foreground; the sheet requiresvisible+ foreground; both immediately reconcile on resume.ELI5
While your phone was downloading a dictation model, it pinged the computer for progress every 1.5 seconds even if the app was in your pocket — and could stack up pings on a slow connection. Now it only polls while you're looking, never overlaps requests, and instantly refreshes when you return.
Proof of zero regression
dictation-setup-poll-controller.test.ts(hidden/unfocused/background gating, slow-response single-flight, resume coalescing, immediate foreground reconciliation, completion stop), full mobile suite 307 files / 2232 pass,typecheck/oxlint/oxfmt --checkclean.Proof of perf improvement
Eliminates the 1.5s progress poll while backgrounded/covered and prevents overlapping in-flight requests during model installation (up to ~40 RPC/min per poller saved when not foregrounded); zero cost when no setup is running.
Made with Orca 🐋