Repository navigation
Conversation
neur0map
added
the
mika:review
Member
🦊 mika's reviewSummary
Estimated review effort: 2/5 · risk: medium 📁 Walkthrough details
|
| Severity | Category | Fix effort | File | Issue |
|---|---|---|---|---|
| 🟠 major | correctness | low | crates/core/src/state.rs |
Possible Bug — The delayed Spotify retry checks only the track ID. If the user explicitly restarts the same track or navigates to another occurrence of it during the delay, the old task increments the generation and restarts the newer… |
| 🟡 minor | correctness | medium | crates/core/src/state.rs |
Possible Bug — queued_from identifies the source, not a particular insertion. If the same playlist is added twice before its first continuation arrives, the first continuation is inserted after the second copy, interleaving the two c… |
| 🟡 minor | correctness | low | crates/core/src/state.rs |
Possible Bug — The retry budget is shared across all items in this loop. If a track uses retries and then proves unavailable, a later track receives fewer than three network retries; after three earlier retries it receives none. Reset… |
| 🟡 minor | correctness | medium | crates/core/src/state.rs |
Possible Bug — Resetting retried on every position update at or beyond three seconds also clears the Spotify retry guard while its delayed retry is pending. If Spotify reports another Unavailable while mpv continues reporting the o… |
⚡ Quick fixes by effort
Low effort (apply from the diff):
crates/core/src/state.rs:2754 — Possible Bug — The delayed Spotify retry checks only the track ID. If the user explicitly restarts the same track or navigates to another occurrence of it during the delay, the old task increments the generation and restarts the newer…crates/core/src/state.rs:1932 — Possible Bug — The retry budget is shared across all items in this loop. If a track uses retries and then proves unavailable, a later track receives fewer than three network retries; after three earlier retries it receives none. Reset…
Medium effort (a function-level change):
crates/core/src/state.rs:4053 — Possible Bug —queued_fromidentifies the source, not a particular insertion. If the same playlist is added twice before its first continuation arrives, the first continuation is inserted after the second copy, interleaving the two c…crates/core/src/state.rs:2596 — Possible Bug — Resettingretriedon every position update at or beyond three seconds also clears the Spotify retry guard while its delayed retry is pending. If Spotify reports anotherUnavailablewhile mpv continues reporting the o…
🤖 Prompt for AI agents (click, copy, paste into Claude/Codex)
Fix the following issues found by mika-agent on Ryoku-dev/ryotunes#27 (branch fix/queue-order-and-network-recovery):
- crates/core/src/state.rs:2754-2764 — Possible Bug (major severity, low effort): The delayed Spotify retry checks only the track ID. If the user explicitly restarts the same track or navigates to another occurrence of it during the delay, the old task increments the generation and restarts the newer…
- crates/core/src/state.rs:4053-4060 — Possible Bug (minor severity, medium effort): `queued_from` identifies the source, not a particular insertion. If the same playlist is added twice before its first continuation arrives, the first continuation is inserted after the second copy, interleaving the two c…
- crates/core/src/state.rs:1932-1935 — Possible Bug (minor severity, low effort): The retry budget is shared across all items in this loop. If a track uses retries and then proves unavailable, a later track receives fewer than three network retries; after three earlier retries it receives none. Reset…
- crates/core/src/state.rs:2596-2598 — Possible Bug (minor severity, medium effort): Resetting `retried` on every position update at or beyond three seconds also clears the Spotify retry guard while its delayed retry is pending. If Spotify reports another `Unavailable` while mpv continues reporting the o…
For each: verify it is real in the current code, apply the smallest correct fix, and run the repo's tests. Skip anything already resolved and say why.
🚦 Merge recommendation: changes_required
🦊 Generated by Mika — the Ryoku org's self-hosted AI code reviewer. Machine findings: verify before merging.
neur0map
requested changes
Nov 9, 2026
neur0map
left a comment
neur0map
left a comment
Member
There was a problem hiding this comment.
🔴 Changes requested — 0 critical and 1 major issue(s): see the inline comments and the walkthrough for fixes and an agent-ready prompt.
Comment on lines
+1932
to
+1935
| if e.is_transient_network() { | ||
| if network_retries < 3 { | ||
| let delay = std::time::Duration::from_millis(400 << network_retries); | ||
| network_retries += 1; |
Member
There was a problem hiding this comment.
🟡 minor · Possible Bug · fix: low effort
The retry budget is shared across all items in this loop. If a track uses retries and then proves unavailable, a later track receives fewer than three network retries; after three earlier retries it receives none. Reset the budget when advancing to another item.
Comment on lines
+2596
to
+2598
| if pos >= 3.0 { | ||
| q.retried = None; | ||
| } |
Member
There was a problem hiding this comment.
🟡 minor · Possible Bug · fix: medium effort
Resetting retried on every position update at or beyond three seconds also clears the Spotify retry guard while its delayed retry is pending. If Spotify reports another Unavailable while mpv continues reporting the old position, another retry can be scheduled instead of taking the guarded failure path.
Comment on lines
+2754
to
+2764
| tokio::spawn(async move { | ||
| // Let librespot reconnect before replacing the FIFO. If it still reports | ||
| // Unavailable, the one-shot guard below lets the normal skip path run. | ||
| tokio::time::sleep(std::time::Duration::from_millis(2500)).await; | ||
| let gen = { | ||
| let mut q = me.queue.lock().await; | ||
| if q.items.get(q.current).map(|i| i.video_id.as_str()) != Some(&video_id) { | ||
| return; | ||
| } | ||
| q.lookahead_loaded = None; | ||
| me.generation.fetch_add(1, Ordering::SeqCst) + 1 |
Member
There was a problem hiding this comment.
🟠 major · Possible Bug · fix: low effort
The delayed Spotify retry checks only the track ID. If the user explicitly restarts the same track or navigates to another occurrence of it during the delay, the old task increments the generation and restarts the newer playback. Capture the generation when scheduling the retry and abandon it if that generation has changed.
Comment on lines
+4053
to
+4060
| fn queued_block_end(q: &QueueState, from: &str) -> Option<usize> { | ||
| q.items | ||
| .iter() | ||
| .enumerate() | ||
| .skip(q.current + 1) | ||
| .filter(|(_, item)| item.queued_end && item.queued_from.as_deref() == Some(from)) | ||
| .map(|(index, _)| index + 1) | ||
| .last() |
Member
There was a problem hiding this comment.
🟡 minor · Possible Bug · fix: medium effort
queued_from identifies the source, not a particular insertion. If the same playlist is added twice before its first continuation arrives, the first continuation is inserted after the second copy, interleaving the two collections. Track the insertion instance when placing continuation pages.
neur0map
removed
the
mika:review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
A transient network switch could be treated as permanently unavailable media, causing playback to skip tracks or remove items from the queue. Add to queue also inserted collections ahead of the active album or playlist.
Lifecycle / surface
Playback resolution and Spotify session recovery in the Rust core; queue insertion and continuation ordering in the playback queue.
Validation
cargo test --workspace --lockedcargo fmt --all -- --checkgit diff --check