Record page performance, shell redesign, and Spotify API conformance - #11
Merged
Conversation
Two separate reports from a recording session: the Record page felt sluggish while recording, and the elapsed counter sat behind Spotify as if it paused whenever the page was not being watched. The counter was a tally of ticks, not of time. AdvanceElapsed did CurrentPosition += 1 on a one-second PeriodicTimer, and PeriodicTimer schedules the next tick from when the previous one was consumed — it never makes up a late one. Every delayed tick therefore lost a second permanently, so the counter drifted monotonically behind Spotify's own position and could never resync. It now samples a monotonic clock anchored at the track's start, with paused stretches banked rather than counted, so a late tick reports the truth and the drift corrects itself. SpotifyPoller takes a TimeProvider so this is testable without waiting. The waveform was expensive twice over. It emitted a DrawRectangle per bar — several hundred drawing instructions rebuilt thirty times a second — and drove itself from CompositionTarget.Rendering, which wakes the UI thread once per composed frame (144 times a second here) only to decide most of the time that it is not yet time to sample, and keeps WPF composing instead of idling. The bars are now a single frozen StreamGeometry in one DrawGeometry call, sampled by a DispatcherTimer at exactly the rate the scroll speed needs. The scroll itself stays: it is the only thing on the page that distinguishes "recording" from "recording nothing". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three changes against the same report: the Record page still stalls while recording, and the waveform is a solid block that says nothing. The meter was wrong for the job. It reported the loudest sample since the last read, and a peak taken over a thirtieth of a second is at or near full scale for essentially all mastered music — so the display was a filled rectangle whose only information was "audio is arriving". It now reports RMS mapped through a 60 dB scale, which is what loudness actually tracks and what spreads the interesting range across the height available instead of crowding it into the top tenth. The waveform was driving the layout system. InvalidateVisual invalidates arrange as well as rendering, so repainting from OnRender marked the page's tree dirty thirty times a second for a decoration whose size never changes. The bars now render into a child DrawingVisual, which re-composes that one visual and touches layout not at all. The sample timer also drops from Render to Background priority, so it can never sit ahead of user input in the dispatcher queue. HttpClient's own logging was most of the traffic on the log pane. It writes four Information lines per request — start, sending, headers, end — and each one is a dispatcher post, a realised list item and a scroll-to-end on the UI thread, for text like "End processing HTTP request after 501.7018ms". Those categories drop to Warning: failures still surface, the play-by-play does not. Meter tests now assert through a helper that converts an amplitude to the level it should display, so each case still says what its byte layout decodes to, plus two new ones: below the floor reads as silence, and loud and very loud are visibly different heights. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A screen recording settled this: the window is byte-identical for 2.53s and then 2.33s inside fifteen seconds, with the elapsed counter jumping 0:30 to 0:32 without ever drawing 0:31. That is not rendering cost, it is a block. SpotifyPoller.Start() is called from a button click, so the WPF dispatcher is the current SynchronizationContext when the two loops are created — and every await in them captures it. Offstream.Core contains no ConfigureAwait at all (.editorconfig turns CA2007 off with the reasoning "desktop app, context is wanted", which is true of the App and false of the Core), so the continuations came back to the UI thread. Reading Spotify's window title ran there fourteen times a second, every handler these loops raise ran there, and so did StopCurrentRecorder's blocking wait on the outgoing recorder to release the capture buffer — a wait whose own comment calls it safe because it happens "on the poll loop". At a track change that is the UI thread, blocking. Both loops now start with Task.Run, as every other background loop in Core already does. SynchronizationContext.Current is null on the pool, so nothing inside them can find a way back to the dispatcher. Pinned by a test that starts the poller under a counting synchronization context and asserts it is never posted to, and the .editorconfig comment now records why the two projects differ rather than implying the exemption is global. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scrolling waveform could not work against this source. Spotify normalises loudness, so its output sits in a band a few decibels wide: on any fixed scale every bar came out the same height and the control drew a solid block. Moving from peak to RMS made the scale correct without making the picture useful — a waveform needs dynamic range the source does not have. A level bar answers the question actually being asked, which is whether audio is arriving and how loud, without pretending to be a visualisation. It fills left to right, rises instantly and falls gradually, and carries a peak marker that holds for just over a second before decaying. WaveformView becomes LevelMeterView, keeping what the last few commits earned: it samples on a Background-priority timer rather than per composed frame, and renders into a child DrawingVisual so a repaint never invalidates arrange. The history ring buffer goes with the scroll, and drawing is now three rounded rectangles instead of a geometry of several hundred figures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The peak-hold bar answered "is audio arriving" but nothing more. This makes the meter readable as an instrument: two channels, a printed decibel ruler, and a held peak with a number next to it. Core gains per-channel accumulation. AudioLevelMeter folded every sample into one energy total, so there was no left and right to draw; it now attributes each sample to a channel by its position in the frame and drains through a new LevelReading, which carries both the 0-1 level and the dBFS figure it was built from. Two forms rather than one derived at the call site: a control with a scale of its own needs the decibels, and recomputing them from a clamped level is how a bar ends up not lining up with the ruler printed under it. LcdMeterView replaces LevelMeterView. It draws the display of a field recorder - L and R segment bars over a -50..0 dB scale, ticks at -50, -30, -20, -12, -6 and 0, and the held peak in dBFS at the right - because that instrument is already solved and needs no explaining to anyone who has held one. The ruler is the part a progress bar cannot offer: it turns "about two-thirds along" into "near -12 dB". Every position on it comes from LevelReading.Decibels, so the bars and the numbers are one measurement. Three details are load-bearing. Unlit cells stay faintly visible, so a silent meter reads as a working meter showing nothing rather than one that has stopped - which is the whole reason this control exists. The palette is fixed rather than themed, since a physical LCD looks the same in a dark room. And the grid is a tiled mask over continuous bars, one draw call at any width, with the pitch derived from the width to hold about forty cells: at a fixed 5 px a wide panel came out as a hundred hairlines that read as texture on a solid bar rather than as steps. The text layer is skipped while the readout, the width and the channel count all hold, so a sampled frame costs a handful of rectangles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The meter landed as an instrument face; the card above it was still Fluent text, so the page read as a recorder's display with a settings panel glued to its top. This makes the whole thing one piece of glass, which is what the borrowing was for: transport state, what is playing, the counter, the output format and the meters, in the arrangement a field recorder puts them in. The indicator field is the part that earns its place. "Running" and "writing audio" are different states and the transport buttons cannot tell them apart - both show Stop - so the block inverts while audio is reaching the encoder and outlines while the session is armed between tracks. That distinction is the first thing a recordist looks for and the page could not previously show it. The counter is now a fixed-width 00H03M42S. It used to drop the hours until a track had one, which reads better in prose and worse on an instrument: the field changed width partway through a session and the eye had to find it again. It is built from TotalHours rather than an hh format string, which counts hours within a day and would roll a long session back to zero. The format line is new: MP3 320K 44.1K, read from settings so an idle display says what pressing Start would produce. The sample rate only appears once a session exists, because it is the capture endpoint's and is not knowable until the endpoint is open - a guess on the one line claiming to describe the file would be worse than nothing. The buttons stay outside the panel in the app's own style. An LCD is not clickable and styling a button to look like one would be a lie about what can be pressed. Plan §6 gains the Record page display section: the dispatcher capture behind the freeze, the tick-counting behind the drift, why a waveform cannot work against a loudness-normalised source, and the two independent ways the log pane grew. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Standby against rolling is the one distinction the page could not make. A session is running from the moment Start succeeds, but between tracks it is listening rather than writing, and both states show Stop on the buttons and sit in the same indicator field. Every recorder answers this the same way: the indicator blinks while armed and holds solid once audio is reaching the encoder. The blink says "waiting for something to play" without a word. IsArmed is IsRecording without IsCapturing, computed rather than reported so the two can never disagree, and notified from both halves — a computed property with a missing notification leaves the indicator stuck on whichever state it was in when the trigger last evaluated. Discrete key frames, not a fade: an LCD segment has no in-between, and a cross-fade reads as a glow rather than a driven segment. One second per cycle, which is well under the rate that matters for photosensitivity. Gated on SystemParameters.ClientAreaAnimation. Someone who turned animations off in Windows asked for this, and nothing is lost by honouring it — the armed state still reads as an outlined block against capturing's inverted one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rebuilds the window around a flat tab strip with the transport beside it, and rebuilds all three pages onto Offstream's own dark surfaces. NavigationView is gone. The design asks for a label over an accent underline, which is not Fluent's navigation idiom -- reproducing it meant retemplating the control down to a shape it does not have. Three RadioButtons in one group bound to a ShellTab enum is less machinery, and it takes NavigationViewContentPresenter out of the tree with it, which is what had forced ScrollViewer.CanContentScroll="False" onto the Record page. PageProvider is deleted; the shell takes the three pages as constructor parameters, so a missing registration now fails at startup rather than showing a blank tab. The pages became UserControls in the same change, because a Page throws "Page can have only Window or Frame as parent" the moment a ContentControl hosts one -- and it throws from MeasureOverride, so it survives compilation and only appears when the shell arranges. Record: the display carries transport and counter on one line, track and format on the next, meters below, and no status row -- the panel says what state it is in, so the only text worth interrupting for is the app declining to start, which is now a red bar above it. The transport buttons moved to the header, on the Record tab only. Settings: one card, three inline groups, label left and control right on a tight pitch. Advanced: two columns split by subject, with the ten-row token reference moved into a flyout -- inline it pushed everything below it off the page the moment it opened. Neither page scrolls, and MinWidth/MinHeight of 1024x700 is what makes that structural rather than aspirational. 879 tests green, including the 15 FlaUI desktop tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things from the design pass. The bars carry a spectrum now -- muted pigment inks rather than saturated LEDs, so the display still reads as something printed and held. The gradient is pinned to the grid's own pixels with absolute brush mapping, which is the part that matters: an ordinary brush stretches across the bounding box of whatever it fills, and for a level meter that is the reading itself, so every bar would run the full spectrum and clipping red would show up on a signal at -40 dB. Pinned to the scale, a cell's colour says where on the ruler it sits. The held peak takes its colour from the same brush, so it carries the hue of the decibel it is holding at. The transport button is Hidden rather than Collapsed off the Record tab. It is the tallest thing in the header row, so collapsing it took the tab strip up on the way to Settings and back down on the way to Record. The lamp beside the Record tab is always present and changes colour -- dark when idle, recording red when a session is running. A lamp that appears and disappears is a layout that moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The label sat hard left and the control hard right, so at this window
width six hundred empty pixels ran between a setting's name and its
value. Nothing on the page said which belonged to which, and that gulf
was where nearly all the wasted space came from.
Labels now sit directly above their control and the control takes the
whole column, so the pairing is four pixels apart at any width and the
space goes to the fields. Fields run two abreast inside each section:
Settings is one card of three sections, Advanced keeps its two columns
with the same grammar inside them.
A switch is its own answer, so toggles keep the label beside them --
there is no value to put underneath.
Sections got a band: an accent tick, the name in small caps, and a rule
running out to the card edge. The rule used to be a separate separator
above the heading, which read as two unrelated pieces of furniture;
carrying it inside the header ties the label to the span it introduces
and makes a section one element in the markup as well as one thing on the
page. Upper case rather than a larger size, so the band never competes
with the settings under it.
Also sized the fields to their contents -- a two-digit "discard shorter
than" and a clock-shaped timer no longer get full-width boxes -- and made
Advanced's file-counter column Auto, so a template without {count} gives
its half of the card to the policy field instead of holding it empty.
864 tests green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>File names goes across the top. Its content is the widest thing on the page -- a path template, and the rendered path it produces -- and folding that into a half-width column wrapped both onto three lines. Template and presets sit on the left of that card, the resulting path on the right, which is the order they are read in. Detection and Application take a column each below it, because their content is a list of short rows and that is the shape a narrow column suits. Detection was the mess. Every switch was docked to the right of a stretching row, so a maximised window put three hundred empty pixels between a setting and the switch that answered it, and each row's switch landed somewhere different because each label is a different length. All the switches in a section now share one grid whose label column is star with a 330 cap: the cap stops them drifting to the far edge, the star lets the column give ground at the minimum size where the timer needs room on the same row, and a floor on the switch column makes every section resolve to the same width so the switches line up across a section rule rather than only within it. The timer keeps its switch in that column with the duration hanging off it, so it reads as one more on-or-off setting rather than the odd one out it was. One real bug fixed on the way: styling a WPF-UI ToggleSwitch replaces its implicit style outright, template included, so the first pass rendered every switch as a bare two-pixel line. BasedOn the implicit style keeps the control. Cards fill the space they are given now instead of floating above a void. 864 tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects on the panel face. The bezel carried Padding, which shrinks the area every child is given - the scanline overlay included. That left the top twelve pixels and the bottom eight unscanned: two full-width bands a shade lighter than the glass, sitting exactly where a border would sit and reading as one. The breathing room is a Margin on the content now, so the overlay gets the whole face. Sampling a column down the panel shows the sheen gradient stepping one unit every eight rows with no discontinuity, where before there was a hard edge at the padding line. The counter read 00H00M09S. At display size that is nine glyphs of which three are letters, and the eye parses it as a word before it can find the number. Colons are the separator every clock uses, so the grouping is recognised rather than read and the digits are the only things on the line with any weight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…aths Two silent gaps, both invisible at compile time. SpotifyClientConfig.CreateDefault() attaches no retry handler — verified by reflection, not assumed. A 429 therefore threw, fell through TrackEnricher's catch-all, and the track recorded untagged, making rate limiting the one recoverable API failure that was treated as fatal. SpotifyRetryHandler now waits exactly the Retry-After Spotify sends, and backs off exponentially only where there is no such instruction: a 429 without the header, and the 5xx family. Nothing else is retried. It needs no timeout of its own because enrichment already runs under a deadline, so a throttle longer than that cancels the lookup rather than parking a task on it. Three scopes were requested and one was used. Nothing ever called GetCurrentPlayback or GetRecentlyPlayed; the app makes two calls, and the album one needs no user scope at all. A scope asked for ahead of the feature that needs it is a permission granted for nothing, on a consent screen where the spare lines look exactly like the load-bearing one. Existing sign-ins are unaffected — a stored token carries the grant it was issued with. A dead refresh token also had no exit: the 401 was logged like any other fault and retried on every subsequent track while the Settings page went on claiming the account was connected. AuthorizationExpired now fires on that status alone, and clearing the token raises SettingsDocument.Changed, which returns the page to its signed-out state. Every other status is explained using the message Spotify itself sent, which lands in the Record page's activity log. Attribution sits beside the provider that requires it. The Developer Terms' caching clause is deliberately untouched at the user's direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Being throttled was logged at Debug, which is invisible: the Record page's activity log shows Information and above by default, so the one condition that most needs explaining — files coming out untagged — was reported only to whoever thought to switch the filter to "All". A 429 is now a warning naming the wait Spotify asked for, and a throttle that outlasts the retry budget gets a second line saying the lookup was abandoned and that recording itself is unaffected. Transient 5xx stays at Information deliberately: it usually clears on the next attempt, and promoting it would make the Problems filter noisy enough to stop being read. Quota and rate limit are different failures. A 403 can mean the account is not on the dashboard app's allowlist or that the app has run past the user quota its mode allows, and only the body distinguishes them, so it is quoted rather than replaced. The handler takes its logger the way it already took its delay. Log.Logger is static, so asserting on these lines by reassigning it would leak into every test running alongside; injecting ILogger keeps the assertions honest and resolves per call, so production picks up whatever Serilog is configured with rather than whatever existed at construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rules were binding in practice but recorded nowhere in the repo, so they bound nobody else working in it. They go beside the other non-negotiables: spec-derived endpoints, PKCE, loopback-literal redirect, minimum scopes, protected and refreshed tokens, Retry-After honoured on 429, warning-level logging for throttling, quota and rate limit told apart, no deprecated endpoints, and attribution. The Developer Terms' caching clause is deliberately excluded and said to be excluded, so the next reader does not "fix" it: writing tags and cover art into recorded files is what this app is for, and that is the user's call. Separately, three RecordViewModel tests asserted immediately after IProgress<T>.Report. Progress<T> captures the SynchronizationContext at construction and a unit test has none, so the callback goes to the thread pool and Report returns before the view model has changed anything. The suite lost that race about two runs in three — verified as pre-existing at 1f1270d, where it failed three for three, so a green run has not meant much for a while. They now wait on PropertyChanged, the shape ShellViewModelTests already used for the same reason. Five consecutive clean runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
revtex added a commit
that referenced
this pull request
Aug 14, 2026
* Bring the changelog up to date with PRs 12-23 It was last touched in #11 and ten PRs have merged since, so the file described an app several phases behind the one in the tree - which for a Keep a Changelog file is worse than an empty one, because it reads as current. Entries for: the refresh token lost an hour into every session, SMTC as the primary track source and the TFM raise that allows it, endpoint hot-plug, extended-length paths, VB-CABLE detection, the existing-file policy checked before it could know the destination, the Logs tab and the Record page rework, the Spotify match guard, genre from Spotify's artists with Last.fm behind it, the media-session floor and its two mappers, Last.fm's missing album guard, the shutdown that left a ghost process, and the provider summaries on Settings. Readiness is deliberately absent: it was added in #18 and removed in #19, so no release ever carried it and an entry for each would be two lines describing nothing. Test count updated to 1051 (877 Core, 174 UI). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Require a changelog entry per PR, and drop two stray images The changelog going ten PRs stale was not an oversight anyone would catch by trying harder, so it is a rule with a check behind it now rather than a good intention. CLAUDE.md states it; a CI job fails a pull request whose diff does not touch CHANGELOG.md, with a `no-changelog` label as the escape hatch so a test-only fix opts out by saying so instead of by staying quiet. The job runs on ubuntu with no `needs`, so it answers in seconds alongside the Windows build rather than behind it - which is why it is a job of its own, despite the note on `build` arguing against exactly that for publish-check. That case was gated on `needs: build` and paid for a second VM to run strictly afterwards; this one does not. Also removes 06aa2f03-...jpg and b514cfc3-...jpg from the repository root. Both were untracked, and both were mine to be suspicious of and wrong about: I guessed earlier they were leaked cover-art downloads. They are not. CoverArtFetcher writes to GetTempPath() under the name "<random>.offstream-cover.jpg" and never to the working directory, and these were a matched pair of 1024x572 images, which is not the shape album art comes in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- 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
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.
Started as a fix for two Record page reports and grew to cover the redesign that followed, then a conformance pass over the Spotify client. Eight commits; the CI check on this PR now covers all of them.
The reports that started it
The counter was a tally of ticks, not of time.
AdvanceElapsedincremented on a one-secondPeriodicTimer, which schedules the next tick from when the previous one was consumed and never makes up a late one — so every delayed tick lost a second permanently and could never resync. That is the "it pauses when I'm not looking" symptom exactly. It now samples a monotonic clock anchored at the track's start, with paused stretches banked rather than counted.The waveform was expensive twice over: a
DrawRectangleper bar rebuilt thirty times a second, andCompositionTarget.Renderingwaking the UI thread at the display's refresh rate to mostly decide it was not yet time to sample. Now one frozen geometry, and aDispatcherTimerat the sample rate.The real freeze was neither.
SpotifyPoller.Start()was called from a button click, so it captured the WPFSynchronizationContextand everyawaitin the poll loop resumed on the UI thread — window-title reads fourteen times a second, plus a blocking wait on the capture buffer at every track change. Proven by hashing the frames of a screen recording: 2.53s and 2.33s of byte-identical frames. Three earlier diagnoses were all wrong; each made the page cheaper to draw and none touched a blocked thread.Redesign
Rebuilt to the supplied mockups.
NavigationViewis gone — the flat tab strip isRadioButtons bound to a tab enum, which also removed theDynamicScrollViewerinfinite-height trap. Pages becameUserControls (Pagecannot be hosted in aContentControl— it throws fromMeasureOverride).The LCD gained colour: the meter's lit cells run a muted e-paper spectrum keyed to the dB scale, not to the current reading. That is the whole trick —
BrushMappingMode.Absolutepins the gradient to grid pixels, so a cell's colour is a function of its ruler position instead of stretching across whatever the level happens to be.Both settings pages were relaid out twice: labels now sit above their controls rather than across a 600px gulf, File names runs full width, and Detection's switches share one grid so they align across sections. Minimum window size is 1024x700 — the no-scroll promise made structural.
Display fixes: the bezel's
Paddingwas shrinking the scanline overlay too, leaving unscanned strips top and bottom that read as mismatched borders; and the counter now reads00:00:00instead of00H00M00S.Spotify Web API conformance
Audited against the rules now written into
CLAUDE.md. Two real gaps, both silent at runtime:SpotifyClientConfig.CreateDefault()attaches no retry handler (verified by reflection), so rate limiting — the one recoverable API failure — was fatal.SpotifyRetryHandlerhonoursRetry-Afterexactly, and backs off exponentially only where there is no such instruction.user-read-currently-playing. Existing sign-ins are unaffected; a stored token carries the grant it was issued with.Also: a dead refresh token now clears itself and returns the user to sign-in (401 only — a rate limit or outage must not sign anyone out); Spotify's own error message is what reaches the activity log; throttling logs at Warning, because the log shows Information and above and anything quieter is invisible.
The Developer Terms' caching clause is deliberately out of scope and documented as such.
Also
A flaky test fixed. Three
RecordViewModeltests asserted immediately afterIProgress<T>.Report;Progress<T>captures theSynchronizationContextat construction and a unit test has none, so the callback goes to the thread pool andReportreturns first. The suite lost that race about two runs in three — verified pre-existing at1f1270d, where it failed three for three.906 tests green (738 core + 168 UI), 0 warnings, format clean.
The FlaUI desktop suite is excluded from CI and was not run: per your instruction, UI is verified manually. Its cross-fixture failure remains open.
🤖 Generated with Claude Code