Add local STT model selection for Hyprnote #2468
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR refactors local STT model download logic from an embedded component implementation into a reusable hook, and adds support for local STT configuration in the onboarding flow. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
ComputelessComputer
force-pushed
the
feat/local-stt-model-selection
branch
from
7735035 to
4bb8f16
Compare
- Create useLocalSttModel.ts with shared query key factory (localSttKeys) and query options (localSttQueries) - Move useLocalModelDownload hook to shared location for reuse - Update configure.tsx (settings) to use shared hook - Update configure-notice.tsx (onboarding) to use shared queries - Remove duplicated sttModelQueries from both files This follows React Query key factory patterns for better cache management and code reuse. Co-Authored-By: yujonglee <[email protected]>
![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 (3)
apps/desktop/src/components/onboarding/configure-notice.tsx (1)
181-181: Redundant query per row when parent already fetches download status.
LocalModelRowcallsuseQuery(localSttQueries.isDownloaded(model))for each model, but the parentLocalConfigureNoticealready queries the same data (p2Downloaded,p3Downloaded). Consider passing the downloaded status as a prop to avoid duplicate queries.🔎 Proposed refactor to pass downloaded status as prop
function LocalModelRow({ model, displayName, description, isSelected, onSelect, + isDownloaded, }: { model: SupportedSttModel; displayName: string; description: string; isSelected: boolean; onSelect: () => void; + isDownloaded: boolean; }) { - const isDownloaded = useQuery(localSttQueries.isDownloaded(model)); return ( // ... use isDownloaded directly instead of isDownloaded.dataapps/desktop/src/hooks/useLocalSttModel.ts (2)
1-2: Consolidate duplicate imports.
useQueryandqueryOptionscan be imported from@tanstack/react-queryin a single import statement.🔎 Proposed fix
-import { queryOptions } from "@tanstack/react-query"; -import { useQuery } from "@tanstack/react-query"; +import { queryOptions, useQuery } from "@tanstack/react-query";
22-47: Consider the polling frequency impact.Both
isDownloadedandisDownloadingqueries userefetchInterval: 1000(1 second). When multiple models are displayed (e.g., in the settings page with 5+ model rows), this creates frequent backend calls. Consider whether a longer interval (e.g., 2-3 seconds) would suffice, or use event-driven updates instead of polling for download status.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/onboarding/configure-notice.tsxapps/desktop/src/components/settings/ai/stt/configure.tsxapps/desktop/src/components/settings/ai/stt/shared.tsxapps/desktop/src/hooks/useLocalSttModel.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/components/settings/ai/stt/shared.tsxapps/desktop/src/components/onboarding/configure-notice.tsxapps/desktop/src/hooks/useLocalSttModel.tsapps/desktop/src/components/settings/ai/stt/configure.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Never do manual state management for form/mutation. UseuseFormfrom tanstack-form anduseQuery/useMutationfrom tanstack-query for 99% cases.
Files:
apps/desktop/src/components/settings/ai/stt/shared.tsxapps/desktop/src/components/onboarding/configure-notice.tsxapps/desktop/src/hooks/useLocalSttModel.tsapps/desktop/src/components/settings/ai/stt/configure.tsx
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
apps/desktop/src/components/settings/ai/stt/shared.tsxapps/desktop/src/components/onboarding/configure-notice.tsxapps/desktop/src/hooks/useLocalSttModel.tsapps/desktop/src/components/settings/ai/stt/configure.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/settings/ai/stt/shared.tsxapps/desktop/src/components/onboarding/configure-notice.tsxapps/desktop/src/hooks/useLocalSttModel.tsapps/desktop/src/components/settings/ai/stt/configure.tsx
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/hooks/useLocalSttModel.ts
🧠 Learnings (1)
📚 Learning: 2025-12-16T07:24:36.000Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T07:24:36.000Z
Learning: Applies to **/*.{ts,tsx} : Never do manual state management for form/mutation. Use `useForm` from tanstack-form and `useQuery`/`useMutation` from tanstack-query for 99% cases.
Applied to files:
apps/desktop/src/components/onboarding/configure-notice.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/onboarding/configure-notice.tsx (4)
apps/desktop/src/components/onboarding/config.tsx (1)
getNext(17-30)apps/desktop/src/hooks/useLocalSttModel.ts (1)
localSttQueries(22-47)apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(6-41)packages/utils/src/cn.ts (1)
cn(20-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (9)
apps/desktop/src/components/onboarding/configure-notice.tsx (2)
93-97: Auto-navigation effect may cause unexpected behavior.The
useEffectnavigates away immediately if either model is already downloaded. This could be surprising UX if a user wants to download a different model than the one they already have. Consider whether this is the intended behavior, or if users should be allowed to choose even when they have a model downloaded.Additionally,
onNavigateis a callback prop that likely changes on each render. IfonNavigateis not memoized by the parent, this effect could trigger unexpectedly.
184-199: Good accessibility implementation.The interactive div correctly implements
role="button",tabIndex={0}, and keyboard event handling for Enter and Space keys. Thecnutility is used properly with array grouping for conditional classes.apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
12-19: Clean re-export pattern for backward compatibility.The alias export
localSttQueries as sttModelQueriesmaintains backward compatibility for existing consumers while centralizing the query logic in the new hook file. This follows good module organization practices.apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
5-5: Clean refactor to use shared hook.The imports are streamlined appropriately - removed
useStateanduseEffectthat were previously used for local download state management, and now imports the centralizeduseLocalModelDownloadhook. This follows the DRY principle and aligns with the PR's goal of extracting shared logic.Also applies to: 22-22, 25-25
307-317: Hook integration looks correct.The
useLocalModelDownloadhook is properly integrated, with theuseSafeSelectModelcallback passed foronDownloadComplete. This ensures the model selection only happens when the listener is inactive, preventing issues during active transcription sessions.apps/desktop/src/hooks/useLocalSttModel.ts (4)
11-20: Well-structured query key factory.The
localSttKeysfactory follows the recommended React Query key-factory pattern, providing consistent and hierarchical cache keys. This enables effective cache invalidation and query deduplication.
96-109: Good guard against duplicate downloads.The
handleDownloadfunction correctly guards against initiating a download when one is already in progress (isDownloaded.data || isDownloading.data || isStarting). The error handling for the download command result is also properly implemented.
69-87: Event listener cleanup pattern is correct.The effect properly returns a cleanup function that awaits the unlisten promise and calls the returned function. This ensures the listener is removed when the component unmounts or the model changes.
89-94: No action needed. TheonDownloadCompletecallback passed to this hook is memoized by the caller viauseCallbackinuseSafeSelectModel(), so including it in the dependency array is appropriate and won't cause excessive re-runs.
| const handleUseModel = useCallback(() => { | ||
| if (!selectedModel) return; | ||
|
|
||
| handleSelectProvider("hyprnote"); | ||
| handleSelectModel(selectedModel); | ||
| void localSttCommands.downloadModel(selectedModel); | ||
| onNavigate({ ...search, step: getNext(search) }); | ||
| }, [ | ||
| selectedModel, | ||
| search, | ||
| onNavigate, | ||
| handleSelectProvider, | ||
| handleSelectModel, | ||
| ]); |
There was a problem hiding this comment.
Download initiated but navigation happens immediately without waiting for completion.
handleUseModel calls downloadModel and then navigates to the next step immediately without awaiting the download result. This means the user is navigated away before knowing if the download started successfully. Consider either:
- Waiting for the download command to return before navigating
- Or keeping the user on this screen to show download progress (similar to the settings page behavior)
The current approach differs from HyprProviderLocalRow in configure.tsx which shows progress and doesn't navigate away.
🔎 Suggested approach to handle download result
const handleUseModel = useCallback(() => {
if (!selectedModel) return;
handleSelectProvider("hyprnote");
handleSelectModel(selectedModel);
- void localSttCommands.downloadModel(selectedModel);
- onNavigate({ ...search, step: getNext(search) });
+ void localSttCommands.downloadModel(selectedModel).then((result) => {
+ if (result.status === "ok") {
+ onNavigate({ ...search, step: getNext(search) });
+ }
+ });
}, [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleUseModel = useCallback(() => { | |
| if (!selectedModel) return; | |
| handleSelectProvider("hyprnote"); | |
| handleSelectModel(selectedModel); | |
| void localSttCommands.downloadModel(selectedModel); | |
| onNavigate({ ...search, step: getNext(search) }); | |
| }, [ | |
| selectedModel, | |
| search, | |
| onNavigate, | |
| handleSelectProvider, | |
| handleSelectModel, | |
| ]); | |
| const handleUseModel = useCallback(() => { | |
| if (!selectedModel) return; | |
| handleSelectProvider("hyprnote"); | |
| handleSelectModel(selectedModel); | |
| void localSttCommands.downloadModel(selectedModel).then((result) => { | |
| if (result.status === "ok") { | |
| onNavigate({ ...search, step: getNext(search) }); | |
| } | |
| }); | |
| }, [ | |
| selectedModel, | |
| search, | |
| onNavigate, | |
| handleSelectProvider, | |
| handleSelectModel, | |
| ]); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/configure-notice.tsx around lines 99
to 112, the handler kicks off localSttCommands.downloadModel(selectedModel) but
immediately navigates away; change the callback to an async handler that awaits
the download, sets and uses a loading/progress state, and only calls onNavigate
after a successful download (or shows an error and stays on the page on
failure). Implement try/catch around the await to surface errors (e.g., toast or
inline error state), disable UI controls while downloading, and ensure
dependencies (like localSttCommands) are included in the effect/handler closure.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const handleUseModel = useCallback(() => { | ||
| if (!selectedModel) return; | ||
|
|
||
| handleSelectProvider("hyprnote"); | ||
| handleSelectModel(selectedModel); | ||
| void localSttCommands.downloadModel(selectedModel); | ||
| onNavigate({ ...search, step: getNext(search) }); | ||
| }, [ | ||
| selectedModel, | ||
| search, | ||
| onNavigate, | ||
| handleSelectProvider, | ||
| handleSelectModel, | ||
| ]); |
There was a problem hiding this comment.
Critical bug: The download is started but errors are not handled, and navigation happens immediately without waiting for download to start successfully.
If localSttCommands.downloadModel() returns an error status (e.g., network failure, insufficient disk space), the user will:
- Navigate to the next onboarding step
- Believe the model is downloading when it actually failed
- Complete onboarding with no working STT model
The settings page (configure.tsx, old lines 401-407) properly handles this by checking the result status and setting error state.
Fix:
const handleUseModel = useCallback(async () => {
if (!selectedModel) return;
handleSelectProvider("hyprnote");
handleSelectModel(selectedModel);
const result = await localSttCommands.downloadModel(selectedModel);
if (result.status === "error") {
// Handle error - show toast/alert to user
return;
}
onNavigate({ ...search, step: getNext(search) });
}, [...]);| const handleUseModel = useCallback(() => { | |
| if (!selectedModel) return; | |
| handleSelectProvider("hyprnote"); | |
| handleSelectModel(selectedModel); | |
| void localSttCommands.downloadModel(selectedModel); | |
| onNavigate({ ...search, step: getNext(search) }); | |
| }, [ | |
| selectedModel, | |
| search, | |
| onNavigate, | |
| handleSelectProvider, | |
| handleSelectModel, | |
| ]); | |
| const handleUseModel = useCallback(async () => { | |
| if (!selectedModel) return; | |
| handleSelectProvider("hyprnote"); | |
| handleSelectModel(selectedModel); | |
| const result = await localSttCommands.downloadModel(selectedModel); | |
| if (result.status === "error") { | |
| // Handle error - show toast/alert to user | |
| return; | |
| } | |
| onNavigate({ ...search, step: getNext(search) }); | |
| }, [ | |
| selectedModel, | |
| search, | |
| onNavigate, | |
| handleSelectProvider, | |
| handleSelectModel, | |
| ]); | |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Summary
Changes
configure-notice.tsx)useLocalSttModel.tshook with:localSttKeys) following React Query best practiceslocalSttQueries) forisDownloadedandisDownloadinguseLocalModelDownloadhook for download state managementconfigure.tsx) to use the shared hooksttModelQueriesfrom shared location for backward compatibilityReview & Testing Checklist for Human
Recommended test plan:
?local=trueparam, select Parakeet v2, verify download initiatesNotes
["stt", "model", ...]to["local-stt", "model", ...]- this is a minor breaking change for any existing cached queries but should be fine since these are polling queries with 1s refetch intervalsttModelQueriesexport inshared.tsxis preserved as an alias for backward compatibilityLink to Devin run: https://app.devin.ai/sessions/988b07e83be847daa828d3bf1880c2b3
Requested by: yujonglee (@yujonglee)