Show toasts based on local stt connection status #2332
Conversation
📝 WalkthroughWalkthroughPropagates local STT connection and server status into the sidebar banner system, adds two STT-specific banners (loading/unreachable) and refines missing-STT conditions for local models, and introduces an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
✅ 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. |
![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 (1)
apps/desktop/src/components/main/sidebar/banner/index.tsx (1)
100-101: Potential unnecessary re-renders from unused dependency.
sttServerStatusis included in theuseMemodependency array, but it's not actually used in any banner logic withincreateBannerRegistry(as noted in the registry.tsx review). This means the registry will be recreated wheneversttServerStatuschanges, even though it has no effect on which banner is shown.Consider one of the following:
- Remove from dependencies until it's actually used in banner logic:
[ isAuthenticated, hasLLMConfigured, hasSttConfigured, - sttServerStatus, isLocalSttModel, isAiTranscriptionTabActive, isAiIntelligenceTabActive, handleSignIn, handleOpenLLMSettings, handleOpenSTTSettings, ],
- Add a banner that uses
sttServerStatusto display connection errors (see suggestions in registry.tsx review), which would justify its inclusion here.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/main/sidebar/banner/component.tsx(1 hunks)apps/desktop/src/components/main/sidebar/banner/index.tsx(4 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(4 hunks)apps/desktop/src/components/main/sidebar/banner/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/components/main/sidebar/banner/types.ts
**/*.{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 instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/sidebar/banner/types.tsapps/desktop/src/components/main/sidebar/banner/index.tsxapps/desktop/src/components/main/sidebar/banner/component.tsxapps/desktop/src/components/main/sidebar/banner/registry.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/main/sidebar/banner/index.tsx (1)
apps/desktop/src/hooks/useSTTConnection.ts (1)
useSTTConnection(13-128)
⏰ 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). (3)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
🔇 Additional comments (4)
apps/desktop/src/components/main/sidebar/banner/types.ts (1)
16-16: LGTM! Clean type extension.The optional
variantfield is well-designed with an appropriate union type and maintains backward compatibility with existing banners that don't specify a variant.apps/desktop/src/components/main/sidebar/banner/component.tsx (1)
20-23: LGTM! Proper conditional styling implementation.The variant-based styling is correctly implemented using
cnwith an array as specified in the coding guidelines. The error variant provides clear visual distinction with red border and shadow.apps/desktop/src/components/main/sidebar/banner/index.tsx (1)
38-48: LGTM! Well-structured STT connection logic.The differentiation between local and non-local STT models is clear and correct:
- Local models (hyprnote, non-cloud) only need a connection
- Other models need provider, model, AND connection
This properly handles the different configuration requirements for each model type.
apps/desktop/src/components/main/sidebar/banner/registry.tsx (1)
52-53: Banner correctly suppressed for local STT; connection status shown via Listen button tooltip.The condition does suppress the "missing-stt" banner when a local STT model is selected (
isLocalSttModel=true), even if the server isn't running (hasSttConfigured=falsedue tosttConnectionbeing null).However, users are notified through other UI elements:
- Listen button tooltip (in
useListenButtonState): Shows "Transcription model not available." when!sttConnection, with an actionable "Configure" button- Settings health indicator: Displays explicit server status ("Local server status: unavailable/loading/ready")
Note: The
sttServerStatusparameter is passed tocreateBannerRegistrybut never used in any banner condition—it appears to be dead code.This appears intentional: the banner system handles initial setup (no model selected), while the Listen button provides real-time feedback about connection issues for configured models.
ComputelessComputer
force-pushed
the
feat/add-banner-error-variant
branch
from
97c3485 to
f140090
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/sidebar/banner/registry.tsx(5 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/main/sidebar/banner/registry.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/select.tsxapps/desktop/src/components/main/sidebar/banner/registry.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/select.tsxapps/desktop/src/components/main/sidebar/banner/registry.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/select.tsxapps/desktop/src/components/main/sidebar/banner/registry.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
apps/desktop/src/hooks/useSTTConnection.ts (1)
useSTTConnection(13-122)
⏰ 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). (3)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (4)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
198-214: LGTM!The conditional error messages correctly handle mutually exclusive states -
isNotConfiguredrequires missing provider/model whileisUnreachablerequires them to be set. The targeted messaging improves UX by distinguishing configuration issues from server failures.apps/desktop/src/components/main/sidebar/banner/registry.tsx (3)
37-60: LGTM!The loading banner appropriately shows during STT server startup with clear visual feedback via the animated text.
61-83: LGTM!The unreachable banner correctly uses the new error variant and provides actionable guidance to configure transcription.
55-100: Potential gap whensttServerStatusis undefined.When
isLocalSttModelis true butsttServerStatusisundefined(e.g., query still loading or returnednull), no banner displays:
stt-loadingrequiressttServerStatus === "loading"stt-unreachablerequiressttServerStatus === "unreachable"missing-sttrequires!isLocalSttModelThis leaves a brief window where users see no feedback while the status query initializes.
Consider whether this gap is acceptable UX, or if you'd prefer a fallback:
condition: () => - !hasSttConfigured && !isLocalSttModel && !isAiTranscriptionTabActive, + !hasSttConfigured && + (!isLocalSttModel || sttServerStatus === undefined) && + !isAiTranscriptionTabActive,This would show the generic missing-stt banner while status is unknown, then switch to the specific loading/unreachable banner once status resolves.
ComputelessComputer
force-pushed
the
feat/add-banner-error-variant
branch
from
8e92637 to
c6a264a
Compare
ComputelessComputer
changed the title
Overview