refactor(gladia): clean up adapter code #2121
Conversation
- Extract shared URL building helper (build_url_with_scheme) - Add SessionChannels struct to encapsulate session channel tracking - Simplify error handling with map_err().ok()? pattern - Use .then() for conditional Option creation - Add Default derive to structs for unwrap_or_default() - Remove duplicate code in build_ws_url_with_api_key Co-Authored-By: yujonglee <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis pull request refactors the Gladia adapter module to improve code clarity and maintainability. Changes include introducing a SessionChannels struct for session-to-channels mapping management, adding Default trait derivations for cleaner deserialization handling, simplifying conditional expressions throughout, and centralizing HTTP URL construction logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
owhisper/owhisper-client/src/adapter/gladia/live.rs (2)
21-25: Consider logging lock acquisition failures.The
insertmethod silently ignores lock acquisition failures, which could lead to missing session data and confusing behavior downstream. Consider adding a warning log when the lock fails.fn insert(session_id: String, channels: u8) { - if let Ok(mut map) = Self::store().lock() { - map.insert(session_id, channels); + match Self::store().lock() { + Ok(mut map) => { + map.insert(session_id, channels); + } + Err(e) => { + tracing::warn!(session_id = %session_id, error = ?e, "failed_to_lock_session_channels_for_insert"); + } } }
27-39: Lock failure returnsNone, which is ambiguous with "not found".The
getandremovemethods returnNoneon both lock failure and missing entries. While this simplifies the API, it could mask concurrency issues. Consider logging lock failures to aid debugging.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
owhisper/owhisper-client/src/adapter/gladia/batch.rs(4 hunks)owhisper/owhisper-client/src/adapter/gladia/live.rs(5 hunks)owhisper/owhisper-client/src/adapter/gladia/mod.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
owhisper/owhisper-client/src/adapter/gladia/live.rs (1)
owhisper/owhisper-client/src/adapter/gladia/mod.rs (1)
build_http_url(39-46)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: Devin
🔇 Additional comments (12)
owhisper/owhisper-client/src/adapter/gladia/batch.rs (3)
76-76: LGTM: Default implementations improve deserialization resilience.Adding
Defaultto these structs enables cleaner fallback handling when API responses are missing optional fields.Also applies to: 90-90
188-191: LGTM: Idiomatic conditional construction.The use of
.then()and.then_some()is more concise and idiomatic than explicit if/else blocks.
265-266: LGTM: Cleaner fallback handling.Using
unwrap_or_default()is more concise than explicit struct construction and leverages theDefaultimplementations added above.owhisper/owhisper-client/src/adapter/gladia/mod.rs (4)
48-64: LGTM: Centralized URL construction.The
build_url_with_schemefunction effectively consolidates scheme determination logic, reducing duplication and improving maintainability.
66-76: LGTM: Improved organization of default URLs.Extracting default URL construction into dedicated functions improves code organization and provides a single source of truth.
83-83: LGTM: Simplified URL parsing.Direct parsing is cleaner and more straightforward.
39-46: TheWS_PATHconstant is intentionally used for both WebSocket and HTTP endpoints. The Gladia API uses the/v2/livepath for both connection types, as confirmed by thedefault_http_url()function on line 73, which also usesWS_PATH. Thebuild_url_with_schemefunction correctly handles this by varying only the scheme (http/https vs ws/wss) based on theuse_wsparameter, while the path remains consistent. No separate HTTP path constant is needed.Likely an incorrect or invalid review comment.
owhisper/owhisper-client/src/adapter/gladia/live.rs (5)
41-43: LGTM: Sensible fallback strategy.The
get_or_infermethod provides a reasonable fallback when session data is missing, improving resilience.
87-97: LGTM: Clean usage of helpers and idiomatic conditionals.The code effectively uses the new
build_http_urlhelper and employs idiomatic.then()for conditional construction, matching the improvements in batch.rs.
111-131: LGTM: Cleaner error handling pattern.The refactored error handling using
map_errand.ok()?is more concise while maintaining error logging. ReturningNoneallows initialization to fail gracefully.
134-134: LGTM: Clean usage of SessionChannels API.Using
SessionChannels::insertis cleaner than direct map manipulation.
176-179: LGTM: Graceful fallback handling.Both
SessionChannels::removewith fallback andget_or_inferprovide resilient error handling with appropriate logging.Also applies to: 365-365
refactor(gladia): clean up adapter code
Summary
Refactors the Gladia STT adapter code to improve readability and reduce duplication:
build_url_with_schemehelper to eliminate duplicate scheme/host/port logic between WebSocket and HTTP URL constructionSessionChannelsstruct with clear helper methods (insert,get,remove,get_or_infer)map_err().ok()?pattern.then()and.then_some()for cleaner conditional Option creationDefaultto response structs to enableunwrap_or_default()Net reduction of ~12 lines while improving code organization.
Review & Testing Checklist for Human
GLADIA_API_KEY=00239f35-f300-45c5-8fe0-dc5eb4402712 cargo test -p owhisper-client -- --ignored gladiato verify both single and dual channel transcription workSessionChannelsrefactor manages channel count tracking - confirm dual-channel transcription correctly identifies speaker channelsbuild_http_urlproduces correct URLs (e.g.,https://api.gladia.io/v2/livefor empty base, correct scheme for localhost)Notes
build_http_urlfunction usesWS_PATH(/v2/live) which is intentional - Gladia's HTTP endpoint for initializing live sessions shares the same path as the WebSocket endpoint