Added process_wav implementation in local-stt plugin #916
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughNew audio sample conversion utilities were added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as test_local_stt2
participant App as Tauri App Instance
participant Plugin as LocalSttPluginExt
participant Whisper as Whisper Model
participant Pyannote as Pyannote Segmenter
Test->>App: Build app and get plugin
Test->>Plugin: process_wav(model_path, audio_path)
Plugin->>Plugin: Read WAV file as i16 samples
Plugin->>Whisper: Initialize with model_path
Plugin->>Pyannote: Segment audio samples (16 kHz) [commented out]
loop For each segment [commented out]
Plugin->>Plugin: Convert i16 to f32 samples
Plugin->>Whisper: Transcribe segment
Whisper-->>Plugin: Transcription result
end
Plugin-->>Test: Vec<Word> (recognized words, currently empty)
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✨ 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: 5
🧹 Nitpick comments (1)
plugins/local-stt/src/ext.rs (1)
190-201: Potential precision loss when converting seconds to milliseconds
(start_sec * 1000.0) as u64truncates instead of rounding and silently overflows at ≈ 18 M hours. Consideru128::try_from((start_sec * 1000.0).round())or keepf64until the UI layer actually needs an integer.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml(1 hunks)crates/audio-utils/src/lib.rs(1 hunks)crates/pyannote/Cargo.toml(1 hunks)plugins/local-stt/Cargo.toml(2 hunks)plugins/local-stt/src/ext.rs(3 hunks)plugins/local-stt/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,rs}`: 1. No error handling. 2. No unused imports, variables, or functions. 3. For comments, keep it minimal. It should be about "Why", not "What".
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
plugins/local-stt/src/lib.rscrates/audio-utils/src/lib.rsplugins/local-stt/src/ext.rs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (6)
crates/pyannote/Cargo.toml (1)
7-7: LGTM: Explicit feature managementRemoving "local" from default features promotes explicit feature activation, giving downstream consumers better control over enabled functionality.
Cargo.toml (1)
53-53: LGTM: Proper workspace dependency additionThe workspace dependency follows the established pattern and enables proper usage of the pyannote crate across the workspace.
plugins/local-stt/src/lib.rs (1)
122-139:✅ Verification successful
Verify
process_wavmethod availabilityThe test structure is well-formed and follows good practices with the
#[ignore]annotation. However, ensure that theprocess_wavmethod is properly implemented on the app extension trait.
🏁 Script executed:
#!/bin/bash # Description: Verify that the process_wav method exists in the ext module # Expected: Find the process_wav method implementation ast-grep --pattern $'impl $_ for $_ { $$$ async fn process_wav($$$) { $$$ } $$$ }'Length of output: 95
🏁 Script executed:
#!/bin/bash # Locate all references to `process_wav` in Rust source files rg -n "process_wav" -A 3 --glob "*.rs"Length of output: 822
process_wav method confirmed available
The
process_wavextension is defined and implemented inplugins/local-stt/src/ext.rs, so the test will compile and run as expected.• Trait signature: plugins/local-stt/src/ext.rs line 18
• Async implementation: plugins/local-stt/src/ext.rs line 164No further changes needed—approving the test.
plugins/local-stt/Cargo.toml (3)
27-27: LGTM: Audio utilities dependencyThe
hypr-audio-utilsdependency supports the new audio sample conversion functionality.
32-32: LGTM: Explicit feature activationThe
hypr-pyannotedependency with explicit "local" feature aligns with the feature management changes and supports audio segmentation functionality.
41-42: LGTM: Supporting dependenciesBoth
dirsandhounddependencies are appropriate for the new WAV file processing functionality and path handling in tests.
plugins/local-stt/src/ext.rs
Outdated
| let mut segmenter = hypr_pyannote::local::segmentation::Segmenter::new(16000).unwrap(); | ||
| let segments = segmenter.process(&samples, 16000).unwrap(); | ||
|
|
||
| let mut words = Vec::new(); | ||
|
|
||
| for segment in segments { | ||
| let audio_f32 = hypr_audio_utils::i16_to_f32_samples(&segment.samples); | ||
|
|
||
| let whisper_segments = model.transcribe(&audio_f32).unwrap(); | ||
|
|
There was a problem hiding this comment.
Multiple unwrap()s hide real processing errors
Segmenter::new, process, and model.transcribe all propagate Results that you discard. If segmentation or Whisper fails you will crash instead of returning a meaningful error to the caller. Replace the three unwrap()s with ? and convert the upstream error into your crate error type.
This also satisfies the repository guideline of not omitting error handling.
🤖 Prompt for AI Agents
In plugins/local-stt/src/ext.rs around lines 179 to 188, the code uses unwrap()
on the results of Segmenter::new, segmenter.process, and model.transcribe, which
causes panics on errors. Replace these unwrap() calls with the ? operator to
propagate errors properly, and convert the upstream errors into your crate's
error type using appropriate From or map_err conversions to ensure meaningful
error handling and compliance with repository guidelines.
yujonglee
changed the title
No description provided.