feat: shuffle and repeat for Local Files, Subsonic, and YouTube#373
Merged
Conversation
- Wrap decoded context to the first track under Repeat All when suspending to the native queue (was stopping at the boundary) - Persist decoded repeat mode, shuffle intent, and the shuffle permutation; restore them on launch and reconcile a deferred toggle on the next tick - Route MPRIS shuffle/repeat setters and explicit IoEvent::Repeat to the decoded state when a queueable source owns playback - Hide shuffle/repeat controls for radio and native-queue slots in the playbar and MPRIS snapshots (self-contained placeholder segments) - Push local-file decoded modes to MPRIS and reset loop state for mode-less sources - Defer shuffle reordering while a track change is in flight to avoid a stale play_index target; validate persisted permutations before indexing - Broaden the std::sync::Arc import gate to audio-decode so decoded-only builds compile; silence the resulting dead-code warnings
Encode repo conventions as path instructions (IoEvent dispatch, keybinding lookups, dialog cleanup, slim-build feature gating) and point the knowledge base at CLAUDE.md, AGENTS.md and copilot-instructions.md. Raise the github-checks timeout to 15m so reviews wait for the Rust matrix build instead of giving up at the 90s default, skip dependabot and allcontributors PRs, and drop the docstring coverage check. .luacheckrc declares the injected `spotatui` global so the example plugins do not trip undefined-variable warnings.
Temporary, not for main. Carries the write-up between machines while the native-queue re-gating fix is in progress. See the cleanup section in the file before removing it.
A queued song preempts a decoded context via Decision::SuspendToQueue, which
advance_decision returns as soon as the queue is non-empty, before it ever
consults the repeat mode. The suspend path then computed its resume index with
advance_index(.., forward: true), where RepeatMode::Track shares the clamping
arm with Off and yields current + 1.
So when a track ended with Repeat One on and anything queued, the repeated track
was silently skipped (track 5 of 10 resumed at 6), and on the last track
advance_index returned None, which reads as "context exhausted" and tore the
whole context down instead of resuming it.
Add resume_index_after_queue, taking a SuspendCause because the two handoffs to
the queue need opposite rules under Repeat One:
- AutoAdvance (the track ended on its own): replay the current track, so a
queued song cannot consume the repeat.
- ManualSkip (the user pressed Next): advance past it, matching the per-source
skip paths. Repeat-one only replays on *auto* advance.
Every other mode defers to advance_index either way: Context still wraps last to
first, Off still clamps to None. advance_index itself is unchanged -- it is
shared with the manual Next/Previous paths, whose repeat-one clamping is already
correct.
Add the Unreleased "Added" entry for shuffle/repeat on the decoded sources.
docs/configuration.md was left behind when the default playbar_status_source
template gained {shuffle}/{repeat}: it documented the old default and claimed
"the defaults above reproduce the built-in output exactly", which was no longer
true. It also said both playbar templates accept the same keys without noting
that {shuffle}/{repeat} are bare values in playbar_status but come pre-composed
with their own " | Label: " prefix in playbar_status_source, so the segment can
vanish for sources without the control.
The queue engine's presence was keyed off `audio-decode`, but its use requires a source with a finite track list. Internet radio enables `audio-decode` (it decodes a live stream) without being queueable, so a radio-only build compiled the whole queue-for-decoded-tracks machinery and never used it: 9 dead-code warnings from `cargo check --no-default-features --features internet-radio`. Re-gate those items on `any(local-files, subsonic, youtube)` -- the set that actually constructs `QueueNowPlaying::Decoded` and drives the fetch/advance/ resume paths. The queue-slot field and its accessors move to `any(streaming, local-files, subsonic, youtube)`, since a slot can also hold a native-streamed Spotify track. Genuinely shared decode/output machinery that radio does use (the rodio sink, the local playbar view) stays on `audio-decode`. This is pure gating: no logic or control flow changes. The re-gated code was provably unreachable in the affected configs -- `try_play_queued` has no radio arm, so `queue_now` was permanently `None` there, which is what the compiler was reporting. Also fixes a latent compile error the warnings were hiding: the `stray_over_queue` guard in player/events.rs sits inside streaming-gated code, so `streaming, internet-radio` failed to build. Neither the radio-only build nor CI's full matrix (which enables the queueable sources too) covered that combination. Verified warning- and error-free across every single-source build, radio paired with each queueable source, streaming+internet-radio, all-sources, and both CI clippy gates.
The controls row rendered all eight buttons from the unfiltered geometry helper, while hit-testing ran the filtered app-level one and kept only Play/Pause for a non-Spotify source. Two sources of truth: local files drew Shuffle and Repeat that looked clickable and silently swallowed the click. Route rendering and hit-testing through one source-aware list, so a drawn button is always one that works. The old filter kept only Play/Pause because, when it was written, nothing else had a per-source route and a click would have driven the paused librespot instead of the audible source. That premise is now stale: prev/next, volume, and (as of this branch) shuffle/repeat all branch on the owning source first. Only Like still needs a Spotify item id, which no decoded source has. So a queueable decoded source (Local/Subsonic/YouTube) now offers prev, next, play/pause, shuffle, repeat and volume, but not Like. Internet radio and the native queue slot have no finite track list to skip or shuffle, so they keep play/pause and volume only -- matching the playbar title, which already hides their shuffle/repeat segments. The support matrix is a pure function of three booleans rather than of &App: a LocalPlaybackState needs an Arc<LocalPlayer> holding an open audio device, so it could not otherwise be tested.
….com/LargeModGames/spotatui into feature/repeat-shuffle-other-sources
- Repeat One now keeps the context alive when Next is pressed on the last track with a song queued. `resume_index_after_queue` returned `None`, which `advance_index` means as "clamp, the skip is a no-op" but every resume path reads as "context exhausted, tear down"; the two only coincide under Off. - Repeat All no longer walks an entirely unplayable queue at tick rate. The deliberate skip-past self-terminates under Off but Context wraps forever, so local play failures are bounded to the tracks that exist. - Shuffle and repeat no longer reach the real Spotify device from radio or the native queue slot, across the four surfaces that must agree: the keyboard, the playbar controls, the MPRIS snapshot, and the MPRIS setters. - The queue slot's playbar keeps the transport it can honour and drops Like, which acted on the suspended context rather than the audible track. - The snapshot describes the queued track instead of the suspended context, and macOS Now Playing gained the queue filter its MPRIS twin already had. - Correct the changelog's repeat cycle order and drop the stale temp/ bug report now that the re-gating fix it tracked has landed.
📝 WalkthroughWalkthroughDecoded Local, Subsonic, and YouTube playback now support repeat and shuffle controls, persistence, MPRIS routing, repeat-aware advancement, replay handling, and source-specific playbar rendering. Queue ownership and feature gates were also aligned with these behaviors. ChangesDecoded playback controls
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TUI
participant App
participant SourceRouter
participant MPRIS
User->>TUI: toggle shuffle or repeat
TUI->>App: update decoded playback mode
App->>SourceRouter: apply mode to active Local/Subsonic/YouTube queue
SourceRouter->>App: update playback state and snapshot
App->>MPRIS: publish shuffle and loop status
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/configuration.md (1)
172-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMinor: MD038 markdown-lint spacing in the key table.
The literal leading/trailing spaces inside
` | Shuffle: On `and` | Repeat: All `(used to illustrate the exact rendered spacing) trip markdownlint-cli2's MD038 rule. If this repo enforces markdownlint in CI, this would fail that check.📝 Suggested fix: avoid literal spaces inside the code span
-| `{shuffle}` | `On` / `Off` | padded to 3 characters. In `playbar_status_source`: pre-composed as ` \| Shuffle: On `, empty for radio/queue slots | -| `{repeat}` | `Off` / `Track` / `All` | padded to 5 characters. In `playbar_status_source`: pre-composed as ` \| Repeat: All `, empty for radio/queue slots | +| `{shuffle}` | `On` / `Off` | padded to 3 characters. In `playbar_status_source`: pre-composed as `\| Shuffle: On` (with its own leading separator and trailing pad), empty for radio/queue slots | +| `{repeat}` | `Off` / `Track` / `All` | padded to 5 characters. In `playbar_status_source`: pre-composed as `\| Repeat: All` (with its own leading separator and trailing pad), empty for radio/queue slots |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/configuration.md` around lines 172 - 179, Update the `{shuffle}` and `{repeat}` table descriptions in the configuration documentation to avoid literal leading or trailing spaces inside inline code spans, while still documenting the exact rendered spacing using a markdown-safe notation or surrounding prose. Preserve the existing pre-composed output examples and their spacing semantics.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/configuration.md`:
- Around line 172-179: Update the `{shuffle}` and `{repeat}` table descriptions
in the configuration documentation to avoid literal leading or trailing spaces
inside inline code spans, while still documenting the exact rendered spacing
using a markdown-safe notation or surrounding prose. Preserve the existing
pre-composed output examples and their spacing semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 570661ff-b934-4f70-a850-aea097d9f260
📒 Files selected for processing (22)
.coderabbit.yaml.luacheckrcCHANGELOG.mddocs/configuration.mdexamples/config.example.ymlsrc/core/app.rssrc/core/persisted_playback.rssrc/core/user_config.rssrc/infra/local/dispatch.rssrc/infra/local/mod.rssrc/infra/media_metadata.rssrc/infra/network/mod.rssrc/infra/player/events.rssrc/infra/queue/dispatch.rssrc/infra/queue/mod.rssrc/infra/subsonic/dispatch.rssrc/infra/subsonic/mod.rssrc/infra/youtube/dispatch.rssrc/infra/youtube/mod.rssrc/runtime.rssrc/tui/runner.rssrc/tui/ui/player.rs
This was referenced Jul 15, 2026
Open
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
1 participant
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.
Closes #367.
Shuffle (
Ctrl+s) and repeat (Ctrl+r) now work when one of the decoded sources owns playback, not just on Spotify.What this adds
IoEvent::ReplayCurrentTrack(remote sources replay the retained tempfile, so no re-download).last_session.yml, and are routed through MPRIS soplayerctl shuffle/loopand desktop media controls drive them.Internet radio and the native queue slot have no track queue of their own, so they drop the shuffle/repeat controls rather than showing inert ones.
Design
Player-global
App::decoded_repeat/App::decoded_shuffle, with the decision logic as pure, unit-tested helpers insrc/infra/queue/mod.rs(RepeatMode,advance_decision,advance_index,resume_index_after_queue,toggle_shuffle/ShuffleBackup). The per-source dispatchers share those helpers, so the three sources cannot drift apart.The native queue engine is also re-gated from
feature = "audio-decode"onto the queueable sources (local-files/subsonic/youtube). Internet radio decodes audio but is never a queue item, so a radio-only build previously compiled the whole queue engine and never used it.cargo check --no-default-features --features internet-radiois now warning-free.Two subtleties worth a reviewer's attention
Option<usize>carries two meanings. Toadvance_index,Nonemeans "clamp, the skip is a no-op". To everyresume_index_after_queueconsumer, it means "context exhausted, tear down". Those coincide only underRepeatMode::Off. Under Repeat One that conflation stopped playback dead, soresume_index_after_queuenow never returnsNonefor an in-range index underTrack.Repeat All removes a terminator. Local playback deliberately skips past unplayable files, which self-terminates under Off (the last track has no next) but wraps forever under Context. Failures are now bounded to the distinct tracks that exist, and the teardown happens under the same lock that observes exhaustion. Clearing the guard and unlocking first raced the runner tick into leaking a
NextTrackto the user's real Spotify device.One invariant, four surfaces. "The queue slot and radio have no shuffle/repeat" has to hold on the keyboard, the playbar controls, the MPRIS snapshot, and the MPRIS setters. Each one had a different hole; a mismatch meant a keypress silently mutating your real Spotify device with nothing on screen to show for it.
Testing
cargo fmt,cargo clippyclean on both the slim CI feature set and--features all-sources.all-sources, 436 under the slim set. New unit tests cover the repeat/shuffle state machine, the queue-handoff resume indices, and the playbar control matrix.local-files,subsonic,youtube,internet-radio) compile warning-free.Known gaps
infra::local::tests::uri_round_tripfails on Windows (hardcoded Unix path). Pre-existing, verified failing identically onmain, and invisible to CI becauselocal-filesis not in the slim feature set.ubuntu-latestjob covers MPRIS; macOS is only built incd.ymlon release, so the one-line macOS filter has no PR-time compile check.Pre-existing issues found but deliberately left alone
Out of scope here, worth follow-up issues: queue-slot Play/Pause misroutes to a Web-API resume for a Spotify slot; keyboard Like/
Shift+Wacts on the suspended context rather than the audible track; a Lua plugin'sIoEvent::Repeatstill reaches Spotify during radio.Summary by CodeRabbit
New Features
{shuffle}and{repeat}placeholders.Bug Fixes
Documentation