Conversation
|
Ok so update, I had codex decompile the spotify desktop app, then pull the precise way spotify "generates" the radio playlists, which is with the api endpoint I have no idea how maintainers want to move forward with this changset. I also have another change that makes the radio pages just render with librespot's built in radio API, but that generates different radios to the actual spotify internal ones (I think) Please let me know what to do --- More context --- Librespot's station API hits I don't know if this behaviour is desirable or not (creating playlists on the user account), but for personal use I've built this so that it matches the behaviour I like. I can also make it a settings toggle if that's desired. I've also got a local changeset that bypasses librespot's thing where they don't want to writeback recently played songs/playlists to spotify, which I'm using, and can upstream if that's wanted. |
crmne
left a comment
There was a problem hiding this comment.
The visible scope, browsing song radio separately from starting playback, fits. Please use your version backed by librespot’s existing typed context resolver. We can accept the station it provides without matching Spotify’s private radio playlist exactly.
Remove the inspiredby-mix endpoint and keep playlist creation and listening-history writeback out of this PR. Rebase that scoped version onto main and add the required tests and visual evidence.
Np. Just a question, does using the internal api break spotify tos lol Also, those other changes were not meant to be in this PR, that's my fault. Thanks for the comments. |
Separate radio-page navigation from immediate station playback. Preserve loading metadata, ignore stale responses, and retry restored pages when the local playback session becomes ready. Keep private playlist creation and listening-history reporting outside this change. Include regression tests and isolated light/dark before-and-after captures. Co-Authored-By: GPT-6 (OpenAI) <noreply@openai.com>
0e57e87 to
81ac67e
Compare
crmne
left a comment
There was a problem hiding this comment.
Visible scope remains the previously accepted split between browsing a radio page and starting radio playback. The current code uses the requested librespot resolver approach, and CI passes.
Please turn the screenshot pairs into the required HTML comparison, with theme/size selectors and Before/After controls, and include loading, error/retry and empty states. The existing scope decision stands; final visual review and authenticated station-resolution validation remain outstanding.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved navigation, caching, API-contract, restoration, and track-validity issues remain, and visual-review evidence is incomplete.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 4
Open (5)
Radio pages are never evicted, causing unbounded memory growth · New Opening radio from fullscreen lyrics does not exit fullscreen · New Radio browsing consumes shared Web API quota · New Radio actions are available for unsupported tracks · New Missing required visual review evidence for radio page · New
What changed in this PR
Adds a browsable song-radio page with librespot resolution, playback controls, queue integration, tests, demos, and documentation.
Changes:
- Adds radio navigation and track-menu actions.
- Implements asynchronous station loading and metadata.
- Adds radio UI, caching, artwork handling, and playback.
- Updates documentation and visual evidence.
| File | Summary |
|---|---|
src/ui/widgets.rs |
Adds song-radio menu actions. |
src/ui/radio.rs |
Renders the radio page and controls. |
src/ui/mod.rs |
Registers radio routing and tinting. |
src/ui/collection.rs |
Preserves sorted radio playback order. |
src/radio.rs |
Resolves stations and metadata. |
src/player.rs |
Exposes radio resolution. |
src/model.rs |
Adds radio pages and actions. |
src/lib.rs |
Exports the radio module. |
src/images.rs |
Adds artwork readiness checks. |
src/demo.rs |
Adds demo radio data and tests. |
src/backend.rs |
Loads radio stations asynchronously. |
src/app/radio.rs |
Manages radio state and navigation. |
src/app.rs |
Integrates radio events, playback, and caching. |
README.md |
Documents song radio. |
docs/assets/song-radio/README.md |
Provides screenshot evidence. |
docs/_reference/queue.md |
Updates queue behavior documentation. |
docs/_reference/how-it-connects.md |
Documents radio architecture. |
docs/_guide/using-spotifast.md |
Adds user guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pub playlist_pages: HashMap<String, PlaylistPage>, | ||
| load_generation: u64, | ||
| pub album_pages: HashMap<String, AlbumPage>, | ||
| pub radio_pages: HashMap<String, crate::radio::RadioPage>, |
| } | ||
| match action { | ||
| Action::Open(page) => self.open(page), | ||
| Action::OpenRadio(track) => self.open_radio(*track), |
| self.track_cache.insert(id.clone(), track); | ||
| } | ||
| } | ||
| self.request_contains(uris); |
| if menu_item(ui, &palette, Some(Icon::Radio), "Go to song radio") { | ||
| app.actions.push(Action::OpenRadio(Box::new(track.clone()))); | ||
| } | ||
| if menu_item(ui, &palette, Some(Icon::Play), "Start song radio") { | ||
| app.actions.push(Action::PlayTrackRadio(uri.clone())); | ||
| } |
| | Theme and size | Before | After | | ||
| | --- | --- | --- | | ||
| | Dark, 1240x800 |  |  | | ||
| | Light, 1240x800 |  |  | | ||
| | Dark, 760x800 |  |  | | ||
| | Light, 760x800 |  |  | |


[agent] Browse song radio without interrupting playback
Co-Authored-By: GPT-6 (OpenAI) noreply@openai.com