-
Notifications
You must be signed in to change notification settings - Fork 206
listening autodetect #1155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
listening autodetect #1155
Conversation
📝 WalkthroughWalkthroughThis change introduces a new Tauri plugin, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant TauriPlugin (meeting-automation)
participant AutomationLogic
participant Detector
participant Database
participant Notification
participant Recorder
User->>Frontend: Triggers "Start Meeting Automation"
Frontend->>TauriPlugin: invoke startMeetingAutomation()
TauriPlugin->>AutomationLogic: start()
AutomationLogic->>Detector: start detection loops
loop Detection Events
Detector-->>AutomationLogic: Meeting app launched/terminated or mic activity
AutomationLogic->>Notification: Show notification (if enabled)
AutomationLogic->>Recorder: Start/stop recording session (if triggered)
AutomationLogic->>Frontend: Emit event (e.g., recording_auto_started)
end
AutomationLogic->>Database: Query for upcoming scheduled meetings (periodically)
Database-->>AutomationLogic: Return events in range
AutomationLogic->>Notification: Show pre-meeting notification (if enabled)
AutomationLogic->>Recorder: Start recording if meeting imminent
User->>Frontend: Triggers "Stop Meeting Automation"
Frontend->>TauriPlugin: invoke stopMeetingAutomation()
TauriPlugin->>AutomationLogic: stop()
AutomationLogic->>Detector: stop detection loops
AutomationLogic->>Recorder: Stop any active recordings
sequenceDiagram
participant Frontend
participant TauriPlugin (meeting-automation)
participant ConfigManager
Frontend->>TauriPlugin: invoke configureAutomation(config)
TauriPlugin->>ConfigManager: save_config(config)
ConfigManager-->>TauriPlugin: Result
TauriPlugin-->>Frontend: Success/Error
Frontend->>TauriPlugin: invoke getAutomationConfig()
TauriPlugin->>ConfigManager: get_config()
ConfigManager-->>TauriPlugin: AutomationConfig
TauriPlugin-->>Frontend: AutomationConfig
sequenceDiagram
participant Detector
participant AutomationLogic
participant Frontend
Detector-->>AutomationLogic: App/mic event detected
AutomationLogic->>Frontend: Emit event (e.g., meeting_notification, recording_auto_started)
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
✨ 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (8)
plugins/listener/js/bindings.gen.ts (1)
1-4
: Avoid manual formatting changes to auto-generated filesThis file is auto-generated by tauri-specta (as noted in line 3). Manual formatting changes may be lost when the file is regenerated. Consider updating the generator configuration instead to ensure consistent formatting.
crates/detect/src/app/macos.rs (1)
61-65
: Consider using an enum for event types instead of string prefixesWhile the implementation correctly distinguishes between launch and terminate events, using string prefixes like "app_launched:" and "app_terminated:" could be error-prone. Consider defining an enum or structured event type.
Example approach:
enum AppEvent { Launched(String), Terminated(String), } // Then serialize to a consistent format when calling the callbackplugins/meeting-automation/src/lib.rs (2)
33-45
: Consider making the runtime type generic.The function hardcodes
tauri::Wry
as the runtime type in the command collection. This could limit flexibility if the plugin needs to support different runtime implementations.-fn make_specta_builder<R: tauri::Runtime>() -> tauri_specta::Builder<R> { +fn make_specta_builder<R: tauri::Runtime>() -> tauri_specta::Builder<R> +where + R: tauri::Runtime + 'static, +{ tauri_specta::Builder::<R>::new() .plugin_name(PLUGIN_NAME) .commands(tauri_specta::collect_commands![ - commands::start_meeting_automation::<tauri::Wry>, - commands::stop_meeting_automation::<tauri::Wry>, - commands::get_automation_status::<tauri::Wry>, - commands::configure_automation::<tauri::Wry>, - commands::get_automation_config::<tauri::Wry>, - commands::test_meeting_detection::<tauri::Wry>, + commands::start_meeting_automation::<R>, + commands::stop_meeting_automation::<R>, + commands::get_automation_status::<R>, + commands::configure_automation::<R>, + commands::get_automation_config::<R>, + commands::test_meeting_detection::<R>, ]) .error_handling(tauri_specta::ErrorHandlingMode::Throw) }
69-80
: Add assertion to verify export success.The test generates TypeScript bindings but doesn't verify the operation succeeded.
#[test] fn export_types() { - make_specta_builder::<tauri::Wry>() + let result = make_specta_builder::<tauri::Wry>() .export( specta_typescript::Typescript::default() .header("// @ts-nocheck\n\n") .formatter(specta_typescript::formatter::prettier) .bigint(specta_typescript::BigIntExportBehavior::Number), "./js/bindings.gen.ts", - ) - .unwrap() + ); + assert!(result.is_ok(), "Failed to export TypeScript types"); }plugins/meeting-automation/src/automation.rs (1)
382-409
: Document platform-specific behavior.Window focus detection is only implemented for macOS. Consider documenting this limitation in the plugin documentation and configuration.
Add a comment at the function level:
+ /// Gets the bundle identifier of the currently focused application. + /// + /// # Platform Support + /// - macOS: Fully supported using AppleScript + /// - Windows/Linux: Not implemented, returns an error async fn get_focused_app() -> Result<String> {plugins/meeting-automation/src/ext.rs (1)
90-107
: Release mutex before blocking operation.Similar to
start_meeting_automation
, this holds a mutex while blocking.fn get_automation_config(&self) -> Result<AutomationConfig> { let config_manager = { let state = self.state::<ManagedState<R>>(); let state = state.lock().unwrap(); state .config_manager .as_ref() .ok_or_else(|| { crate::Error::ConfigurationError("Config manager not initialized".to_string()) })? .clone() - }; + }?; tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async { config_manager.get_config().await }) }) }plugins/meeting-automation/src/storage.rs (2)
31-33
: Verify directory path before creation.The code creates directories without checking if the path already exists as a file.
-std::fs::create_dir_all(&app_data_dir).map_err(|e| { - crate::Error::ConfigurationError(format!("Failed to create config directory: {}", e)) -})?; +if app_data_dir.exists() && !app_data_dir.is_dir() { + return Err(crate::Error::ConfigurationError( + format!("Config path exists but is not a directory: {}", app_data_dir.display()) + )); +} +std::fs::create_dir_all(&app_data_dir).map_err(|e| { + crate::Error::ConfigurationError(format!("Failed to create config directory: {}", e)) +})?;
113-128
: Potential race condition when loading configuration.Dropping the read lock before acquiring the write lock could allow multiple threads to simultaneously load the configuration from disk.
Consider using a double-checked locking pattern or accepting the race condition with a comment:
pub async fn get_config(&self) -> Result<AutomationConfig> { let cached = self.cached_config.read().await; if let Some(config) = cached.as_ref() { debug!("Returning cached config"); return Ok(config.clone()); } drop(cached); + // Note: Multiple tasks might load config simultaneously here. + // This is acceptable as they'll all load the same data. let config = self.storage.load_config()?; let mut cached = self.cached_config.write().await; *cached = Some(config.clone()); Ok(config) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lock
is excluded by!**/*.lock
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
Cargo.toml
(1 hunks)apps/desktop/package.json
(1 hunks)apps/desktop/src-tauri/Cargo.toml
(1 hunks)apps/desktop/src-tauri/src/lib.rs
(1 hunks)apps/desktop/src/locales/en/messages.po
(18 hunks)apps/desktop/src/locales/ko/messages.po
(18 hunks)crates/db-user/src/events_ops.rs
(1 hunks)crates/detect/src/app/macos.rs
(2 hunks)crates/detect/src/lib.rs
(2 hunks)crates/detect/src/mic/macos.rs
(5 hunks)plugins/listener/js/bindings.gen.ts
(1 hunks)plugins/meeting-automation/Cargo.toml
(1 hunks)plugins/meeting-automation/build.rs
(1 hunks)plugins/meeting-automation/js/bindings.gen.ts
(1 hunks)plugins/meeting-automation/js/index.ts
(1 hunks)plugins/meeting-automation/package.json
(1 hunks)plugins/meeting-automation/src/automation.rs
(1 hunks)plugins/meeting-automation/src/commands.rs
(1 hunks)plugins/meeting-automation/src/config.rs
(1 hunks)plugins/meeting-automation/src/error.rs
(1 hunks)plugins/meeting-automation/src/ext.rs
(1 hunks)plugins/meeting-automation/src/lib.rs
(1 hunks)plugins/meeting-automation/src/storage.rs
(1 hunks)plugins/meeting-automation/tsconfig.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (2)
plugins/listener/js/bindings.gen.ts (2)
plugins/local-stt/js/bindings.gen.ts (1)
events
(54-58)plugins/windows/js/bindings.gen.ts (1)
events
(57-65)
plugins/meeting-automation/src/commands.rs (2)
plugins/meeting-automation/js/bindings.gen.ts (1)
AutomationConfig
(47-57)plugins/meeting-automation/src/ext.rs (12)
start_meeting_automation
(7-7)start_meeting_automation
(16-45)stop_meeting_automation
(8-8)stop_meeting_automation
(47-58)get_automation_status
(9-9)get_automation_status
(60-65)configure_automation
(10-10)configure_automation
(67-88)get_automation_config
(11-11)get_automation_config
(90-107)test_meeting_detection
(12-12)test_meeting_detection
(109-140)
🔇 Additional comments (19)
apps/desktop/src/locales/ko/messages.po (1)
254-262
: LGTM: Localization metadata updates reflect code reorganization.The line number updates are expected when source code is reorganized during plugin integration. The added comment at line 254 provides helpful context for the placeholder condition.
apps/desktop/package.json (1)
38-38
: LGTM: Workspace dependency addition follows established pattern.The meeting automation plugin dependency is correctly added and follows the same workspace pattern as other plugins in the project.
plugins/meeting-automation/package.json (1)
1-6
: LGTM: Standard package.json structure for TypeScript plugin.The minimal package.json follows standard conventions. Having both
main
andtypes
point to the same TypeScript file is appropriate for this plugin structure.apps/desktop/src-tauri/src/lib.rs (1)
98-98
: LGTM: Plugin registration follows Tauri best practices.The meeting automation plugin is correctly integrated into the Tauri plugin builder chain using the standard initialization pattern.
plugins/meeting-automation/build.rs (1)
1-3
: LGTM: Standard build script pattern.The build script correctly instructs Cargo to rerun when source files change, which is essential for plugin development and follows standard Rust practices.
Cargo.toml (1)
95-95
: LGTM! Plugin dependency addition follows established patterns.The workspace dependency is correctly added and follows the same pattern as other plugin dependencies in the file.
apps/desktop/src-tauri/Cargo.toml (1)
40-40
: LGTM! Plugin dependency correctly added.The dependency addition follows the established pattern and properly references the workspace dependency.
plugins/meeting-automation/tsconfig.json (1)
1-9
: LGTM! Standard TypeScript configuration.The TypeScript configuration is well-structured and follows standard conventions for a plugin with TypeScript bindings.
apps/desktop/src/locales/en/messages.po (1)
254-262
: LGTM! Localization metadata updates are correctly maintained.The line number updates and added placeholder comments properly maintain translation references after code changes.
crates/detect/src/lib.rs (1)
28-31
: LGTM! Detector fields reactivated correctlyThe
app_detector
andbrowser_detector
fields and their corresponding lifecycle methods follow the same pattern asmic_detector
. The implementation is consistent and properly integrated.Also applies to: 66-68, 72-74
plugins/meeting-automation/js/index.ts (1)
1-17
: LGTM! Clean event interface definitionThe module properly re-exports generated bindings and defines a clear event interface for meeting automation events. The event types and their properties are well-structured.
crates/detect/src/app/macos.rs (1)
7-28
: Comprehensive app coverage for meeting detectionGood expansion of the meeting app list to cover popular communication and meeting platforms. The additions include major platforms like Chrome, Safari, Discord, Slack, and various meeting-specific applications.
crates/detect/src/mic/macos.rs (2)
31-31
: Excellent implementation of microphone state trackingThe state tracking mechanism is well-designed:
- Thread-safe state management using
Arc<Mutex<bool>>
- Clean state transition detection logic
- Proper emission of both start and stop events
- Guards against race conditions with mutex locks
This enhancement enables accurate microphone lifecycle tracking for the meeting automation feature.
Also applies to: 35-35, 48-48, 52-52, 65-82
141-145
: Consistent tuple type updates for the new state fieldThe tuple types in Box allocations and unsafe casts have been correctly updated to include the new
mic_state
field, maintaining type safety throughout the callback system.Also applies to: 209-213
plugins/meeting-automation/src/error.rs (1)
1-35
: Well-structured error handling module!The error types comprehensively cover the various failure scenarios in the meeting automation plugin. Good use of the
thiserror
crate for automatic error trait implementation.plugins/meeting-automation/Cargo.toml (1)
1-27
: LGTM!The Cargo manifest is properly configured with appropriate dependencies for the meeting automation plugin.
plugins/meeting-automation/js/bindings.gen.ts (1)
1-126
: Auto-generated bindings look correct.The TypeScript bindings properly reflect the Rust command interface. Since this is generated code, any issues should be addressed in the source Rust files.
plugins/meeting-automation/src/ext.rs (1)
47-58
: LGTM!The stop method correctly takes ownership of the automation before stopping it, preventing any race conditions.
plugins/meeting-automation/src/config.rs (1)
1-189
: Well-structured configuration module!The configuration types are properly organized with sensible defaults, comprehensive app support, and efficient helper methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/settings/views/notifications.tsx
(2 hunks)apps/desktop/src/locales/en/messages.po
(23 hunks)apps/desktop/src/locales/ko/messages.po
(23 hunks)apps/desktop/src/routes/app.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/locales/ko/messages.po
- apps/desktop/src/locales/en/messages.po
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (1)
apps/desktop/src/routes/app.tsx (1)
packages/ui/src/components/ui/toast.tsx (1)
sonnerToast
(6-6)
@coderabbitai review |
✅ Actions performedReview triggered.
|
No description provided.