Metadata correctness, a stoppable shutdown, and the logging audit - #23
Merged
Conversation
The "tagged" line reported album and track position but not genre, which left the one tag whose source is not obvious from the outside as the one tag the log never mentioned. It can arrive from the provider named in the message or from the fallback behind it, and until now the only way to know it had been written at all was to run ffprobe over the finished file — which also means picking the right ffprobe invocation, since Ogg and Opus keep tags at stream level and the obvious -show_format returns nothing. "none" rather than the "unknown" the album and position use, because it is a different answer: those two are missing from a reply that was received, while an empty genre means every source in the chain was asked and none had one. That is exactly the distinction someone working out whether tagging is working needs to see. The formatter is a pure internal helper so it can be asserted directly. The alternative was swapping the static Serilog logger inside a test, which has no precedent here and no parallelism config to make it safe across test classes. 1030 tests green, 4 new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fallback logged its own success at Debug, which the Logs tab hides by default — so the tag's value was visible at Information while where it came from was not, and "the primary had no genre for this artist" is the half worth knowing. Folded into the line that already prints once per track rather than promoted to a second one: a genre the tagging provider supplied itself needs no attribution, since that provider is already named at the start of the message. The suffix appears only when the fallback answered, so an ordinary hit reads exactly as before and no track gains a line it did not have. Spotify tagged Artist - Title: album X, track 4, genre trance, eurodance. Spotify tagged Artist - Title: album X, track 4, genre trip hop (LastFm). IGenreFallback gains Kind for this, which ProviderGenreFallback takes from the provider behind it. ApplyGenreFallbackAsync returns that kind instead of logging, so the decision about how to say it stays with the caller that owns the message. The failure line stays at Debug. It is a non-fatal secondary fault and the rest of the tags are already correct by then; a warning on screen would claim something went wrong with the recording when nothing did. 1034 tests green, 8 new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A pass over every path that can fail, checking each is said once, at a level that matches what the user is supposed to do about it. **Every recording failure was printed twice.** RaiseFailed put the message on the Failed event *and* on the progress report beside it; the controller logs the first at Error and the second at Information. So each failure appeared twice with identical text — once in a colour that means act on this and once in one that means carry on. The report still fires, because it is what moves the display back to waiting; it no longer narrates. The progress message was never read by the UI, so nothing is lost. **A cover-art failure was filed as news.** It went out as a progress message, which lands at Information — below the Problems filter, invisible to anyone who went looking for exactly this. It is now a Warning: the recording is fine and says so, but a tag that was asked for did not get written. Logged directly because a progress message cannot carry a level. **The artist-genre lookup said nothing at all.** Whether it asked Spotify, reused this session's answer, or had no artist id to ask about were all the same silence, which made an empty genre tag impossible to explain. Three Debug lines, one per outcome. **ffmpeg ran unrecorded.** An encode that fails is explained by what it was asked to do, and stderr names an option without saying which invocation carried it. The argv now goes out at Debug before the run. Joined for reading only — the process still gets the array, so this is not a quoting model anyone should rely on. The post-process cover-art write logs too, since Ogg and Opus keep art where -show_format cannot see it and a silent success is indistinguishable from a silent skip. Checked and left alone: settings saves, sign-in, rate limiting, endpoint loss, track-source fallback and encode failure all already log once at a level that fits. The genre fallback's own failure stays at Debug — the rest of the tags are correct by then. 1035 tests green, 1 new pinning that a failure is not narrated twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fallback was working. It just asked one question and stopped. Diagnosed against the live API with a real case: ATB - 9Pm (Till I Come). Spotify returned an empty genre list for the artist, Last.fm's track tags came back as an empty cloud rather than an error, and the tag was left empty -- correctly, given what was asked. Asking Last.fm about the artist instead returns trance, electronic and dance. The answer was one request away the whole time. So the Last.fm rung now asks about the track first and the artist second. Track tags stay preferred, because they describe the recording rather than its performer; they are simply absent for a great many tracks. It no longer goes through the full provider. That path only maps anything -- genres included -- when Last.fm also returns an *album*, which is the wrong gate for a question that never mentioned albums, and it fetched a release, its artwork and its track listing to read three strings off the side. The same lookup that prompted this matched an unrelated compilation as its album, which is the other reason not to let a genre query drag one along. track.getTopTags and artist.getTopTags are the endpoints that answer the actual question, both with autocorrect=1 so Last.fm canonicalises rather than missing on punctuation. Two silences closed while here, both of which made this harder to diagnose than it should have been: the fallback returning nothing, and no fallback being configured at all, were previously indistinguishable in the log from never having run. The second says where to set a Last.fm key. 1029 tests green, 8 new over the two rungs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was only ever asked half a question. The mapper reads genres from the
track's own tag cloud, which Last.fm leaves empty for a great many tracks --
so as the selected provider it tagged album, position, artwork and length
correctly and then handed back no genre at all. The artist's tags were one
request away, and the genre fallback added last commit only ran when Spotify
was primary, so Last.fm users got none of it.
Now the primary path asks the same second question: track tags first, the
artist's behind them. Cached per artist for the life of the session, keyed
case-insensitively since Last.fm has no ids -- an album is one artist repeated,
and that is the cost the cache exists to avoid.
One existing test asserted the lookup sent exactly one request. It now sends
two for a track with no tags, so it asserts the shape of the first rather than
that it is alone; the second is covered on its own.
Verified against the live API rather than assumed, and one finding is worth
recording: Last.fm cannot supply a year. The releasedate field its album
response used to carry now comes back empty, and the published date beside it
is a wiki edit timestamp -- 2024 for a 1999 album -- so mapping it would write
a confidently wrong tag. Left unset. The path template degrades cleanly
already, because Tidy strips the empty brackets a missing {year} leaves behind.
1031 tests green, 3 new.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Last.fm's track.getInfo answers with whichever release its community database associates, and for a well-known track that is regularly a DJ set or a radio show the recording once turned up on. Nothing caught it: this provider had no equivalent of Spotify's DetectedTrackMatch, so whatever came back was written. ATB's "9Pm (Till I Come)" was tagged "A Cutie Who Hates Blu Ray (W/Severine) - N10.AS - 09 - 23 - 2020" over the "Movin' Melodies" the media session had already reported. The session is not guessing there - it is reporting the release the client is playing the track out of - so a Last.fm album that disagrees is a bad match, and its artwork, track listing and credited artists are wrong along with its name. DetectedTrackMatch.AlbumAgrees compares the two through the normalisation already used for titles, treating an edition suffix as more said about the same record rather than a different one, and a rejected release is nulled out so the existing retry-with-a-simplified-title path gets its turn. A rejected release is not a failed lookup. Tags describe the recording, not the release, so the genre still lands on top of what the session supplied - otherwise fixing the album would have cost the genre, which is the one thing that path could still contribute. Also stop the mapper overwriting a real album artist. Last.fm has no album-artist field, so the track's own artist stands in for one; that is right for a window title and wrong the moment a session has reported "Various Artists" on a compilation. It now fills a gap like every other field there rather than replacing an answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SMTC reports AlbumTrackCount alongside TrackNumber and we were reading only the second of the pair, so Track.AlbumTrackCount was populated by Spotify alone. FFmpegArguments needs both halves to write "5/12" and falls back to a bare "5" without the total - which meant the full form required a configured provider, even though the client had already volunteered both numbers. The same zero-means-unreported rule the position uses applies: the count is a count or it is absent, never a zero-track album. The Spotify album mapper now fills rather than clears it too. It set the field unconditionally from TotalTracks, which is non-nullable on the SDK model and so reads as zero on an unpopulated album - harmless while nothing else supplied a count, and a way to lose the session's one now that something does. Its neighbours, the album and its artists, already worked this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closing the window mid-recording left Offstream running with no window and no tray icon, killable only from Task Manager. Three defects, each of which hides the next. OnExit was `async void`, which WPF does not await. It ran as far as its first await, returned, and let Application.Run finish tearing the Dispatcher down - so the continuation carrying the rest of the shutdown was posted to a Dispatcher that would never run it. The host was never stopped, the WASAPI capture client was never closed, the encode backlog was never drained, and Log.CloseAndFlushAsync never ran, which is why the whole failure left nothing in the log to read. It is now a synchronous OnExit that blocks on Task.Run, which both waits properly and keeps the continuations off the Dispatcher - blocking the UI thread on work that wanted to resume on the UI thread would only swap one hang for another. The `using (_host)` then disposed the container synchronously. RecordingController is a singleton implementing IAsyncDisposable only, and ServiceProvider.Dispose() throws InvalidOperationException on such a service rather than falling back - so the one call that stops the running session threw, at the moment it most needed to work. It goes through DisposeAsync now. And that shutdown had no bound. RecordingController.DisposeAsync drains the encode backlog through EncodeBacklog.CompleteAsync, which takes no deadline, so a wedged ffmpeg meant waiting forever with nothing on screen. Thirty seconds - generous, because a queued encode is a file the user is waiting for; finite, because closing Offstream has to end Offstream. On expiry it says so in the log and exits anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems with the Track details box, both of them the same problem:
it described a source that stopped being the primary one.
"Nothing - use the window title only" has been wrong since the media
session became the detection source. Spotify reports artist, title,
album, album artist and track number to Windows, and Offstream tags all
of it whatever is selected here - so the option that reads as "no
metadata" in fact produces a well-tagged file missing only cover art,
genre and year. It now says it uses what Spotify already reports.
And the dropdown named three providers while saying nothing about how
they differ, which left the one question worth asking - what do I lose
by picking this one - answerable only by recording something and
running ffprobe over it. A summary under the box now answers it, phrased
as what each provider *adds*, because none of them is the floor. The
difference that matters most is the least visible: only Spotify carries
a release date, so {year} in a filename template is empty under either
of the others, and nothing said so until a library came out unsorted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>"Track details and cover art are provided by Spotify" overclaimed on both counts. Artist, title, album, album artist and track number come from the Windows media session whatever provider is selected, and genre can come from Last.fm through the fallback even while Spotify is the chosen provider - so the sentence named one source for tags that routinely have two others. The provider summary added alongside the dropdown already names Spotify as the source of what it does supply, on the same page and next to the control that turns it on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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.
Started as "print the genre tag in the activity log" and grew each time the log revealed something real. Ten commits in four groups.
1. Logging audit
Every recording failure was printed twice.
RaiseFailedput the message on theFailedevent and on the progress report beside it — logged atErrorand atInformation, so each failure appeared twice with identical text, once in a colour meaning act on this and once meaning carry on. The report still fires (it moves the display back to waiting); it no longer narrates.A cover-art failure was filed as news. It went out as a progress message, landing at
Information— below the Problems filter, invisible to anyone looking for exactly this. Now aWarning, logged directly, because a progress message cannot carry a level.Silence filled. The artist-genre lookup (asked / cached / no artist id), and the ffmpeg argv before each run, all at
Debug. Ogg and Opus keep cover art where-show_formatcannot see it, so a silent success was indistinguishable from a silent skip.Genre attribution. The "tagged" line now carries genre and names the source when it was not the provider already named:
"none"rather than the"unknown"album and position use: those are missing from a reply that was received, while an empty genre means every source was asked and none had one.2. Last.fm correctness
Genres exist where nobody was asking. Last.fm frequently has no tags for a recording while carrying a rich set for its performer, and the lookup stopped at the empty track answer. It now falls through to
artist.getTopTags, cached per artist for the session. ATB's "9Pm (Till I Come)" is the case that prompted it — no track tags, three good artist tags.Last.fm as the chosen provider was doing less than as the fallback. Its own path only mapped anything when an album came back too, so a track whose tags were sitting there got none.
No match guard at all.
track.getInforeturns whichever release its community database associates, which for a well-known track is regularly a DJ set or radio show. Nothing checked it — Spotify hasDetectedTrackMatch, Last.fm had nothing, so whatever came back was written. ATB's track was tagged with a radio show's tracklist over the "Movin' Melodies" the media session had already reported.DetectedTrackMatch.AlbumAgreesnow compares the two, treating an edition suffix as more said about the same record. A rejected release still yields its genre — tags describe the recording, not the release.3. The media-session floor
AlbumTrackCountwas reported by SMTC and never read, so the5/12track tag needed a configured provider even though the client had volunteered both numbers. The Spotify mapper'sAlbumTrackCountand the Last.fm mapper'sAlbumArtistsboth overwrote rather than filled — the second would replace a session-reported "Various Artists" with whoever performed one track.4. Shutdown, and saying what the providers do
Closing the window mid-recording left a ghost process, killable only from Task Manager. Three defects, each hiding the next:
OnExitwasasync void, which WPF does not await. It ran to its first await, returned, and letApplication.Runtear the Dispatcher down — so the continuation carrying the rest of the shutdown was posted to a Dispatcher that would never run it. The host was never stopped, the capture client never closed, the backlog never drained, andCloseAndFlushAsyncnever ran, which is why this left nothing in the log to read.using (_host)disposed the container synchronously.RecordingControlleris a singleton implementingIAsyncDisposableonly, andServiceProvider.Dispose()throws on such a service rather than falling back — so the one call that stops the session threw, at the moment it most needed to work.EncodeBacklog.CompleteAsynctakes no deadline. Thirty seconds now: generous, because a queued encode is a file the user is waiting for; finite, because closing Offstream has to end Offstream."Nothing — use the window title only" had been wrong since the media session became the detection source, and the dropdown said nothing about how the three providers differ. Each now carries a summary of what it adds — none of them is the floor. The difference that matters most is the least visible: only Spotify carries a release date, so
{year}is empty under the others.The standalone Spotify attribution line is removed as overclaiming; the provider summary names Spotify as the source of what it does supply, in the same place.
Verified, not assumed
releasedatecomes back empty andpublishedis a wiki edit timestamp (2024 on a 1999 album).AlbumArtist,AlbumTitle,AlbumTrackCount,Artist,Genres,PlaybackType,Subtitle,Thumbnail,Title,TrackNumber.IsBackground = trueon both its capture and playback threads, so neither was what held the process up. The exact handle was never identified — no hung process survived to inspect — which is why the shutdown is bounded and logs on expiry.Tests
1051 green (877 Core + 174 UI).
dotnet format --verify-no-changesclean.🤖 Generated with Claude Code