refactor(voice): load STT availability through React Query - #6224
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Adds Reviewed by Cursor Bugbot for commit f06b866. Configure here. |
Greptile SummaryThe PR moves voice-provider availability loading into a cached, cancellable React Query hook and updates speech-to-text capability gating.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported test-root leak is fixed because every created root is registered and synchronously unmounted after each test.
|
| Filename | Overview |
|---|---|
| apps/sim/hooks/queries/voice.ts | Introduces the React Query hook for STT availability with cancellation, infinite freshness, conditional enablement, and retry-on-mount. |
| apps/sim/hooks/use-speech-to-text.ts | Replaces local effect-driven availability state with the shared query while preserving browser capability and hydration gating. |
| apps/sim/hooks/queries/voice.test.tsx | Covers the query contract and now registers every created root for complete afterEach teardown. |
| apps/sim/hooks/queries/chats.test.tsx | Applies the same root-registration and teardown fix to the existing chat query tests. |
Reviews (3): Last reviewed commit: "fix(voice): let a failed STT probe recov..." | Re-trigger Greptile
384ab77 to
b01eab8
Compare
useSpeechToText fetched `/api/settings/voice` inside an effect and stored the result in useState behind a hand-rolled mountedRef guard: no cache, no dedupe across mounts, and no AbortSignal, so the response was fetched and parsed even after unmount. Two simultaneously mounted consumers issued two requests. It also bypassed hooks/queries/**, which is where every other server read in the app lives — and it escaped `check:react-query`, whose audit only covers useQuery/useMutation call sites. The value is server env read at request time, so it cannot change within a session; the new hook uses an infinite staleTime and a caller-controlled `enabled` so clients without the audio APIs never issue the request. Hydration is unchanged: SSR renders unavailable, and the first client render still resolves unavailable because `data` is undefined until the fetch settles. No initialData, deliberately — adding it would break that. mountedRef stays; it is still load-bearing for the streaming lifecycle.
renderHookWithClient created a React root per test but never tore it down, so trees stayed mounted with live QueryClient observers until worker teardown and async notifications could cross test boundaries. Audited every test in the repo using createRoot: 51 of 53 already unmount. The two that did not were both mine — voice.test.tsx here and chats.test.tsx from #6223 — so both are fixed and the pattern is now uniform.
b01eab8 to
7af7b83
Compare
|
@cursor review |
The app QueryClient sets retryOnMount: false and retry: 1, and refetchOnWindowFocus only refetches stale queries — which an infinite staleTime never becomes. So one transient failure cached the error for the life of the client and hid the mic until a full page reload. The effect this replaced refetched on every run, so retryOnMount: true restores parity: no refetch after success, a retry per mount after failure. Test asserts recovery under the app's real query defaults and fails without the override.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f06b866. Configure here.
Summary
useSpeechToTextfetched/api/settings/voiceinside auseEffectand stored the result inuseStatebehind a hand-rolledmountedRefguard.Concretely that meant:
AbortSignalwas passed, so the response was fetched and parsed even after unmount; the guard only suppressed thesetState.hooks/queries/**, where every other server read in the app lives. It also escapedbun run check:react-query, whose audit only coversuseQuery/useMutationcall sites — so nothing flagged it.Adds
hooks/queries/voice.tswith a key factory and an infinitestaleTime(the value is server env read at request time, so it cannot change within a session), and a caller-controlledenabledso clients lackingAudioContext/WebSocket/getUserMedianever issue the request at all.Hydration
Unchanged, and deliberately so. SSR renders unavailable; the first client render also resolves unavailable because
dataisundefineduntil the fetch settles, so the markup matches. NoinitialData— adding it would break that guarantee. A failed request also still reads as unavailable rather than throwing, matching the previous.catch.mountedRefstays — it is still load-bearing for the streaming lifecycle (WebSocket, MediaStream, AudioContext teardown).Blast radius
The only consumer is the workspace home input (
app/workspace/[workspaceId]/home/components/user-input/user-input.tsx), which gates the mic button onisSupported. Nothing on the chat surface uses this hook any more.Type of Change
Testing
New
hooks/queries/voice.test.tsxcovers availability, the disabled gate, failure reading as unavailable, and dedupe across simultaneous consumers. Theenabled-gate test was verified to fail when the option is removed. 1058 tests pass acrosshooks; typecheck, lint andcheck:react-queryall clean.Not exercised in a live browser — worth confirming the mic button still appears on the workspace home input.
Checklist