Download speech-to-text model while onboarding#2468
Download speech-to-text model while onboarding#2468ComputelessComputer merged 3 commits intomainfrom
Conversation
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
📝 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
![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.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
ComputelessComputer
changed the title
✅ Deploy Preview for howto-fix-macos-audio-selection canceled.
|
ComputelessComputer
force-pushed
the
feat/local-stt-model-selection
branch
from
4a1ba02 to
ca01aac
Compare
ComputelessComputer
force-pushed
the
feat/local-stt-model-selection
branch
from
ca01aac to
d23c718
Compare
Add error state and loading indicator for model download process to provide better user feedback and prevent multiple download attempts.
1 participant
to download speech-to-text models in onboarding
onboarding.mp4
as you can see in the video, #2470 is needed as well
This is part 1 of 2 in a stack made with GitButler: