Created UI for spoken language configuration #1144
Conversation
|
Warning Rate limit exceeded@ComputelessComputer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 45 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughA new "spoken languages" feature was added to the application's general settings, allowing users to select one or more languages they speak. This involved updating the frontend UI, backend configuration structures, localization files, and TypeScript type definitions to support the new array field, including serialization logic and translation keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsUI
participant BackendAPI
participant Database
User->>SettingsUI: Opens General Settings
SettingsUI->>SettingsUI: Loads spokenLanguages (default ["en"] or from config)
User->>SettingsUI: Adds/Removes spoken language(s)
SettingsUI->>BackendAPI: Sends updated spokenLanguages
BackendAPI->>Database: Updates ConfigGeneral.spoken_languages
Database-->>BackendAPI: Confirms update
BackendAPI-->>SettingsUI: Responds with updated config
SettingsUI-->>User: Displays updated spoken languages
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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/locales/ko/messages.po (1)
1014-1017: Korean translations needed for new spoken languages feature.The new message IDs for "Spoken Languages" and "Select the languages you speak for better transcription" are added but lack Korean translations. This is expected for new features.
Consider adding Korean translations or creating a follow-up task to complete the localization for the Korean locale.
Also applies to: 1046-1049
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/components/settings/views/general.tsx(7 hunks)apps/desktop/src/locales/en/messages.po(28 hunks)apps/desktop/src/locales/ko/messages.po(28 hunks)crates/db-user/src/config_types.rs(3 hunks)plugins/db/js/bindings.gen.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (1)
crates/db-user/src/config_types.rs (1)
crates/language/src/lib.rs (1)
iso639(254-256)
⏰ 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: ci
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (9)
crates/db-user/src/config_types.rs (3)
44-50: LGTM: Well-structured field definition with proper annotations.The
spoken_languagesfield is correctly defined with appropriate type annotations for specta, serde, and schemars. The custom serialization functions ensure proper conversion between internal Language types and external string representations.
63-63: LGTM: Consistent default value.The default initialization with English language matches the existing
display_languagedefault and provides a sensible fallback.
115-136: LGTM: Robust serialization/deserialization implementation.The custom serialization functions correctly handle the conversion between
Vec<Language>andVec<String>. The deserialization includes proper error handling and validation of ISO639 codes.plugins/db/js/bindings.gen.ts (1)
154-154: LGTM: Type definition correctly reflects backend changes.The
spoken_languages: string[]field is properly positioned and typed to match the serialized format from the Rust backend.apps/desktop/src/locales/en/messages.po (1)
1014-1017: LGTM: Complete English translations for spoken languages feature.The new translation entries "Spoken Languages" and "Select the languages you speak for better transcription" are properly added with correct English text.
Also applies to: 1046-1049
apps/desktop/src/components/settings/views/general.tsx (4)
12-28: LGTM: Appropriate imports for the new UI components.The new imports are necessary for implementing the spoken languages UI: Badge for displaying selected languages, Button for interactions, Command components for the language picker, Popover for the dropdown interface, and X icon for removal actions.
79-79: LGTM: Proper validation ensures at least one spoken language.The schema validation with
.min(1)prevents users from removing all spoken languages, which is essential for transcription functionality.
104-104: LGTM: Consistent default values and form handling.The form properly initializes with English as default, resets from backend data, and includes the field in mutations. The handling is consistent across all form lifecycle stages.
Also applies to: 116-116, 134-134
265-346: Well-implemented spoken languages UI with good UX.The implementation provides excellent user experience:
- Badge display clearly shows selected languages
- Remove functionality with visual feedback (X button)
- Prevention of removing the last language
- Command palette for adding new languages
- Proper filtering to avoid duplicates
- Immediate backend synchronization
The UI patterns follow established design conventions and handle edge cases appropriately.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/whisper-local/src/model.rs(6 hunks)crates/whisper/src/lib.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci
🔇 Additional comments (5)
crates/whisper/src/lib.rs (1)
2-3: LGTM!Adding
#[repr(u8)]ensures a compact representation and predictable memory layout. TheCopyandClonetraits are appropriate for this simple enum type.crates/whisper-local/src/model.rs (4)
67-67: Thelanguagesfield is stored but never used.The
languagesfield is set during construction but thetranscribemethod sets language toNoneinstead of using the stored languages. Is this intentional for auto-detection, or is the implementation incomplete?Also applies to: 89-89
62-62: Thetoken_sotfield is retrieved but never used.The start-of-transcription token is retrieved from the context and stored, but it's not used anywhere in the code. Is this reserved for future functionality?
Also applies to: 71-71, 93-93
114-119: LGTM!The transcription logic correctly sets up the logits filter callback and uses language auto-detection by setting language to
None.
209-230: LGTM!The callback implementation correctly suppresses the begin token and safely handles null pointers.
| impl Language { | ||
| pub fn all_indices() -> Vec<usize> { | ||
| (Language::En as usize..=Language::Yue as usize).collect() | ||
| } | ||
|
|
||
| pub fn whisper_index(self) -> usize { | ||
| self as usize | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a more robust implementation for all_indices().
The current implementation assumes enum variants are contiguous and ordered from En to Yue. This could break if variants are reordered or new ones are inserted in the middle.
Consider using strum's EnumIter trait for a more maintainable solution:
-#[derive(strum::EnumString, strum::Display, strum::AsRefStr, Copy, Clone)]
+#[derive(strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter, Copy, Clone)]
pub enum Language {Then update the implementation:
+use strum::IntoEnumIterator;
+
impl Language {
pub fn all_indices() -> Vec<usize> {
- (Language::En as usize..=Language::Yue as usize).collect()
+ Language::iter().map(|lang| lang as usize).collect()
}📝 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.
| impl Language { | |
| pub fn all_indices() -> Vec<usize> { | |
| (Language::En as usize..=Language::Yue as usize).collect() | |
| } | |
| pub fn whisper_index(self) -> usize { | |
| self as usize | |
| } | |
| } | |
| impl Language { | |
| pub fn all_indices() -> Vec<usize> { | |
| Language::iter().map(|lang| lang as usize).collect() | |
| } | |
| pub fn whisper_index(self) -> usize { | |
| self as usize | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/whisper/src/lib.rs around lines 207 to 215, the all_indices() method
assumes the Language enum variants are contiguous and ordered, which is fragile.
To fix this, derive the EnumIter trait from the strum crate for the Language
enum, then implement all_indices() by iterating over all variants using
Language::iter() and collecting their usize representations. This approach
avoids relying on variant order and is more maintainable.
The changes in this commit add support for configuring multiple spoken languages in the application's general settings. Previously, the configuration only allowed setting a single display language, but now users can select multiple languages they are comfortable with. The changes include: - Updating the `ConfigGeneral` type to include a `spoken_languages` field, which is a list of language codes. - Implementing serialization and deserialization functions for the `spoken_languages` field in the Rust codebase. - Updating the desktop application's settings UI to include a multi-select dropdown for the spoken languages. - Updating the form validation schema to include the new `spokenLanguages` field. - Ensuring the existing functionality for setting the display language is preserved. These changes will provide users with more flexibility in configuring their language preferences, which is important for a global application.
The changes in this commit add support for configuring multiple spoken languages in the application's general settings. Previously, the configuration only allowed setting a single display language, but now users can select multiple languages they are comfortable with. The changes include: - Updating the `ConfigGeneral` type to include a `spoken_languages` field, which is a list of language codes. - Implementing serialization and deserialization functions for the `spoken_languages` field in the Rust codebase. - Updating the desktop application's settings UI to include a multi-select dropdown for the spoken languages. - Updating the form validation schema to include the new `spokenLanguages` field. - Ensuring the existing functionality for setting the display language is preserved. These changes will provide users with more flexibility in configuring their language preferences, which is important for a global application.
Adds new UI components, including Badge, Button, Command, CommandEmpty, CommandGroup, CommandInput, and CommandItem, to the general settings view. These components provide a more visually appealing and user-friendly interface for configuring the application's general settings.
Adds new UI components, including Badge, Button, Command, CommandEmpty, CommandGroup, CommandInput, and CommandItem, to the general settings view. These components provide a more visually appealing and user-friendly interface for configuring the application's general settings.
ComputelessComputer
force-pushed
the
add-support-for-multiple-spoken-languages
branch
from
59dcbd0 to
81f7978
Compare
|
will continue on another PR |
Also did DB schema change as well