Repository navigation
Fix the moving-phone crash class and the pager's black bars, on a perf + Liquid Glass audit - #144
Merged
Merged
Conversation
The Now Playing transition panel was rebuilt 20 times a second: it read `secondsUntilTransition`, which derives from the tick-driven `position`, so every tick re-ran `TransitionPreview.make`, re-read both tracks' `beatTimes` out of SwiftData and redrew two Canvases. The countdown now mirrors at 1 Hz (`Player.transitionCountdownSeconds`) and the two moving parts — countdown and blend playhead — are leaves, so the panel itself only rebuilds on track changes. Beat positions and curve samples resolve once in `init`. Also on the hot paths: - `AudioCache`/`StemCache`/`ArtworkStore.directory` were computed properties running `createDirectory` + `setResourceValues` on every access — i.e. two syscalls per artwork URL, per row, per frame. Memoized as `static let`. - `Playlist.artworkURL` sorted and copied the whole track relationship on every playlist card body; now a single pass. - Playlist detail and library search rows took the now-playing highlight as a parameter, so each track change invalidated the parent and re-sorted the whole list. They read the player themselves now. Library search also filters before ordering instead of sorting every playlist per keystroke. - Artwork now goes through one shared bounded decode cache (`ArtworkImageStore`) with ImageIO downsampling, replacing `AsyncImage`, which caches nothing and re-decoded full-size JPEGs per row. Backdrop renders are coalesced per URL (the pager asked for the same artwork twice per track change), bounded in an NSCache instead of an unbounded dictionary, and share one `CIContext`. - Launch: the resume pass did up to five `fileExists` probes per track; it now takes one directory listing per cache. Library cleanup's enumeration and deletes moved off the main actor, and playlist deletes no longer re-list the audio cache once per removed track. Session restore and queue refill fetch ids only rather than hydrating every track's analysis arrays. - Stem-cache budget passes coalesce instead of stacking one full directory scan per skip. Liquid Glass: every `Material` stand-in is now a real `glassEffect` — the skip badge, transition countdown and chips, DEMO badge, search pill, error card, suggestion chips and the in-app keyboard's keys, with the key grid and the chip row in `GlassEffectContainer`s so each group composites in one pass. The keyboard's per-keystroke `UIImpactFeedbackGenerator` (never prepared, and a new object reference that invalidated all ~40 keys) is now shared and pre-armed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
Three CPU paths, all verified against the previous implementations rather than eyeballed. `CatalogAutocorrect` runs a bounded Levenshtein against the entire vocabulary on every keystroke, and with the user's library seeded in that is thousands of words. Each candidate used to allocate two scalar arrays and two matrix rows; words now carry their scalars and length from the moment they're learned, and the matrix rows are allocated once per query and reused. A differential run over 5,520 fuzzed queries (three limits each, plus corrections) produces identical output, at roughly a third of the time. `LoudnessMeter.integratedLUFS` ran three integer divisions per sample — tens of millions per track — to wrap the ring buffer, index the sum and test the block boundary. The write cursor now wraps by hand, block boundaries are counted, and each block's sum walks the ring as two contiguous runs, visiting the same elements in the same order. Results are bit-identical on 31 s, 95 s, 7 s and 360 s signals at 44.1 and 48 kHz; a six-minute track measures ~35% faster. `Player.displayProgress` called `Deck.elapsed` on every read, which round-trips through the engine for a render timestamp and a player time. Three progress views read it per tick during a blend; they now share the tick's snapshot, which is the same value they were each recomputing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
…variants The shared artwork and backdrop loads deliberately outlive any one view's task so several views can share one fetch — which means view cancellation no longer reaches them. Without an explicit check, a slow load for the previous track could land after the next track's cached one and repaint in the wrong song's colors, or hand a recycled row the image its previous track asked for. Each call site now drops a result it no longer wants. `NowPlayingBridge` kept whatever the artwork decoded to for as long as the track played — routinely 1400 px square, about 7.8 MB resident, for a lock screen thumbnail. It now decodes bounded through ImageIO and reads local files directly instead of through the URL loading system. A failed load also used to latch `artworkURL` at the track's URL, so every later update took the "already have it" branch and the lock screen stayed blank for the rest of the song; clearing it lets the next update retry. CLAUDE.md gains a "Performance invariants" section so the tick-path, artwork-cache, batching and Liquid Glass rules survive future sessions, and the known-issues list is corrected: what this pass fixed is gone, what it measured and deliberately left (the per-window memmove, `Deck.load`'s file opens, the demo tone synth) is written down with the reasoning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
Importing a folder of local files did all of its heavy work on the main actor: a recursive directory enumeration with per-file resource reads, then a multi-megabyte `copyItem` and an artwork write per song. A real music folder is thousands of syscalls and gigabytes of copying, so the import spinner had no frames to animate in. Enumeration, copy and artwork write now run off it. The duplicate check rescanned the growing "Local Files" playlist for every file imported — three SwiftData property reads per comparison, quadratic over the import. The caller snapshots the playlist once as plain values and carries it forward, with the same title/artist/±1s semantics. It's a lookup now rather than find-or-create, so pointing the picker at a folder with no music in it no longer leaves an empty playlist behind. Bulk enqueues (playlist import, catalog album, Apple Music, sync, retry-all, the launch resume pass) saved the model context once per track — for a 500-track YouTube playlist, 500 synchronous saves in one loop, each costing more than the last. `enqueue` takes `saving:` so those callers save once at the end; single-track callers are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
`fetchBestArtwork` was main-actor isolated, so the ImageIO decode of a 1280x720 maxres thumbnail happened on the main thread before the palette and gaussian work hopped off it. The whole render — fetch, decode, area average, blur — is now one detached task, which is also what makes it correct to share between the two backdrops that request it. Also trims the artwork cache's byte ceiling to 32 MB. This app gates stem separation on 1.4 GB of headroom; a decoded-image cache has no business being a visible fraction of that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
The crashes are not a sensor. The app uses no motion, altimeter or location APIs at all, and is portrait-only — movement is a proxy for audio route churn (AirPods connecting and dropping, car stereos, a jostled cable) and for network path changes. Around a route change or an engine configuration change, AVAudioPlayerNode's time base can report an unset sample rate, and `sampleTime / 0` is `.infinity` in Swift rather than an error. That escaped `Deck.elapsed` into `Player.position`, and from there into two things that trap rather than degrade: `AVAudioFramePosition(seconds * sampleRate)`, which traps on a non-finite double (and `scheduleSegment` raises on a negative start frame), and SwiftUI layout, `Shape.trim` and `Slider`, which trap on one too. A negative clock baseline reached the same conversion. So: `Deck.elapsed` now requires a valid sample time and a positive sample rate and reads 0 otherwise; `scheduleSegment` clamps into the file before converting; `Player.duration` and every published progress fraction go through a clamp that treats non-finite as zero, because `min`/`max` do not sanitize NaN — `min(NaN, 1)` is NaN; `seek` ignores a non-finite target instead of reinterpreting it as zero; the countdown clamps before `Int(_:)`. `PlayerClockSafetyTests` poisons the clock directly and pins the invariant. Two related hardenings on the same theme: the session opts out of system-alert interruptions (every interruption stops the engine, and every engine stop is a chance to hit the check-then-call window where an AVFAudio call raises an uncatchable ObjC exception), and the 158 MB model download is size-validated before being committed — a transfer cut off by a cell handoff, or a captive portal answering with HTML under a 200, used to be cached as "the model" forever and handed to ONNX Runtime as a protobuf on every later separation. The black bars are a measurement mismatch. The pager's GeometryReader is laid out inside the safe area while its ScrollView ignores it, so `proxy.size.height` was short by exactly the insets it reports — while every page's `containerRelativeFrame(.vertical)` is the full window height. The shared backdrop was therefore three insets' worth shorter than the content it backs and, centred as a `.background`, left uncovered bands of flat `systemGroupedBackground` (black in dark mode) at top and bottom, with the album gradient slid out of register with the Now Playing page. Adding the insets back makes the backdrop span exactly its three pages, so the artwork gradient now runs edge to edge under the status bar and home indicator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
`Dictionary(uniqueKeysWithValues:)` traps when a key repeats, and all three uses sat somewhere a trap is the wrong answer: two on the cold-launch path (session restore and the queue-exhausted refill, both building an id lookup from a store fetch) and one behind the Flow toggle. Keeping the first occurrence is the correct behaviour for every one of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
sanylax2
approved these changes
Sep 11, 2026
sanylax2
left a comment
Collaborator
There was a problem hiding this comment.
goated pr back tot he lobby
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 free
to 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.
Two units of work. They're in one PR because the crash fixes build directly on code the
audit introduced, so they don't cherry-pick onto
maincleanly. Say the word and I'll splitthem into a stack instead.
claude/performance-liquid-glass-audit-4t2tskis an ancestor ofthis branch and is redundant once this merges.
Device verification is yours — I'm on Linux, so nothing here has been built or run.
The transient crashes are not a sensor
The app calls no motion, altimeter or location APIs at all, and is portrait-only. "More likely
when the phone is moving" is a proxy for audio route churn — AirPods connecting and
dropping, car stereos, a jostled cable — and for network path changes.
Around a route change or an
AVAudioEngineConfigurationChange,AVAudioPlayerNode's time basecan report an unset sample rate. In Swift
sampleTime / 0is.infinity, not an error, sonothing complained. That escaped
Deck.elapsedintoPlayer.positionand reached two kinds ofcode that trap rather than degrade:
AVAudioFramePosition(seconds * sampleRate)inDeck.scheduleSegmenttraps on a non-finitedouble, and
scheduleSegmentraises an uncatchable ObjC exception on a negative start frame.A negative clock baseline reaches the same conversion.
Shape.trimandSlidertrap on a non-finite number too, so the scrubber andprogress ring were a second, independent way to die on the same value.
Worth knowing:
min/maxare no defence, becausemin(NaN, 1)is NaN. Several places lookedclamped and weren't.
Fixes:
Deck.elapsedrequires a valid sample time and a positive rate, reading 0 otherwise;scheduleSegmentclamps into the file before converting;Player.durationand every publishedfraction go through
Player.fraction, which maps non-finite to 0;seekignores a non-finitetarget rather than reinterpreting it as zero; the 1 Hz countdown clamps before
Int(_:).PlaybackTests/PlayerClockSafetyTestspoisons the clock directly and pins the invariant.Two hardenings on the same theme. The session opts out of system-alert interruptions — every
interruption stops the engine, and every engine stop is another chance at the check-then-call
window where an AVFoundation call raises. And the 158 MB model download is size-validated before
being committed: a transfer cut off by a cell handoff, or a captive portal answering with HTML
under a 200, used to be cached as "the model" permanently and handed to ONNX Runtime as a
protobuf on every later separation.
Also: three
Dictionary(uniqueKeysWithValues:)calls, two of them on the cold-launch path, trapon a duplicate key. They keep the first occurrence now.
The black bars are a measurement mismatch
MainPagerView'sGeometryReaderis laid out inside the safe area while itsScrollViewignores it, so
proxy.size.heightwas short by exactly the insets it reports — while everypage's
containerRelativeFrame(.vertical)is the full window height. The shared backdrop wasthree insets' worth shorter than the content it backs; centred as a
.background, that leftuncovered bands of flat
systemGroupedBackground(black in dark mode) top and bottom, and slidthe album gradient out of register with Now Playing.
pageHeightnow adds the insets back, so the backdrop spans exactly its three pages and theartwork gradient runs edge to edge under the status bar and home indicator. I deliberately did
not restructure the safe-area handling — CLAUDE.md records that the hard-padding architecture is
what fixed the previous round of black bars.
Performance
The Now Playing transition panel rebuilt 20 times a second: it read
secondsUntilTransition,which derives from the tick-driven position, so every tick re-ran
TransitionPreview.make,re-read both tracks'
beatTimesout of SwiftData and redrew two Canvases. The countdownpublishes at 1 Hz now and the two genuinely moving parts are leaves; the panel rebuilds on track
changes.
Also: the three cache directory accessors ran
createDirectory+setResourceValueson everyread (two syscalls per artwork URL, per row, per frame);
Playlist.artworkURLsorted the wholetrack relationship per card; list rows took the now-playing highlight as a parameter, so each
track change invalidated the parent and re-sorted the list; artwork had no cache at all
(
AsyncImagere-decodes per row) and backdrop renders were duplicated per track change andaccumulated unbounded; launch probed the filesystem up to five times per track; a folder import
copied multi-megabyte files on the main actor; playlist import saved the context once per track,
500 times in one loop.
Three CPU paths, measured against the previous implementations rather than eyeballed:
Liquid Glass
Nine places faked glass with
Materialplus hand-drawn hairlines — skip badge, transitioncountdown and chips, DEMO badge, search pill, error card, suggestion chips, and the in-app
keyboard's ~40 keys and backing plane. All real
glassEffectnow. The key grid and chip row sitin
GlassEffectContainers so each group composites in one pass,spacing: 0so neighbours don'tmerge into blobs, and glass is never layered on glass (the keyboard plane stays an opaque fill).
Verification
Parse-checked every file, 195 ContinuityCore tests green, plus two differential harnesses
(autocorrect equivalence, bit-identical LUFS).
PlayerClockSafetyTestsneeds the simulatorscheme on your Mac.
CLAUDE.md gains the new crash class, the pager height invariant, and a "Performance invariants"
section, and its known-issues list is corrected — what this fixed is gone, what I measured and
deliberately left (the per-window memmove,
Deck.load's main-actor file opens, the demo tonesynth) is written down with the reasoning.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MVt2eFfLU2wrwxFSnwCAhc
Generated by Claude Code