Keep what the media session knew when the provider cannot help - #22
Merged
Conversation
Precedence is API first, media session underneath — not the reverse, and not "client first with the API as fallback", which sounds equivalent and is not. The two sources have asymmetric coverage. SMTC supplies title, artist, album, album artist and track number; it never supplies year, release date, genre, album track count, disc or copyright. The API is the only source for half the tag set, so a literal client-first rule would call it on every track anyway and save nothing. The direction worth having is the other one: the provider wins wherever it answers, and what the client already knew fills the gaps. The gap that closes was real and silent. When the match guard rejected every attempt — or no provider was configured, or Spotify was down — the file was written bare, while the media session had already reported artist, title, album, album artist and position for that exact track, with more certainty than any lookup can offer. All of it was discarded. `SmtcSnapshot` gains `AlbumArtist` and `TrackNumber`, both optional so the window-title path and every existing construction site are unchanged. A track number of 0 maps to null, since Spotify numbers from 1 and zero means "not reported"; a blank album artist maps to null rather than to an album credited to the empty string, which would otherwise beat a provider that did know. The load-bearing half is that both mappers now fill rather than clear. `Album`, `AlbumArtists` and `AlbumPosition` are assigned only when the provider has a value. The `SetArtistFromApi` idiom already worked this way for artist and title; this extends it to the three fields the media session can now supply. One existing test asserted the opposite and was updated rather than worked around: `ApplyAlbum_WithAnEmptyResponse` pinned that an empty album object wrote "" and [] over whatever was there. Harmless while the only other source was a window title carrying no album; not harmless once the media session carries one. Cover art is deliberately not taken from SMTC — its thumbnail is far smaller than the API's 640x640, and anything that could win over the API's image is a downgrade users notice. 1024 tests green, 14 new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 freeto 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.
Implements API-first with the media session underneath, as agreed.
Why not literally "client first, API as fallback"
Worth stating because it sounds equivalent and isn't. The two sources have asymmetric coverage:
So the API is the only source for half the tag set. A literal client-first rule would still call it on every track — because year alone guarantees it — and save nothing. The direction that actually pays off is the one you picked: the provider wins wherever it answers, and what the client already knew fills the gaps it leaves.
The gap this closes
It was real and silent. When the match guard rejected all four attempts — or no provider was configured, or Spotify was down — the file was written bare. Meanwhile the media session had already reported artist, title, album, album artist and position for that exact track, with more certainty than any lookup can offer, and every bit of it was discarded.
Same class of failure as the
9Pm (Till I Come)bug: not a wrong answer, but a partial failure degrading all the way to nothing instead of to the next-best thing.What changed
SmtcSnapshotgainsAlbumArtistandTrackNumber, both optional — so the window-title path (which supplies neither) and every existing construction site are untouched. Two mapping rules:0maps to null. Spotify numbers from 1, so zero is "not reported", not a zeroth track.Both mappers now fill rather than clear. The load-bearing half, and a deliberate behaviour change:
Album,AlbumArtistsandAlbumPositionare assigned only when the provider actually has a value, inSpotifyTrackMapperandLastFmTrackMapperalike. TheSetArtistFromApi/SetTitleFromApiidiom already worked this way for artist and title; this extends it to the three fields SMTC can now supply.One test asserted the opposite
ApplyAlbum_WithAnEmptyResponse_ReturnsExpectedTrackpinned that an empty album object wrote""and[]over whatever was there. Updated rather than worked around, with the reasoning left in the test: that behaviour was harmless while the only other source was a window title carrying no album at all. It stopped being harmless the moment the media session started carrying one, because a provider that couldn't answer would erase what the client had said for certain.Deliberately not included
Cover art from the SMTC thumbnail. Much smaller than the API's 640×640, and anything that could win over the API's image is a downgrade people notice in a library. Defensible someday as a last resort for a file that would otherwise have no art; left out until there's a reason to want it. Flagging it as the one piece of SMTC data now going unused.
Genre — the media session has none to give (
count=0, probed live). That was #20.Verification
Rebased onto
mainafter #20 merged, then re-verified from scratch:dotnet buildclean,dotnet format --verify-no-changesclean, 1026 tests green (855 Core + 171 UI), 14 new:Worth checking on a real recording: set the provider to None and confirm files still get album, album artist and track number rather than just artist/title.
🤖 Generated with Claude Code