Show genres on detail pages and fill them on scrobble - #1433
Conversation
|
Four browser tests in Likely cause, from the browser console ( No fix exists yet, and it belongs in #1424, so I'm not widening this PR. It stays red until #1424 is fixed. Generated by Claude Code |
- The artist view already built genre_chips from MusicBrainz, but the template never rendered them. - Register a Genres sidebar section for music_artist so the layout settings can show and reorder it.
- After the write transaction, fill an album saved without genres from its MusicBrainz release group, then resync the play. - Plays fall back to the album artist's genres when the album has none. - Store artist genres from scrobbles as names, matching the artist page, instead of raw MusicBrainz genre objects.
Album and podcast show rows already store genres, but those detail pages never rendered them. Register the album sidebar section the same way artist pages already do.
Season metadata already copies the show's genres. The episode page never rendered them.
- Copy the first non-empty list onto the related rows that are still empty, including after a listen hook writes one. - Show artist genres on the album and artist pages when the album itself has none.
- The artist's genres were copied onto an empty album while the scrobble item was created, so the release-group fill never ran for artists with genres. Note the empty album before that copy and use it as the gate. - Add tests for the genre copy rules, the artist/album/podcast chips, and the scrobble fill order. - Tidy a docstring line that had been joined. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNPhmcXNv6M1Beby8LQime
8d992a8 to
8aa3219
Compare
dannyvfilms
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aa3219248
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if album and not getattr(event, "defer_cover_prefetch", False): | ||
| if album_needs_genre_fill: | ||
| try: | ||
| populate_album_implied_genres(album) |
There was a problem hiding this comment.
When an empty album with a release-group ID belongs to an artist that already has genres, _get_or_create_item() calls sync_music_item_genres_from_album(), which copies the artist fallback through the item and into the album before this call. populate_album_implied_genres() then treats those artist genres as the album's direct genres, so _album_direct_genres() never calls get_release_group_genres() and the album permanently retains the less-specific artist genres. The saved album_needs_genre_fill flag does not force a provider lookup; the release-group fill must happen before the fallback is persisted or explicitly bypass existing album genres.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, you were right: the flag alone still let populate_album_implied_genres keep the artist's genres. Fixed in d237f45. The scrobble now fetches the release group's genres and puts them on the album before filling, and keeps the artist fallback only when the release group has none. The new test mocks the MusicBrainz call instead of the fill, and it fails on the previous commit.
Generated by Claude Code
- An album holding the artist's genres made the release-group lookup skip itself. Fetch the release group's list first and put it on the album. - Test the real lookup path (mock the MusicBrainz call, not the fill) and the no-genres case that keeps the fallback. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNPhmcXNv6M1Beby8LQime
Keep the release-group genre fill before the new listen hook and the genre match after it, so a hook that writes genres is matched too. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNPhmcXNv6M1Beby8LQime
Keep both the album genre helper and the new play-link helpers in music_views. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNPhmcXNv6M1Beby8LQime
3 participants
Requested by Daniel · project thread
Summary
latest. Their five content commits are kept with their authorship; one had a small conflict with the new music track page code, resolved by keeping both. Show genres on detail pages and fill them on scrobble #1311's merge-of-latest commit is left out.Screenshots
Before: these pages had no genre chips (the new tests fail without the change).
After: artist, album (own genres and artist fallback) and podcast pages are on this page: https://claude.ai/artifact/CkCsetqcb4wBcxPaeQPymX (private until shared).
Not shown: the episode page, which needs TMDB season data.
AI Assistance
Claude Sonnet 5.5 (
claude-sonnet-5-5) moved the commits ontolatest, wrote the tests and the ordering fix, and ran the browser check.How It Was Tested
scripts/test.sh app.tests.test_music_genres-> Passed (10 tests; the page and copy tests fail without Show genres on detail pages and fill them on scrobble #1311)scripts/test.shover every music test module, the podcast show, template tag and users suites -> 1569 run, 2 errors, both Playwright browser-launch failures in the sandboxuv run --no-sync ruff check src-> PassedPublic API & Documentation Handoff
Human Review & Quality Assurance
/gstack-qaor browser testing)Database & Migration Safety (Only if modifying models)
Related Issues
Supersedes #1311 (to be closed with credit once this merges). Refs #1424.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WNPhmcXNv6M1Beby8LQime