Update Rust dependencies and remove deprecated packages#59
Conversation
📝 WalkthroughWalkthroughEdition/toolchain and many dependency versions bumped; reqwest-compat added; OpenAI chat/tool usage migrated to new chat types; redis AsyncCommands imports moved to deadpool_redis; numerous lazy_static globals replaced with std::sync::LazyLock; small stylistic tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App
participant OpenAI as async_openai
participant Parser as JSON_Parser
participant Redis as deadpool_redis
Note over Client,App: Recommendation request using new chat API with ChatCompletionTool
Client->>App: request recommendation
App->>OpenAI: create_chat_completion(request with ChatCompletionTool)
alt OpenAI returns function call
OpenAI-->>App: message (tool call)
App->>Parser: parse tool_call.function.arguments (JSON)
Parser-->>App: parsed payload
App->>Redis: enqueue/store recommendations
Redis-->>App: ack
App-->>Client: return recommendations
else OpenAI returns non-function
OpenAI-->>App: message (non-function)
App-->>Client: return error / bail
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-01-01T17:47:59.430ZApplied to files:
🔇 Additional comments (4)
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: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
Cargo.tomlsrc/app.rssrc/queue/profanity_check.rssrc/rickroll.rssrc/services/magic.rssrc/services/metrics.rssrc/services/rate_limit.rssrc/services/recommendasion.rssrc/services/skippage.rssrc/services/spotify_polling_backoff.rssrc/services/user.rssrc/services/word_definition.rssrc/spotify/errors.rssrc/spotify/mod.rssrc/telegram/actions/analyze.rssrc/telegram/actions/recommendasion.rs
🧰 Additional context used
🧬 Code graph analysis (2)
src/app.rs (5)
src/services/song_link.rs (1)
new(93-95)src/lyrics/genius.rs (1)
new(87-95)src/lyrics/musixmatch.rs (1)
new(91-98)src/lyrics/lrclib.rs (1)
new(90-96)src/metrics/influx.rs (2)
new(12-56)InfluxClient(5-9)
src/spotify/errors.rs (1)
src/infrastructure/error_handler.rs (2)
spotify_resp_error(68-169)spotify_client_error(172-204)
⏰ 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). (1)
- GitHub Check: Validation
🔇 Additional comments (20)
src/services/word_definition.rs (2)
2-6: LGTM!The import path updates correctly reflect the async-openai 0.32.2 API changes, moving to the chat module's message types instead of builder argument types.
77-79: LGTM!The migration from builder pattern to
Fromconversion for chat messages is clean and aligns with async-openai 0.32.2's API changes. The logic remains functionally equivalent.src/services/metrics.rs (1)
1-1: LGTM!The AsyncCommands import path update to
deadpool_redis::redis::AsyncCommandscorrectly aligns with the redis 1.0.2 upgrade and maintains consistency with the deadpool-redis connection pooling pattern used throughout the codebase.src/services/user.rs (1)
2-2: LGTM!The AsyncCommands import path update is consistent with the redis 1.0.2 migration and the deadpool_redis pattern used across the codebase.
src/spotify/mod.rs (1)
11-11: LGTM!The AsyncCommands import path update maintains consistency with the redis 1.0.2 upgrade and deadpool_redis integration.
src/services/magic.rs (1)
2-2: LGTM!The AsyncCommands import path update is consistent with the broader redis 1.0.2 migration.
src/services/rate_limit.rs (1)
2-2: LGTM!The AsyncCommands import path update completes the consistent pattern of migrating to deadpool_redis::redis::AsyncCommands across the codebase for the redis 1.0.2 upgrade.
src/app.rs (1)
245-247: The code correctly usesreqwest_compat(reqwest 0.12) for the OpenAI client because async-openai 0.32.2 requires reqwest 0.12.x. The approach of using a compatibility layer while maintaining reqwest 0.13 elsewhere is sound and requires no changes.src/services/spotify_polling_backoff.rs (1)
2-2: LGTM!The import path update to
deadpool_redis::redis::AsyncCommandscorrectly aligns with the deadpool-redis integration pattern. This ensures the trait methods are resolved from the same Redis crate version that deadpool-redis uses internally, avoiding potential version conflicts.src/services/skippage.rs (2)
1-1: LGTM!Import path correctly updated to use the re-exported
AsyncCommandsfromdeadpool_redis::redis.
70-77: LGTM!Using
deadpool_redis::redis::cmd("SCAN")is the correct approach for raw Redis commands that aren't covered by theAsyncCommandstrait. The command construction and async query execution are properly implemented.src/services/recommendasion.rs (1)
2-2: LGTM!Import path correctly updated to align with the crate-wide deadpool-redis integration pattern.
src/queue/profanity_check.rs (1)
2-2: LGTM!Import correctly updated. Using
as _is appropriate here since the trait is only needed for its methods, not referenced by name.src/rickroll.rs (1)
2-2: LGTM!Import path correctly updated to use the deadpool-redis re-exported
AsyncCommandstrait.src/telegram/actions/analyze.rs (2)
1-4: LGTM!Imports correctly updated to use the new async-openai 0.32.2 chat module types.
160-163: LGTM!The message construction is correctly updated to use the new
From-based API.ChatCompletionRequestUserMessage::from(prompt.as_ref()).into()properly converts the prompt string to a user message and then to the genericChatCompletionRequestMessagetype.src/telegram/actions/recommendasion.rs (3)
4-14: LGTM!Imports correctly updated to use the new async-openai 0.32.2 type system, including the new enum-based tool call types and named tool choice structs.
489-493: LGTM!Message construction correctly uses the
From-based pattern, converting string slices directly to system and user messages.
553-561: LGTM!The response handling correctly pattern-matches on
ChatCompletionMessageToolCalls::Functionto extract the function tool call, and the defensive check for the function name prevents unexpected behavior if the model calls a different function.src/spotify/errors.rs (1)
1-1: Use ofreqwest_compat::Responseis compatible.The
reqwest_compatdependency is an alias forreqwestversion 0.12 (while the main dependency uses 0.13). This compatibility approach supports rspotify's dependency on the older version. Both versions maintain API compatibility for the methods used in this file (chunk()) and by callers (status()), so no compatibility issues exist.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlsrc/infrastructure/cache.rssrc/lyrics/lrclib.rssrc/lyrics/mod.rssrc/lyrics/utils.rssrc/metrics/mod.rssrc/profanity.rssrc/spotify/mod.rssrc/telegram/actions/details.rssrc/telegram/actions/start.rssrc/telegram/commands.rssrc/tick/mod.rssrc/utils/general.rs
🧰 Additional context used
🧬 Code graph analysis (3)
src/metrics/mod.rs (1)
src/metrics/influx.rs (1)
new(12-56)
src/spotify/mod.rs (10)
src/spotify/errors.rs (2)
serde_json(71-71)serde_json(72-72)src/telegram/inline_buttons.rs (1)
from_str(91-93)src/entity/track_status.rs (1)
from_str(105-107)src/entity/user.rs (3)
from_str(180-182)from_str(236-238)from_str(262-264)src/telegram/keyboards.rs (1)
from_str(39-49)src/telegram/inline_buttons_admin.rs (1)
from_str(195-197)src/services/skippage.rs (2)
SkippageService(5-101)SkippageService(3-3)src/services/user.rs (1)
sync_current_playing(142-161)src/services/magic.rs (1)
MagicService(6-34)src/services/track_status.rs (1)
TrackStatusService(47-165)
Cargo.toml (1)
src/app.rs (1)
AIConfig(33-36)
⏰ 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). (1)
- GitHub Check: Validation
🔇 Additional comments (18)
src/telegram/actions/start.rs (1)
55-55: LGTM: Minor formatting improvement.Removing the extraneous semicolon after the closing brace is a correct style improvement with no semantic impact.
src/spotify/mod.rs (2)
11-11: LGTM: AsyncCommands import updated consistently.The import path change from
redis::AsyncCommandstodeadpool_redis::redis::AsyncCommandsaligns with the project-wide migration to use the deadpool_redis re-export.
411-415: LGTM: Cleaner control flow with let chains.The refactoring from nested
if letstatements to chainedif let ... && let ...is more idiomatic and reduces nesting. This feature requires Rust edition 2021, which should already be verified per the previous comment.src/telegram/actions/details.rs (1)
2-2: LGTM: Regex static migrated toLazyLock.The migration maintains lazy initialization semantics while using the standard library. The regex is still compiled once on first use and shared across threads safely.
Also applies to: 74-75
src/lyrics/lrclib.rs (1)
1-1: LGTM: Function-scoped static migrated toLazyLock.The migration is correct. Function-local statics with
LazyLockare idiomatic and maintain the same lazy initialization pattern for the regex used to parse synced lyrics timestamps.Also applies to: 166-169
src/profanity.rs (1)
4-4: LGTM: Multiple statics migrated consistently toLazyLock.All five static variables (three
Typemasks and twoRegexinstances) have been correctly migrated fromlazy_statictoLazyLock. The lazy initialization semantics and thread-safety are preserved.Also applies to: 10-12, 180-181
src/lyrics/utils.rs (2)
3-17: LGTM! Clean migration from lazy_static to LazyLock.The migration from
lazy_statictostd::sync::LazyLockfor regex initialization is correctly implemented and aligns with the modernization goals of this PR.
65-65: Edition 2024 already ensures compatibility withf64::midpoint.The project uses Rust edition 2024, which was released with Rust 1.83.0. This implicitly guarantees that the minimum supported Rust version is at least 1.83.0, where
f64::midpointwas stabilized. No further verification of MSRV is needed.The change from arithmetic mean to
f64::midpointis intentional and safe. For confidence scores in the typical range [0.0, 1.0], both implementations produce identical results, butmidpointis more robust against extreme values.src/lyrics/mod.rs (1)
1-1: LGTM! Consistent LazyLock migration.The migration of
LYRICS_CACHE_TTLfromlazy_statictostd::sync::LazyLockis correctly implemented, maintaining the same initialization and access patterns.Also applies to: 123-123
src/metrics/mod.rs (3)
1-1: LGTM! Proper LazyLock migration for timing.The migration of
START_TIMEfromlazy_statictostd::sync::LazyLockcorrectly initializes the instant for uptime tracking.Also applies to: 74-74
194-194: LGTM! More idiomatic pattern matching.The change from
let Some(ref client)tolet Some(client)is cleaner. Sinceapp.influx()returns&Option<InfluxClient>, the pattern match on the reference automatically bindsclientas a reference, making the explicitrefunnecessary.
200-200: LGTM! Eager initialization for predictable performance.Explicitly dereferencing
START_TIMEduring daemon initialization ensures theLazyLockis initialized early, avoiding first-access latency during metrics collection. This is a good practice for timing-critical code.src/infrastructure/cache.rs (1)
1-1: LGTM! Consistent LazyLock migration.The migration of
REDIS_URLfromlazy_statictostd::sync::LazyLockis correctly implemented, maintaining proper initialization and async access patterns.Also applies to: 11-11
src/tick/mod.rs (1)
6-6: LGTM! Proper channel initialization with LazyLock.The migration of
PROCESS_TIME_CHANNELfromlazy_statictostd::sync::LazyLockcorrectly wraps the broadcast channel initialization. The tuple access pattern at line 151 (PROCESS_TIME_CHANNEL.0.send(...)) remains valid and properly accesses the sender.Also applies to: 29-32
src/utils/general.rs (1)
2-2: LGTM! Clean migration to std::sync::LazyLock.The migration from lazy_static to std::sync::LazyLock for TICK_STATUS is well-executed. The pattern correctly combines LazyLock with RwLock for thread-safe lazy initialization.
Also applies to: 10-11
Cargo.toml (3)
27-28: Good use of reqwest-compat for backward compatibility.The addition of reqwest-compat (aliasing reqwest 0.12) alongside the upgrade to reqwest 0.13 is a smart approach for maintaining compatibility with rspotify while adopting the latest reqwest features elsewhere in the codebase.
10-10: Dependency upgrades look reasonable.Most dependency updates are patch or minor versions that should be backward compatible. The convert_case upgrade from 0.9.0 to 0.10.0 (minor version bump) might include API changes, but given the scope of typical case conversion libraries, this should be low risk.
Also applies to: 15-16, 18-18, 31-31, 33-33, 40-40, 42-42, 44-44, 53-53
9-9: The codebase has been properly updated for async-openai 0.32.2. All chat completion usage insrc/services/word_definition.rs,src/telegram/actions/analyze.rs, andsrc/telegram/actions/recommendasion.rsuses the current API patterns:CreateChatCompletionRequestArgsbuilder, new message types, and the new tools-based function calling (ChatCompletionTool) instead of the deprecated functions array. No legacy API patterns detected.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/utils/general.rs (2)
101-101: Critical: Data race onstatic mut KILLEDremains unresolved.The
static mut KILLEDflag is accessed without synchronization inlisten_for_ctrl_c()(line 108) andctrl_c()(line 112), creating undefined behavior when these functions run concurrently. This critical issue was flagged in a previous review but remains unaddressed.🔎 Recommended fix using AtomicBool
+use std::sync::atomic::{AtomicBool, Ordering}; + -static mut KILLED: bool = false; +static KILLED: AtomicBool = AtomicBool::new(false);Then update the access sites:
In
listen_for_ctrl_c():- unsafe { KILLED = true }; + KILLED.store(true, Ordering::Relaxed);In
ctrl_c():- if unsafe { KILLED } { + if KILLED.load(Ordering::Relaxed) { return; }Also applies to: 108-108, 112-112
98-99: Consider storing only the Sender.As noted in a previous review, the
KILLstatic stores both Sender and Receiver, but only the Sender is used (.0.send()at line 106 and.0.subscribe()at line 116). Storing only the Sender would simplify the code.🔎 Proposed optimization
-static KILL: LazyLock<(broadcast::Sender<()>, broadcast::Receiver<()>)> = - LazyLock::new(|| broadcast::channel(1)); +static KILL: LazyLock<broadcast::Sender<()>> = + LazyLock::new(|| broadcast::channel(1).0);Then update the access sites:
- KILL.0.send(()).ok(); + KILL.send(()).ok(); - KILL.0.subscribe().recv().await.ok(); + KILL.subscribe().recv().await.ok();Also applies to: 106-106, 116-116
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.tomlsrc/utils/general.rs
⏰ 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). (1)
- GitHub Check: Validation
🔇 Additional comments (4)
src/utils/general.rs (1)
2-2: LGTM: Clean migration to LazyLock.The migration from lazy_static to std::sync::LazyLock is well done. LazyLock provides the same thread-safe lazy initialization without requiring an external dependency.
Also applies to: 9-10
Cargo.toml (3)
15-15: No action needed. The convert_case update from 0.9.0 to 0.10.0 is safe for this codebase. While 0.10.0 does include breaking changes (removed case variants, renamed boundary methods, and removedBoundary::from_delim), the code only uses the basicto_case(Case::Title)API, which is unaffected by these changes.
26-27: No action needed. The dual-version reqwest approach is intentional and properly implemented. rspotify 0.15.3 requires reqwest 0.12, necessitating thereqwest-compatalias. The versions are correctly isolated:reqwest-compat(0.12) is used exclusively for Spotify operations (src/app.rs,src/spotify/errors.rs), whilereqwest0.13 handles all other HTTP clients (lyrics services, metrics). Thefeatures = ["query"]flag on line 26 addresses the 0.13 breaking change wherequery()moved to an optional feature. No type incompatibility issues arise because reqwest types are not shared across version boundaries.
8-8: The async-openai upgrade from 0.30.1 to 0.32.2 spans breaking changes introduced in v0.31 (type/shape reshaping). The codebase has been properly updated—all usage sites already employ the correct v0.32-compatible namespaced types (ChatCompletionRequestSystemMessage, ChatCompletionRequestUserMessage, CreateChatCompletionRequestArgs, ChatCompletionTool, etc.) across word_definition.rs, analyze.rs, and recommendasion.rs. No API mismatches detected.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/utils/general.rs (1)
99-100: Consider storing only the Sender to avoid unused Receiver.The migration to
LazyLockis correct, but the tuple still stores bothSenderandReceiverfrombroadcast::channel(1)while only theSender(via.0) is used. This matches the previous review suggestion that remains unaddressed.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlrust-toolchain.tomlsrc/utils/general.rs
⏰ 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). (1)
- GitHub Check: Validation
🔇 Additional comments (7)
src/utils/general.rs (3)
2-3: LGTM! Clean migration to standard library primitives.The imports correctly reflect the migration from
lazy_statictostd::sync::LazyLockand the addition of atomic synchronization primitives. Both are used appropriately throughout the file.
10-11: LGTM! Correct migration toLazyLock.The migration from
lazy_statictostd::sync::LazyLockis implemented correctly. The type signature and initialization are appropriate, and the change maintains API compatibility with existing usage sites.
102-102: LGTM! Properly addresses the previous data race.The
AtomicBoolcorrectly replaces the unsafestatic mut KILLEDand eliminates the data race. The use ofOrdering::Relaxedis appropriate here sinceKILLEDis a simple flag for early-return optimization, and the actual synchronization is provided by theKILLbroadcast channel.Also applies to: 109-109, 113-113
Cargo.toml (4)
10-10: LGTM: Patch and minor version updates.The following dependency updates are low-risk patch or post-1.0 minor version bumps:
- axum 0.8.8
- clap 4.5.53
- derive_more 2.1.1
- rustrict 0.7.37
- serde_json 1.0.148
- tracing 0.1.44
- tracing-subscriber 0.3.22
- uuid 1.19.0
- sea-orm 1.1.19
These maintain backward compatibility. Please verify the project builds successfully with
cargo check --all-features.
16-16: No action required—convert_case 0.10.0 is compatible.The codebase uses only
Case::Titleand theCasing::to_case()method, both of which remain unchanged in 0.10.0. The breaking changes (removal of Toggle/Alternating/Random/PseudoRandom variants, Boundary API changes, and method renames) do not affect this usage.
27-28: Reqwest 0.13 migration is properly configured.The upgrade from 0.12 to 0.13 is correctly implemented:
- The
"query"feature is explicitly enabled in Cargo.toml (required in 0.13 as it became optional)- Direct reqwest usages (
ClientBuilder,Client,StatusCode) remain compatible across both versions- The
reqwest-compatalias properly isolates reqwest 0.12 for rspotify 0.15.3 compatibility, withsrc/spotify/errors.rscorrectly importingResponsefromreqwest_compatNote: The default TLS backend changed from native-tls to rustls in 0.13, but this requires no code changes—only be aware if native-tls-specific behavior is relied upon.
9-9: Breaking changes from async-openai 0.30.1 → 0.32.2 are fully addressed.Version 0.32.2 is correctly specified and all chat completion types are properly migrated to the new API shapes (CreateChatCompletionRequestArgs, ChatCompletionRequestSystemMessage, etc.). No migration issues remain.
vtvz
changed the title
1 participant
Updates major and minor versions across dependency tree including async-openai, reqwest, hyper, tokio, and other core crates. Removes deprecated lazy_static dependency and updates Rust edition to 2024.